enhance Nim support for shared libraries and rpath handling#7272
enhance Nim support for shared libraries and rpath handling#7272waruqi merged 27 commits intoxmake-io:devfrom
Conversation
…rms; add Nim test files for static and shared libraries
Summary of ChangesHello @luadebug, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue where executables linked against shared libraries failed to run on Linux due to an inability to locate the libraries at runtime. The core change involves refactoring the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the _make_runpath_on_windows function into a more generic _make_runpath to support shared library runpaths across multiple platforms, including Linux and macOS, by correctly setting LD_LIBRARY_PATH or DYLD_LIBRARY_PATH. The addition of Nim test cases for static and shared library linking is a good step to validate this new functionality. However, there's a potential issue in how addenvs and setenvs are managed, which could lead to unintended modifications of setenvs or incorrect application of runpath values.
xmake/modules/core/tools/nim.lua
Outdated
| if self:is_plat("linux", "macosx", "bsd") then | ||
| if level == "debug" or level == "all" then | ||
| return "--passL:-s" | ||
| return "--passL:\"-s\"" |
There was a problem hiding this comment.
The previous configuration is not working?
There was a problem hiding this comment.
I think that only flags containing spaces need to be enclosed in double quotes.
There was a problem hiding this comment.
Aw I am confused over this step. Maybe it should be like --passL:"-s" but no matter what it keeps applying before " the \ symbol... Feel free to adjust to your like.
That stuff works but it was not wrapped into "" for safety, that is what Ive assumed.
…ks in dependency flags
| add_files("mainlib.nim") | ||
| add_deps("staticlib") | ||
| add_packages("zlib", "stb") | ||
| add_syslinks("pthread", "m") |
There was a problem hiding this comment.
Are these two libraries required? Do the dependencies require them? These two libraries are not compiling on Windows.
There was a problem hiding this comment.
Yeah my oversight... They are not required, I was just testing on my Linux and forgor about that.
|
It does not work on macOS. checking for /usr/local/bin/nim ... ok
checking for the nim compiler (nc) ... nim
checking for /usr/local/bin/nim ... ok
checking for flags (-d:release) ... ok
[ 27%]: linking.release libsharedlib.dylib
checking for /usr/local/bin/nim ... ok
checking for the nim shared library linker (ncsh) ... nim
/usr/local/bin/nim c -d:release --passC:-Iinc --nimcache:build/.gens/sharedlib/macosx/x86_64/release/nimcache --app:lib --noMain --passL:-s -o:build/macosx/x8
6_64/release/libsharedlib.dylib shared.nim
checking for the nim compiler (nc) ... nim
[ 27%]: linking.release libstaticlib.a
checking for /usr/local/bin/nim ... ok
checking for the nim static library archiver (ncar) ... nim
/usr/local/bin/nim c -d:release --passC:-Iinc --nimcache:build/.gens/staticlib/macosx/x86_64/release/nimcache --app:staticlib --noMain --passL:-s --nimMainPre
fix:libstatic -o:build/macosx/x86_64/release/libstaticlib.a static.nim
error: Hint: used config file '/usr/local/Cellar/nim/2.2.6/nim/config/nim.cfg' [Conf]
Hint: used config file '/usr/local/Cellar/nim/2.2.6/nim/config/config.nims' [Conf]
......................................................................
CC: system/exceptions.nim
CC: std/private/digitsutils.nim
CC: system/dollars.nim
CC: system.nim
CC: static.nim
/Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/staticlib/macosx/x86_64/release/nimcache/@mstatic.nim.c:36:10: fatal error: 't
est_header.h' file not found
36 | #include <test_header.h>
| ^~~~~~~~~~~~~~~
1 error generated.
Error: execution of an external compiler program 'clang -c -w -ferror-limit=3 -fno-strict-aliasing -pthread -Iinc -O3 -I/usr/local/Cellar/nim/2.2.6/nim/lib
-I/Users/ruki/projects/personal/xmake/tests/projects/nim/link_library -o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/stat
iclib/macosx/x86_64/release/nimcache/@mstatic.nim.c.o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/staticlib/macosx/x86_64/
release/nimcache/@mstatic.nim.c' failed with exit code: 1
|
Why it did not wrap flags into |
|
My mistake, I used incorrect xmake version. But it does not work. /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library
ruki:link_library ruki$ xmake f -c
checking for platform ... macosx
checking for architecture ... x86_64
checking for Xcode directory ... /Applications/Xcode.app
checking for SDK version of Xcode for macosx (x86_64) ... 15.2
checking for Minimal target version of Xcode for macosx (x86_64) ... 15.2
updating repositories .. ok
ruki:link_library ruki$ xmake -rv
checking for /usr/local/bin/nim ... ok
checking for the nim compiler (nc) ... nim
checking for /usr/local/bin/nim ... ok
checking for the nim compiler (nc) ... nim
checking for flags (-d:release) ... ok
[ 27%]: linking.release libsharedlib.dylib
checking for /usr/local/bin/nim ... ok
checking for the nim shared library linker (ncsh) ... nim
/usr/local/bin/nim c --cpu:amd64 --define:bit64 -d:release --passC:\"-Iinc\" --nimcache:build/.gens/sharedlib/macosx/x86_64/release/nimcache --passC:\"-Iheade
rs\" --app:lib --noMain --passL:\"-s\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/macosx/x86_64/release/libshare
dlib.dylib shared.nim
[ 27%]: linking.release libstaticlib.a
checking for /usr/local/bin/nim ... ok
checking for the nim static library archiver (ncar) ... nim
/usr/local/bin/nim c --cpu:amd64 --define:bit64 -d:release --passC:\"-Iinc\" --nimcache:build/.gens/staticlib/macosx/x86_64/release/nimcache --passC:\"-Iheade
rs\" --app:staticlib --noMain --passL:\"-s\" --nimMainPrefix:libstatic -o:build/macosx/x86_64/release/libstaticlib.a static.nim
[ 47%]: linking.release executablestatic
checking for /usr/local/bin/nim ... ok
checking for the nim linker (ncld) ... nim
/usr/local/bin/nim c --cpu:amd64 --define:bit64 -d:release --nimcache:build/.gens/executablestatic/macosx/x86_64/release/nimcache --passC:\"-I/Users/ruki/.xma
ke/packages/z/zlib/v1.3.1/3323a56e4f3c40c2895f732924aff059/include\" --passL:\"-L/Users/ruki/.xmake/packages/z/zlib/v1.3.1/3323a56e4f3c40c2895f732924aff059/li
b\" --passL:\"-lz\" --passC:\"-I/Users/ruki/.xmake/packages/s/stb/2025.03.14/aa287dcb4c2b4e16a25c6aaf2eee6e26/include\" --passC:\"-I/Users/ruki/.xmake/package
s/s/stb/2025.03.14/aa287dcb4c2b4e16a25c6aaf2eee6e26/include/stb\" --passC:\"-Iheaders\" --passC:\"-Iinc\" --passL:\"-L/Users/ruki/.xmake/packages/z/zlib/v1.3.
1/3323a56e4f3c40c2895f732924aff059/lib\" --passL:\"-Lbuild/macosx/x86_64/release\" --passL:\"-s\" --passL:\"-lz\" --passL:\"-lstaticlib\" --threads:on --passL
:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/macosx/x86_64/release/executablestatic mainlib.nim
[ 67%]: linking.release executableshared
/usr/local/bin/nim c --cpu:amd64 --define:bit64 -d:release --nimcache:build/.gens/executableshared/macosx/x86_64/release/nimcache --passL:\"-Wl,-rpath=build/m
acosx/x86_64/release\" --passC:\"-Iheaders\" --passC:\"-Iinc\" --passL:\"-Lbuild/macosx/x86_64/release\" --passL:\"-Wl,-rpath=\$ORIGIN\" --passL:\"-s\" --pass
L:\"-lsharedlib\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/macosx/x86_64/release/executableshared maindll.nim
error: Hint: used config file '/usr/local/Cellar/nim/2.2.6/nim/config/nim.cfg' [Conf]
Hint: used config file '/usr/local/Cellar/nim/2.2.6/nim/config/config.nims' [Conf]
..............................................................................................
CC: system/exceptions.nim
CC: std/private/digitsutils.nim
CC: std/assertions.nim
CC: system/dollars.nim
CC: system.nim
CC: hashes.nim
CC: math.nim
CC: tables.nim
CC: strutils.nim
CC: shared.nim
CC: maindll.nim
Hint: [Link]
ld: warning: -s is obsolete
ld: unknown options: -rpath=build/macosx/x86_64/release -rpath=$ORIGIN
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Error: execution of an external program failed: 'clang -o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/macosx/x86_64/release/ex
ecutableshared /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@psystem@sexce
ptions.nim.c.o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@pstd@sprivate@
sdigitsutils.nim.c.o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@pstd@sas
sertions.nim.c.o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@psystem@sdol
lars.nim.c.o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@psystem.nim.c.o
/Users/ruki/projects/personal/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@phashes.nim.c.o /Users/ruki/p
rojects/personal/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@pmath.nim.c.o /Users/ruki/projects/persona
l/xmake/tests/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@ptables.nim.c.o /Users/ruki/projects/personal/xmake/tests
/projects/nim/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@pstrutils.nim.c.o /Users/ruki/projects/personal/xmake/tests/projects/n
im/link_library/build/.gens/executableshared/macosx/x86_64/release/nimcache/@mshared.nim.c.o /Users/ruki/projects/personal/xmake/tests/projects/nim/link_libra
ry/build/.gens/executableshared/macosx/x86_64/release/nimcache/@mmaindll.nim.c.o "-Wl,-rpath=build/macosx/x86_64/release" "-Lbuild/macosx/x86_64/release" "-
Wl,-rpath=\$ORIGIN" "-s" "-lsharedlib" "-lpthread" "-lm" -ldl'
|
|
Lets assume we would swap |
On macOS, we need to additionally pass -Xlinker, and we need to replace $ORIGIN. xmake/xmake/modules/core/tools/gcc.lua Lines 783 to 784 in 45f856e |
|
I think updating nim from 2.0.8 of yay's repo to pixi's repo 2.2.8 has caused: Alright this wors now... As I switched back to yay's nim 2.0.8 Switching zlib back from shared to static validates the result that all is fine: |
| set_project("link_libs") | ||
| add_rules("mode.debug", "mode.release") | ||
|
|
||
| add_requires("zlib", {system = false, config = {shared = true}}) |
There was a problem hiding this comment.
configs
warning: add_requires("zlib") has unknown option: {config=table: 0x600001918740}!
There was a problem hiding this comment.
Thanks it works.
[ 27%]: linking.release libsharedlib.so
checking for /usr/bin/nim ... ok
checking for the nim shared library linker (ncsh) ... nim
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --passC:\"-Iinc\" --nimcache:build/.gens/sharedlib/linux/x86_64/release/nimcache --passC:\"-Iheaders\" --app:lib --noMain --passL:\"-s\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/linux/x86_64/release/libsharedlib.so shared.nim
checking for the nim compiler (nc) ... nim
[ 27%]: linking.release libstaticlib.a
checking for /usr/bin/nim ... ok
checking for the nim static library archiver (ncar) ... nim
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --passC:\"-Iinc\" --nimcache:build/.gens/staticlib/linux/x86_64/release/nimcache --passC:\"-Iheaders\" --app:staticlib --noMain --passL:\"-s\" --nimMainPrefix:libstatic -o:build/linux/x86_64/release/libstaticlib.a static.nim
[ 47%]: linking.release executablestatic
checking for /usr/bin/nim ... ok
checking for the nim linker (ncld) ... nim
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --nimcache:build/.gens/executablestatic/linux/x86_64/release/nimcache --passC:\"-I/home/lin/.xmake/packages/z/zlib/v1.3.1/26943dc7c5a74a63bede41ae03809796/include\" --passL:\"-L/home/lin/.xmake/packages/z/zlib/v1.3.1/26943dc7c5a74a63bede41ae03809796/lib\" --passL:\"-lz\" --passC:\"-I/home/lin/.xmake/packages/s/stb/2025.03.14/67005fc11486400896028eb775a57270/include\" --passC:\"-I/home/lin/.xmake/packages/s/stb/2025.03.14/67005fc11486400896028eb775a57270/include/stb\" --passC:\"-Iheaders\" --passC:\"-Iinc\" --passL:\"-L/home/lin/.xmake/packages/z/zlib/v1.3.1/26943dc7c5a74a63bede41ae03809796/lib\" --passL:\"-Lbuild/linux/x86_64/release\" --passL:\"-s\" --passL:\"-lz\" --passL:\"-lstaticlib\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/linux/x86_64/release/executablestatic mainlib.nim
[ 67%]: linking.release executableshared
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --nimcache:build/.gens/executableshared/linux/x86_64/release/nimcache --passL:\"-Wl,-rpath=build/linux/x86_64/release\" --passC:\"-Iheaders\" --passC:\"-Iinc\" --passL:\"-Lbuild/linux/x86_64/release\" --passL:\"-Wl,-rpath=\$ORIGIN\" --passL:\"-s\" --passL:\"-lsharedlib\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/linux/x86_64/release/executableshared maindll.nim
with static zlib
[ 27%]: linking.release libsharedlib.so
checking for /usr/bin/nim ... ok
checking for the nim shared library linker (ncsh) ... nim
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --passC:\"-Iinc\" --nimcache:build/.gens/sharedlib/linux/x86_64/release/nimcache --passC:\"-Iheaders\" --app:lib --noMain --passL:\"-s\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/linux/x86_64/release/libsharedlib.so shared.nim
checking for the nim compiler (nc) ... nim
[ 27%]: linking.release libstaticlib.a
checking for /usr/bin/nim ... ok
checking for the nim static library archiver (ncar) ... nim
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --passC:\"-Iinc\" --nimcache:build/.gens/staticlib/linux/x86_64/release/nimcache --passC:\"-Iheaders\" --app:staticlib --noMain --passL:\"-s\" --nimMainPrefix:libstatic -o:build/linux/x86_64/release/libstaticlib.a static.nim
[ 47%]: linking.release executablestatic
checking for /usr/bin/nim ... ok
checking for the nim linker (ncld) ... nim
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --nimcache:build/.gens/executablestatic/linux/x86_64/release/nimcache --passC:\"-I/home/lin/.xmake/packages/z/zlib/v1.3.1/994fafa590ed48ac9f71516cc846d155/include\" --passL:\"-L/home/lin/.xmake/packages/z/zlib/v1.3.1/994fafa590ed48ac9f71516cc846d155/lib\" --passL:\"-lz\" --passC:\"-I/home/lin/.xmake/packages/s/stb/2025.03.14/67005fc11486400896028eb775a57270/include\" --passC:\"-I/home/lin/.xmake/packages/s/stb/2025.03.14/67005fc11486400896028eb775a57270/include/stb\" --passC:\"-Iheaders\" --passC:\"-Iinc\" --passL:\"-L/home/lin/.xmake/packages/z/zlib/v1.3.1/994fafa590ed48ac9f71516cc846d155/lib\" --passL:\"-Lbuild/linux/x86_64/release\" --passL:\"-s\" --passL:\"-lz\" --passL:\"-lstaticlib\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/linux/x86_64/release/executablestatic mainlib.nim
[ 67%]: linking.release executableshared
/usr/bin/nim c --cpu:amd64 --define:bit64 -d:release --nimcache:build/.gens/executableshared/linux/x86_64/release/nimcache --passL:\"-Wl,-rpath=build/linux/x86_64/release\" --passC:\"-Iheaders\" --passC:\"-Iinc\" --passL:\"-Lbuild/linux/x86_64/release\" --passL:\"-Wl,-rpath=\$ORIGIN\" --passL:\"-s\" --passL:\"-lsharedlib\" --threads:on --passL:\"-lpthread\" --dynlibOverride:\"pthread\" --passL:\"-lm\" -o:build/linux/x86_64/release/executableshared maindll.nim
xmake/rules/nim/build/target.lua
Outdated
| local pkg_includedirs = pkg:get("includedirs") | ||
| if pkg_includedirs then | ||
| for _, dir in ipairs(pkg_includedirs) do | ||
| local includeflags = compinst:_tool():nf_includedir(dir) |
There was a problem hiding this comment.
please use compiler.map_flags or compinst:map_flags
xmake/xmake/core/tool/builder.lua
Line 797 in 45f856e
xmake/xmake/rules/qt/moc/xmake.lua
Lines 82 to 90 in 45f856e
There was a problem hiding this comment.
and we should use target:get_from to get values from target (deps, packages, ...)
xmake/xmake/rules/qt/moc/xmake.lua
Lines 70 to 77 in 45f856e
xmake/rules/nim/build/target.lua
Outdated
| local pkg_linkdirs = pkg:get("linkdirs") | ||
| if pkg_linkdirs then | ||
| for _, dir in ipairs(pkg_linkdirs) do | ||
| local linkflags = compinst:_tool():nf_linkdir(dir) |
xmake/rules/nim/build/target.lua
Outdated
| -- deduplicate | ||
| includedirs = table.unique(includedirs) | ||
| for _, includedir in ipairs(includedirs) do | ||
| local includeflags = compinst:_tool():nf_includedir(includedir) |
There was a problem hiding this comment.
we need to remove all private api calls. compinst:_tool()
xmake/rules/nim/build/target.lua
Outdated
| local rpathdirs_wrap = {} | ||
|
|
||
| -- add rpathdirs from dependencies | ||
| if target:kind() == "binary" or target:kind() == "shared" then |
There was a problem hiding this comment.
target:is_binary(), target:is_shared()
xmake/rules/nim/build/target.lua
Outdated
| -- we need pass includedirs of static/shared lib to the target | ||
| local includedirs = {} | ||
| for _, dep in ipairs(target:orderdeps()) do | ||
| if dep:kind() == "static" or dep:kind() == "shared" or dep:kind() == "headeronly" then |
| add_rules("mode.debug", "mode.release") | ||
|
|
||
| add_requires("zlib", {system = false, configs = {shared = true}}) | ||
| add_requires("zlib", {system = false, configs = {shared = false}}) |
There was a problem hiding this comment.
default shared config is false, you can remove this configs
| add_deps("staticlib") | ||
| add_packages("zlib", "stb") | ||
| if is_plat("linux") then | ||
| add_syslinks("pthread", "m") |
There was a problem hiding this comment.
No. I just added that to validate syslinks behavior. As it used to be ignore add_syslinks.
xmake/rules/nim/build/target.lua
Outdated
| end | ||
|
|
||
| -- add dependency flags | ||
| function _add_dependency_flags(target, compinst, compflags) |
There was a problem hiding this comment.
I think adding these flags here is incorrect; it completely bypasses the logic of xmake compiler.compflags in retrieving flags, which is a rather hacky implementation.
We simply need to add the missing named flags item to languages/nim/xmake.lua
| target("headers") | ||
| set_kind("headeronly") | ||
| add_files("headers/*.h") | ||
| add_includedirs("headers") |
There was a problem hiding this comment.
The current implementation does not follow the visibility logic of add_includedirs("xxx", {public = true})
| set_kind("binary") | ||
| add_files("mainlib.nim") | ||
| add_deps("staticlib") | ||
| add_packages("zlib", "stb") |
There was a problem hiding this comment.
and here. {public = true}
Currently, they are all privately visible, but other targets can still access their includedirs.
|
|
||
| target("headers") | ||
| set_kind("headeronly") | ||
| add_files("headers/*.h") |
There was a problem hiding this comment.
and it should be add_headerfiles("header/*.h")
Lets assume we have got static and shared library to link against for our executable. It looks like this won't build if we do not help Linux to figure out where is linkdir to check for so files... :(
Well at least static library is being linked and works without issues. I am prolly lack of Windows/MacOS environment to check.
#6488 mentions 3rd option is
--passL:"..."So I tried to extract current flags:
Once we apply
"..."wrapper... flags would change a little bit:I have noticed that I was unable to build x86 executable... :( So I had to enforce cpu via
--cpu:%ARCH%flag. So it would produce x86 executable instead of x64 executable.Without my last commit a9761dd it would not see headerfile at all from binary files perspective...
#include "test.h"