search

Goddard-Fortran-Ecosystem / pFUnit

Parallel Fortran Unit Testing Framework
Other
169 stars 45 forks source link

CMake Bugfix: Set runtime output dir of posix_predefined.x #449

Closed LiamPattinson closed 4 months ago

LiamPattinson commented 7 months ago

This fixes a small CMake bug which can lead to posix_predefined.x being built in the wrong build subdirectory. If CMAKE_RUNTIME_OUTPUT_DIRECTORY is set*, the following lines will fail:

https://github.com/Goddard-Fortran-Ecosystem/pFUnit/blob/49a83b1ff56a953a00468bb783aac31dad1895d1/src/funit/core/CMakeLists.txt#L60-L68

When CMAKE_RUNTIME_OUTPUT_DIRECTORY is set, posix_predefined.x will be built somewhere other than CMAKE_CURRENT_BINARY_DIR, so the command that produces posix_predefined.inc will fail. These changes ensure posix_predefined.x will always be built in the correct subdirectory.

I made a little test repo while I was trying to figure out what was going wrong, and this demonstrates the bug. It uses either FetchContent or Git submodules to load pFUnit.

* In my case, this was being set at a global level by a separate dependency of my project. I normally always prefer to set things like output directories as properties on individual targets, but in this case I'm stuck with it.

tclune commented 7 months ago

Note to self. I need to add a Changelog entry when this passes CI.

tclune commented 7 months ago

@LiamPattinson Please note that in my experience the "robust" option is generally not robust. I've never been able to track it down but I think it is due to weak consistency constraints on filesystems.

tclune commented 7 months ago

(And thank you @LiamPattinson for the contribution.)

LiamPattinson commented 7 months ago

Happy to help! For any future contributions, should I make changelog edits myself?

tclune commented 7 months ago

Yes - please do. Even if it just a placeholder I can then mark it up in the PR, which is easier than the alternatives. Cheers.