Add VERSION_VAR to find_package_handle_standard_args to handle FFTW version - #10
Conversation
|
This sounds like a good addition! I have just added CI and testing to this repo. Could you please add a test for this new functionality in test/CMakeLists.txt? Let me know if you need any help with it. Basically, just make sure the version variable is available after running |
|
By the way, I just noticed that I had totally missed previous PR #8 with the same goal from @kprussing, apologies for not responding to that one. |
|
No worries. I’m short on time at the moment so I’m not sure when I could get a test for my PR. So long as we get the desired functionality I call it a win 😉 |
Set the new FFTW_VERSION variable from the pkg-config version when FFTW is found via pkg-config. Update README.md to document the new variable.
- Accept FFTW_TEST_VERSION to pass version requirement to find_package - Validate that FFTW_VERSION is non-empty and meets the requested version - Add CI steps for version 3 and invalid version rejection - Add version output to success message
… used When `pkg-config` is not found or `FFTW_ROOT` is set, the version of FFTW was not determined. This commit adds a fallback that attempts to extract the version from the `fftw-wisdom-to-conf` binary using its `-V` flag. If the command fails, a warning is issued with the output and error messages. The assignment of `FFTW_VERSION` is now conditional: it is set from `pkg-config` results only when available, and otherwise from the fallback method. This ensures the version variable is only populated when a reliable source is found.
|
Hello. I have added tests with version, including both positive and negative cases and CI passed in my fork. I also updated Sorry I also missed your PR but I wish I gave you credit correctly as shown in code. |
|
Another thing I'd like to confirm is could we format these files with cmake-format? I'd like to use the default setting of cmake-format, but it will look quite differently since it use 2-space indent instead of 4. |
egpbos
left a comment
egpbos
left a comment
There was a problem hiding this comment.
Thank you so much for fleshing this out! Great that you also added the 999.9.9 test. This PR is almost ready to merge, but I have a few final requests below.
I'm happy to take a look at this, but please do this in a separate PR. |
|
I apologize for the mess in the test. Now they are fixed. |
egpbos
left a comment
egpbos
left a comment
There was a problem hiding this comment.
This is awesome, thanks for this great PR!
3 participants
Current implementation does not support specifying minimum version for FFTW, e.g.
Current implementation will set FFTW_FOUND even if fftw found has a version less than specified.
Add VERSION_VAR points to PKG_FFTW_VERSION fix that.
Reference: https://cmake.org/cmake/help/v3.10/module/FindPackageHandleStandardArgs.html.