Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account for Clang based toolchains in CMake logic for --whole-archive flag generation #2120

Closed
ghost opened this issue Jul 7, 2022 · 2 comments · Fixed by #2204
Closed
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Jul 7, 2022

Is your feature request related to a problem? Please describe.
In the target CMakeLists.txt file, there is logic that conditionally adds the --whole-archive linker options. This logic currently checks for a compiler ID of "GNU". When using a VxWorks 7 toolchain that is based on Clang, this logic is not triggered.

Describe the solution you'd like
Adjust the logic to allow for Clang based toolchains. The vxWorks toolchain has compatible options so only the toolchain identification needs to be changed.

Describe alternatives you've considered
We could move these flags into the toolchain files, but that would require changing all existing toolchain files.

Additional context
The current logic is here:
https://github.com/nasa/cFE/blob/main/cmake/target/CMakeLists.txt#L159

The new logic needs to look something like this:
if (("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU") OR
("${CMAKE_C_COMPILER_ID}" STREQUAL "Clang"))

Requester Info
Alan Cudmore, NASA/GSFC Code 582.0

NOTE: This change is likely to be part of a set of changes to support vxWorks 7.0

@skliper
Copy link
Contributor

skliper commented Jul 7, 2022

Could also consider for unmatched compilers if the START_WHOLE_ARCHIVE and STOP_WHOLE_ARCHIVE cmake variables aren't set throw an error with a notification that these should be set from your toolchain file (or update the default processing).

@ghost
Copy link
Author

ghost commented Jul 7, 2022

Good idea - it would be nice to not require someone to hunt this down again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants