Skip to content
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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Respect CMAKE_EXE_LINKER_FLAGS/CMAKE_SHARED_LINKER_FLAGS flags set via CLI #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbed7dc
Respect CMAKE_EXE_LINKER_FLAGS/CMAKE_SHARED_LINKER_FLAGS flags set vi…
pramodk Jun 22, 2022
6f9c31e
Add linker flags after coreneuron library (clang and -Wl,--as-needed …
pramodk Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMake/config/CompilerFlagsHelpers.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--as-needed ${CMAKE_SHARED_LINKER_FLAGS}")

# rest of the world
else()
Expand Down
7 changes: 4 additions & 3 deletions extra/nrnivmodl_core_makefile.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ endif

CXXFLAGS = @CORENRN_CXX_FLAGS@
CXX_COMPILE_CMD = $(CXX) $(CXXFLAGS) @CMAKE_CXX_COMPILE_OPTIONS_PIC@ @CORENRN_COMMON_COMPILE_DEFS@ $(INCLUDES)
CXX_LINK_EXE_CMD = $(CXX) $(CXXFLAGS) @CMAKE_EXE_LINKER_FLAGS@
CXX_LINK_EXE_CMD = $(CXX) $(CXXFLAGS)
CXX_LINK_EXE_FLAGS = @CMAKE_EXE_LINKER_FLAGS@
CXX_SHARED_LIB_CMD = $(CXX) $(CXXFLAGS) @CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS@ @CMAKE_SHARED_LIBRARY_CXX_FLAGS@ @CMAKE_SHARED_LINKER_FLAGS@

# ISPC compilation and link commands
Expand Down Expand Up @@ -210,9 +211,9 @@ endif
# main target to build binary
$(SPECIAL_EXE): coremech_lib_target
@printf " => $(C_GREEN)Binary$(C_RESET) creating $(SPECIAL_EXE)\n"
$(CXX_LINK_EXE_CMD) -o $(SPECIAL_EXE) $(CORENRN_SHARE_CORENRN_DIR)/coreneuron.cpp \
$(CXX_LINK_EXE_CMD) -o $(SPECIAL_EXE) $(CORENRN_SHARE_CORENRN_DIR)/coreneuron.cpp \
-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) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see the changes to this file just re-order some flags. Is that important?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now see the commit message about it needing to be later. Can you add a comment/documentation about that issue/detail?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

-Wl,-rpath,'$(LIB_RPATH)' -Wl,-rpath,$(CORENRN_LIB_DIR) -Wl,-rpath,'$(INSTALL_LIB_RPATH)'

coremech_lib_target: $(corenrnmech_lib_target)
Expand Down