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

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

Add support for rpath with pkg-config #20449

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ikskuh
Copy link
Contributor

@ikskuh ikskuh commented Jun 28, 2024

Right now, pkg-config can output -Wl,-rpath,… which is ignored by the parser.

On NixOS, rpath is an important concept for linking system libraries, especially when they are not the "native" library (x86 on x86_64).

This patch should fix this by configuring the Zig linker such that it respects these rpath requirements

@@ -741,6 +741,8 @@ fn runPkgConfig(compile: *Compile, lib_name: []const u8) !PkgConfigResult {
try zig_cflags.appendSlice(&[_][]const u8{ "-D", macro });
} else if (mem.startsWith(u8, arg, "-D")) {
try zig_cflags.append(arg);
} else if (mem.startsWith(u8, arg, "-Wl,-rpath,")) {
try zig_cflags.appendSlice(&[_][]const u8{ "-rpath", arg[11..] });
Copy link
Contributor

Choose a reason for hiding this comment

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

imo "-Wl,-rpath,".len would read better than a magic number 11.
Even better would be const wl_rpath_arg_prefix = "-Wl,-rpath,"; and using that constant in both places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, you're right. I've also cleaned up the stray commit from my previous PR

@ikskuh ikskuh force-pushed the add_pkg_config_rpath_support branch 2 times, most recently from b996a3e to 3745f50 Compare June 30, 2024 08:55
@ikskuh ikskuh force-pushed the add_pkg_config_rpath_support branch from 3745f50 to 902f669 Compare July 2, 2024 20:04
@@ -741,6 +743,8 @@ fn runPkgConfig(compile: *Compile, lib_name: []const u8) !PkgConfigResult {
try zig_cflags.appendSlice(&[_][]const u8{ "-D", macro });
} else if (mem.startsWith(u8, arg, "-D")) {
try zig_cflags.append(arg);
} else if (mem.startsWith(u8, arg, wl_rpath_prefix)) {
try zig_cflags.appendSlice(&[_][]const u8{ "-rpath", arg[wl_rpath_prefix.len..] });
Copy link
Member

Choose a reason for hiding this comment

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

hmm, this should use the compile.addRPath API instead of C flags I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

very reasonable, i'll fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewrk: This is basically doing exactly the same as addRPath, which is appending -rpath in appendZigProcessFlags as well.
If we would be using addRPath , we'd need to run the pkg-config during the configure phase instead of the build phase, which would be weird, as we'd run it twice then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants