This repository has been archived by the owner on Mar 20, 2023. It is now read-only.
Respect CMAKE_EXE_LINKER_FLAGS/CMAKE_SHARED_LINKER_FLAGS flags set via CLI #830
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…a CLI In case of Clang, we were always overriding the above mentioned flags and hence cmake args were ignored (resuliting in link errors)
pramodk
force-pushed
the
pramodk/omp-simd
branch
from
010d8c6 to
6f9c31e
Compare
olupton
approved these changes
Jun 22, 2022
| @@ -113,8 +113,8 @@ foreach(COMPILER_LANGUAGE ${SUPPORTED_COMPILER_LANGUAGE_LIST}) | |||
| # Force same ld behavior as when called from gcc --as-needed forces the linker to check whether | |||
| # a dynamic library mentioned in the command line is actually needed by the objects being | |||
| # linked. Symbols needed in shared objects are already linked when building that library. | |||
| set(CMAKE_EXE_LINKER_FLAGS "-Wl,--as-needed") | |||
| set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--as-needed") | |||
| set(CMAKE_EXE_LINKER_FLAGS "-Wl,--as-needed ${CMAKE_EXE_LINKER_FLAGS}") | |||
There was a problem hiding this comment.
You can use https://cmake.org/cmake/help/latest/command/string.html#append to avoid repeating the variable name, I think, but no big deal.
| -I$(CORENRN_INC_DIR) $(INCFLAGS) \ | ||
| -L$(OUTPUT_DIR) -l$(COREMECH_LIB_NAME) $(CORENRNLIB_FLAGS) $(LDFLAGS) \ | ||
| -L$(OUTPUT_DIR) -l$(COREMECH_LIB_NAME) $(CORENRNLIB_FLAGS) $(LDFLAGS) $(CXX_LINK_EXE_FLAGS) \ |
There was a problem hiding this comment.
As far as I can see the changes to this file just re-order some flags. Is that important?
There was a problem hiding this comment.
I now see the commit message about it needing to be later. Can you add a comment/documentation about that issue/detail?
There was a problem hiding this comment.
yeah, I should have put this PR in draft - haven't thoroughly checked how/why -Wl,--as-needed causes this but I remember this flag was needed in case wheel building. I will check this bit later and get back here.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In the case of Clang, we were always overriding the above-mentioned flags
and hence cmake args were ignored (resulting in link errors)
How to test this?
On BB5 with Clang compiler
Test System
BB5