Fix cmake build: use target_link_libraries instead of LINK_FLAGS by rljacobuc · Pull Request #26 · SEATStandards/ncvis

rljacobuc · 2026-03-05T22:59:26Z

LINK_FLAGS places library flags before object files in the linker command, causing undefined reference errors with GNU ld which requires libraries to appear after the object files that use them.

Replace LINK_FLAGS with target_link_options (for rpath) and target_link_libraries (for wxWidgets and NetCDF libraries) so that the link order is correct.

LINK_FLAGS places library flags before object files in the linker command, causing undefined reference errors with GNU ld which requires libraries to appear after the object files that use them. Replace LINK_FLAGS with target_link_options (for rpath) and target_link_libraries (for wxWidgets and NetCDF libraries) so that the link order is correct. Co-Authored-By: Oz <[email protected]>

Copilot

Pull request overview

Fixes GNU ld undefined-reference link errors by ensuring libraries are emitted after objects in the final link command.

Changes:

Removes LINK_FLAGS usage from set_target_properties.
Adds target_link_options for rpath and target_link_libraries for wxWidgets/NetCDF linkage.
Parses wx/NetCDF link flags into CMake lists via separate_arguments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T23:14:06Z

src/CMakeLists.txt

 )

+# Link libraries after object files (required for GNU ld)
+target_link_options(ncvis PRIVATE -Wl,-rpath,${WXCONFIG_RPATH}/lib)


Using a raw -Wl,-rpath,... linker flag is toolchain-specific and bypasses CMake’s dedicated RPATH handling. Prefer setting BUILD_RPATH / INSTALL_RPATH (or CMAKE_INSTALL_RPATH) via target properties for better portability and clearer intent, while still achieving correct runtime search paths.

Suggested change

)

# Link libraries after object files (required for GNU ld)

target_link_options(ncvis PRIVATE -Wl,-rpath,${WXCONFIG_RPATH}/lib)

BUILD_RPATH "${WXCONFIG_RPATH}/lib"

INSTALL_RPATH "${WXCONFIG_RPATH}/lib"

)

# Link libraries after object files (required for GNU ld)

Copilot · 2026-03-05T23:14:06Z

src/CMakeLists.txt

+# Link libraries after object files (required for GNU ld)
+target_link_options(ncvis PRIVATE -Wl,-rpath,${WXCONFIG_RPATH}/lib)
+separate_arguments(WX_LINK_LIBS UNIX_COMMAND "${WX_LINK_FLAGS}")
+separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")


Passing -L… search-path flags via target_link_libraries works but is not idiomatic CMake and can be harder to reason about across generators. Prefer expressing the library directory via target_link_directories(ncvis PRIVATE ${NC_LIB_DIR}) (or linking to an imported target like NetCDF::NetCDF if available) and keep target_link_libraries focused on actual libraries (e.g., netcdf).

Suggested change

separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")

separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-lnetcdf ${NC_LINK_FLAGS}")

target_link_directories(ncvis PRIVATE ${NC_LIB_DIR})

Copilot · 2026-03-05T23:14:07Z

src/CMakeLists.txt

+separate_arguments(WX_LINK_LIBS UNIX_COMMAND "${WX_LINK_FLAGS}")
+separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")
+target_link_libraries(ncvis PRIVATE ${WX_LINK_LIBS} ${NC_LINK_LIBS})
+


${WX_LINK_FLAGS} / ${NC_LINK_FLAGS} typically contain a mix of true libraries (-l...) and non-library linker options (e.g., -pthread, -Wl,...). Feeding the whole parsed list into target_link_libraries makes the intent unclear and can complicate future changes. Consider splitting the parsed values into (1) libraries for target_link_libraries and (2) flags for target_link_options, so each API is used for its intended purpose.

Suggested change

separate_arguments(WX_LINK_LIBS UNIX_COMMAND "${WX_LINK_FLAGS}")

separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")

target_link_libraries(ncvis PRIVATE ${WX_LINK_LIBS} ${NC_LINK_LIBS})

separate_arguments(WX_LINK_ITEMS UNIX_COMMAND "${WX_LINK_FLAGS}")

separate_arguments(NC_LINK_ITEMS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")

# Split wxWidgets link items into libraries (-l...) and other linker options

set(WX_LIBRARIES "")

set(WX_OPTIONS "")

foreach(item IN LISTS WX_LINK_ITEMS)

if(item MATCHES "^-l.+")

list(APPEND WX_LIBRARIES "${item}")

else()

list(APPEND WX_OPTIONS "${item}")

endif()

endforeach()

# Split NetCDF link items into libraries (-l...) and other linker options

set(NC_LIBRARIES "")

set(NC_OPTIONS "")

foreach(item IN LISTS NC_LINK_ITEMS)

if(item MATCHES "^-l.+")

list(APPEND NC_LIBRARIES "${item}")

else()

list(APPEND NC_OPTIONS "${item}")

endif()

endforeach()

# Link true libraries

target_link_libraries(ncvis PRIVATE ${WX_LIBRARIES} ${NC_LIBRARIES})

# Apply non-library linker options

target_link_options(ncvis PRIVATE ${WX_OPTIONS} ${NC_OPTIONS})

Copilot · 2026-03-05T23:14:07Z

src/CMakeLists.txt

+separate_arguments(WX_LINK_LIBS UNIX_COMMAND "${WX_LINK_FLAGS}")
+separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")


separate_arguments(... UNIX_COMMAND ...) hard-codes UNIX-style parsing. If this project is ever generated on non-UNIX platforms, this can mis-parse arguments. Using NATIVE_COMMAND (or guarding with if(UNIX) and providing an alternative) makes the behavior more robust.

Suggested change

separate_arguments(WX_LINK_LIBS UNIX_COMMAND "${WX_LINK_FLAGS}")

separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")

separate_arguments(WX_LINK_LIBS NATIVE_COMMAND "${WX_LINK_FLAGS}")

separate_arguments(NC_LINK_LIBS NATIVE_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")

rljacob · 2026-03-05T23:19:10Z

@copilot make a PR to apply changes based on the comments in this thread

- Use BUILD_RPATH/INSTALL_RPATH properties instead of raw -Wl,-rpath flag - Use target_link_directories instead of -L flag in separate_arguments - Split link items into libraries (target_link_libraries) and flags (target_link_options) - Use NATIVE_COMMAND instead of UNIX_COMMAND in separate_arguments Co-Authored-By: Oz <[email protected]>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T05:33:47Z

src/CMakeLists.txt

+separate_arguments(WX_LINK_ITEMS NATIVE_COMMAND "${WX_LINK_FLAGS}")
+separate_arguments(NC_LINK_ITEMS NATIVE_COMMAND "-lnetcdf ${NC_LINK_FLAGS}")
+target_link_directories(ncvis PRIVATE ${NC_LIB_DIR})
+
+# Split wxWidgets link items into libraries (-l...) and other linker options
+set(WX_LIBRARIES "")
+set(WX_OPTIONS "")
+foreach(item IN LISTS WX_LINK_ITEMS)
+    if(item MATCHES "^-l.+")
+        list(APPEND WX_LIBRARIES "${item}")
+    else()
+        list(APPEND WX_OPTIONS "${item}")
+    endif()
+endforeach()
+
+# Split NetCDF link items into libraries (-l...) and other linker options
+set(NC_LIBRARIES "")
+set(NC_OPTIONS "")
+foreach(item IN LISTS NC_LINK_ITEMS)
+    if(item MATCHES "^-l.+")
+        list(APPEND NC_LIBRARIES "${item}")
+    else()
+        list(APPEND NC_OPTIONS "${item}")
+    endif()
+endforeach()
+
+# Link true libraries
+target_link_libraries(ncvis PRIVATE ${WX_LIBRARIES} ${NC_LIBRARIES})
+
+# Apply non-library linker options
+target_link_options(ncvis PRIVATE ${WX_OPTIONS} ${NC_OPTIONS})


target_link_directories() and target_link_options() are used here, but the project’s top-level cmake_minimum_required(VERSION 3.1) is lower than the CMake versions that provide these commands. As-is, configuring with older CMake will fail. Either bump the minimum required CMake version accordingly, or rewrite this logic using commands/properties available in the current minimum version (while still keeping libraries in target_link_libraries).

rljacob requested a review from Copilot March 5, 2026 23:04

Copilot started reviewing on behalf of rljacob March 5, 2026 23:11 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

rljacob requested a review from Copilot March 6, 2026 05:30

Copilot started reviewing on behalf of rljacob March 6, 2026 05:31 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cmake build: use target_link_libraries instead of LINK_FLAGS#26

Fix cmake build: use target_link_libraries instead of LINK_FLAGS#26
rljacobuc wants to merge 2 commits intoSEATStandards:mainfrom
rljacobuc:fix/cmake-link-order

rljacobuc commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

rljacob commented Mar 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")
	separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-lnetcdf ${NC_LINK_FLAGS}")
	target_link_directories(ncvis PRIVATE ${NC_LIB_DIR})

		separate_arguments(WX_LINK_LIBS UNIX_COMMAND "${WX_LINK_FLAGS}")
		separate_arguments(NC_LINK_LIBS UNIX_COMMAND "-L${NC_LIB_DIR} -lnetcdf ${NC_LINK_FLAGS}")

Conversation

rljacobuc commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rljacob commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rljacob commented Mar 5, 2026 •

edited

Loading