Skip to content

Fix ubuntu symbol drop #109

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.

Sign up for GitHub

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

Merged
chengcli merged 2 commits into from
Feb 5, 2026
Merged

Fix ubuntu symbol drop #109

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
7e3f498
wip
Jan 30, 2026
954ecf5
wip
Feb 5, 2026
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: 4 additions & 0 deletions cmake/macros/macro_setup_cuda_test.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,9 @@ macro(setup_cuda_test namel)
target_link_libraries(
${namel}.${buildl} PRIVATE snapy::snap snapy::snap_cu gtest_main)

if (UNIX AND NOT APPLE)
target_link_options(${namel}.${buildl} PRIVATE -Wl,--no-as-needed)
endif()

add_test(NAME ${namel}.${buildl} COMMAND ${namel}.${buildl})
endmacro()
4 changes: 4 additions & 0 deletions cmake/macros/macro_setup_problem.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,8 @@ macro(setup_problem namel)
${namel}.${buildl}
PRIVATE snapy::snap
$<IF:$<BOOL:${CUDAToolkit_FOUND}>,snapy::snap_cu,>)

if (UNIX AND NOT APPLE)
target_link_options(${namel}.${buildl} PRIVATE -Wl,--no-as-needed)
endif()
endmacro()
4 changes: 4 additions & 0 deletions cmake/macros/macro_setup_test.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,9 @@ macro(setup_test namel)
$<IF:$<BOOL:${CUDAToolkit_FOUND}>,snapy::snap_cu,>
gtest_main)

if (UNIX AND NOT APPLE)
target_link_options(${namel}.${buildl} PRIVATE -Wl,--no-as-needed)
endif()

add_test(NAME ${namel}.${buildl} COMMAND ${namel}.${buildl})
endmacro()
15 changes: 12 additions & 3 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,16 @@ def parse_library_names(libdir):
library_names.extend(['netcdf'])

# move current library name to first
current = [item for item in library_names if item.startswith('snap')]
other = [item for item in library_names if not item.startswith('snap')]
return current + other
#current = [item for item in library_names if item.startswith('snap')]
#other = [item for item in library_names if not item.startswith('snap')]
Comment on lines +24 to +25
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The commented-out code should be removed rather than kept as comments. Leaving old code commented out reduces code maintainability and can cause confusion for future developers.

Suggested change
#current = [item for item in library_names if item.startswith('snap')]
#other = [item for item in library_names if not item.startswith('snap')]

Copilot uses AI. Check for mistakes.

# 1) non-cuda libs first (consumers)
snap_non_cuda = [l for l in library_names if l.startswith("snap") and "cuda" not in l]
# 2) cuda libs last (providers)
snap_cuda = [l for l in library_names if l.startswith("snap") and "cuda" in l]
# 3) everything else
other = [l for l in library_names if not l.startswith("snap")]
return snap_non_cuda + other + snap_cuda

site_dir = sysconfig.get_paths()["purelib"]

Expand Down Expand Up @@ -58,11 +65,13 @@ def parse_library_names(libdir):
]
else:
extra_link_args = [
"-Wl,--no-as-needed",
"-Wl,-rpath,$ORIGIN/lib",
"-Wl,-rpath,$ORIGIN/../torch/lib",
"-Wl,-rpath,$ORIGIN/../pydisort/lib",
"-Wl,-rpath,$ORIGIN/../pyharp/lib",
"-Wl,-rpath,$ORIGIN/../kintera/lib",
"-Wl,--as-needed",
Comment on lines +68 to +74
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The --no-as-needed and --as-needed linker flags may not work as intended when placed in extra_link_args. The effectiveness of these flags depends on their position relative to the library link commands. In many build systems, extra_link_args are processed either before or after the libraries specified in the 'libraries' parameter, which could make these flags ineffective. The library ordering changes (lines 27-33) may be sufficient to fix the symbol drop issue without these flags. Consider testing whether these linker flags are actually necessary, or if the library reordering alone resolves the Ubuntu symbol drop problem.

Suggested change
"-Wl,--no-as-needed",
"-Wl,-rpath,$ORIGIN/lib",
"-Wl,-rpath,$ORIGIN/../torch/lib",
"-Wl,-rpath,$ORIGIN/../pydisort/lib",
"-Wl,-rpath,$ORIGIN/../pyharp/lib",
"-Wl,-rpath,$ORIGIN/../kintera/lib",
"-Wl,--as-needed",
"-Wl,-rpath,$ORIGIN/lib",
"-Wl,-rpath,$ORIGIN/../torch/lib",
"-Wl,-rpath,$ORIGIN/../pydisort/lib",
"-Wl,-rpath,$ORIGIN/../pyharp/lib",
"-Wl,-rpath,$ORIGIN/../kintera/lib",

Copilot uses AI. Check for mistakes.
]

ext_module = cpp_extension.CppExtension(
Expand Down
Loading