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

std.Build.Module.linkSystemLibrary() breaks target inheritance #20042

Open
Tracked by #8
ifreund opened this issue May 22, 2024 · 2 comments
Open
Tracked by #8

std.Build.Module.linkSystemLibrary() breaks target inheritance #20042

ifreund opened this issue May 22, 2024 · 2 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@ifreund
Copy link
Member

ifreund commented May 22, 2024

Zig Version

0.12.0 (nothing looks to have changed on master)

Steps to Reproduce and Observed Behavior

I have a zig-wlroots package with idiomatic Zig bindings to the wlroots C library. This package exposes a wlroots Zig module that internally uses @cImport() to obtain the wlroots version defined in the installed C headers:

https://codeberg.org/ifreund/zig-wlroots/src/commit/938a314162ee5ec9abc6dc8e6e930d5e52c87a8b/src/version.zig

This means that the wlroots Zig module must be provided with the include path for the wlroots C headers, which in turn should be obtained using pkg-config. Currently, the only pkg-config integeration in std.Build is the linkSystemLibrary() function.

The problem is that calling std.Build.Module.linkSystemLibrary() on the wlroots module errors due to this line:

const target = m.requireKnownTarget();

which requires the target to be explicitly specified rather than inherited.

My current workaround is to set the Module.resolved_target field directly before calling Module.linkSystemLibrary():

https://codeberg.org/river/river/src/commit/7fdba05b8249b10d10a2c64c1175429539c01af1/build.zig#L148-L149

Expected Behavior

There are many different changes that could be made which would support this specific use-case well. I'm not sure which option makes the most sense. Here are some reasonable options I can think of:

  1. Make modules inherit include paths from their parent compilation. This would side-step the issue entirely and I would only need a single linkSystemLibrary() call for the parent compilation.

  2. Add a new std.Build API to use pkg-config to only add the required include paths, not link the library. Don't synchronously require a resolved target in this new Module.addIncludePathPkgConfig() function.

  3. Change Module.linkSystemLibrary() to not synchronously require a resolved target. It currently uses this to determine if the requested library name is actually part of libc/libc++. This check could however be deferred until std.Build.Complie.make() if I'm not mistaken.

I don't have a strong opinion on which of these approaches is the best.

@ifreund ifreund added bug Observed behavior contradicts documented or intended behavior zig build system std.Build, the build runner, `zig build` subcommand, package management enhancement Solving this issue will likely involve adding new logic or components to the codebase. and removed bug Observed behavior contradicts documented or intended behavior labels May 22, 2024
@Vexu Vexu added this to the 0.14.0 milestone May 22, 2024
@pfgithub
Copy link
Contributor

Would this solve it?

    const wlroots = b.dependency("zig-wlroots", .{
        .target = target,
        .optimize = optimize
    }).module("wlroots");

Along with in zig-wlroots:

    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});

    _ = b.addModule("wlroots", .{
        .root_source_file = .{ .path = "src/wlroots.zig" },
        .target = target,
        .optimize = optimize,
    });

@ifreund
Copy link
Member Author

ifreund commented May 23, 2024

@pfgithub setting the target and optimize of the module from standard{Target,Optimize}Options() in zig-wlroots seems to have the same effect regardless of whether or not I pass those values in the dependency() call in the consumer's build.zig. I don't understand what the intended behavior is here.

On a higher level, I think it is indicative of a design flaw if inheritance of the target/optimize options works fine for all cases as long as one doesn't call linkSystemLibrary(). Calling that function doesn't change the fact that I semantically want inheritance and it shouldn't require me to change anything else in my build.zig IMO.

Either inheritance should become explicit through always requiring the user to use standard{Target,Optimize} Options() or it should work regardless of what std.Build API is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Development

No branches or pull requests

3 participants