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

Improve error message when target and optimize is not declared in a dependency #18575

Open
kuon opened this issue Jan 15, 2024 · 8 comments
Open
Labels
error message This issue points out an error message that is unhelpful and should be improved. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@kuon
Copy link
Contributor

kuon commented Jan 15, 2024

Zig Version

0.12.0-dev.2236+32e88251e

Steps to Reproduce and Observed Behavior

When I import a module in my code like this:

    const hello = b.dependency("hello", .{
        .target = target,
        .optimize = optimize,
    });
    exe.root_module.addImport("hello", hello.module("hello"));

I get this error:

error: Invalid option: -Dcpu
error: Invalid option: -Dtarget
error: Invalid option: -Doptimize
/usr/lib/zig/std/Build.zig:1739:35: 0x1116feb in dependency__anon_13722 (build)
            return dependencyInner(b, name, pkg.build_root, if (@hasDecl(pkg, "build_zig")) pkg.build_zig else null, pkg.deps, args);
                                  ^
/home/kuon/Playground/zig-bug/exe/build.zig:15:31: 0x10ccea4 in build (build)
    const hello = b.dependency("hello", .{
                              ^
/usr/lib/zig/std/Build.zig:1858:33: 0x10b8543 in runBuild__anon_8179 (build)
        .Void => build_zig.build(b),
                                ^
/usr/lib/zig/build_runner.zig:319:29: 0x10b41df in main (build)
        try builder.runBuild(root);
                            ^
/usr/lib/zig/std/start.zig:585:37: 0x109f2f5 in posixCallMainAndExit (build)
            const result = root.main() catch |err| {
                                    ^
/usr/lib/zig/std/start.zig:253:5: 0x109ede1 in _start (build)
    asm volatile (switch (native_arch) {
    ^
???:?:?: 0x7 in ??? (???)
Unwind information for `???:0x7` was not available, trace may be incomplete

I made a repo to reproduce the issue:

https://github.com/kuon/zig-bug-2024-01-15

Expected Behavior

Code should compile without error or with a clearer error.

@kuon kuon added the bug Observed behavior contradicts documented or intended behavior label Jan 15, 2024
@xdBronch
Copy link
Contributor

the dependency doesnt declare target or optimize options (b.standardTargetOptions(.{}) and b.standardOptimizeOption(.{})) in its build.zig so when you pass target and optimize to the b.dependency call it gives this error. error message could probably be improved but this isnt a bug

@xdBronch
Copy link
Contributor

side note, in both of the build.zig.zon files in your reproduction repo you only put the "main" file under the paths field. this will cause all other files (including build.zig and zon!) to be deleted if the package is fetched by the package manager
unrelated to this issue but just wanted to point it out just in case you happen to run into that later

@kuon
Copy link
Contributor Author

kuon commented Jan 15, 2024

@xdBronch I thought it had to be something missing on my part, thanks. I think we should keep this open and improve the error message.

About the clarification about the paths field, it was just for my reproduction, but it is good to point it out for future references.

@kuon kuon changed the title Obscure error when adding module from another package Improve error message when target and optimize is not declared in a dependency Jan 15, 2024
@kuon
Copy link
Contributor Author

kuon commented Jan 15, 2024

Just to be sure, and for references, this is the proper way to declare modules in a package's build.zig?

pub fn build(b: *std.Build) void {
    const target = b.standardTargetOptions(.{}); // Was missing
    const optimize = b.standardOptimizeOption(.{}); // Was missing
    const hal = b.addModule("hal", .{
        .root_source_file = .{ .path = "src/hal.zig" },
        .target = target, // Was missing
        .optimize = optimize, // Was missing
    });
    const devices = b.addModule("devices", .{
        .root_source_file = .{ .path = "src/devices.zig" },
        .target = target, // Was missing
        .optimize = optimize, // Was missing
    });
    devices.addImport("hal", hal);
}

@xdBronch
Copy link
Contributor

its probably good practice in case a project wants to override the target/optimize mode for a specific dependency but its not strictly required to declare the target and optimize options, modules will inherit them from the parent module e.g. the exe in most cases. if you want both hal and devices exposed to outside packages then yeah that looks like what i'd do.
if only devices needs to be public i personally would probably do something more like this

pub fn build(b: *std.Build) void {
    _ = b.addModule("devices", .{
        .root_source_file = .{ .path = "src/devices.zig" },
        .imports = &.{
            .{
                .name = "hal",
                .module = b.createModule(.{
                    .root_source_file = .{ .path = "src/hal.zig" },
                }),
            },
        },
    });
}

but i wouldnt necessarily consider this more correct, just depends on how the package is used

@Vexu Vexu added zig build system std.Build, the build runner, `zig build` subcommand, package management error message This issue points out an error message that is unhelpful and should be improved. and removed bug Observed behavior contradicts documented or intended behavior labels Jan 15, 2024
@Vexu Vexu added this to the 0.13.0 milestone Jan 15, 2024
@Srekel
Copy link

Srekel commented Apr 11, 2024

modules will inherit them from the parent module

Is it intentional that the ABI is not inherited? This caught me by surprise and frankly made me spend a lot of time hunting this down. If I build the exe with MSVC ABI I think you would generally want everything to be built with MSVC, right?

@Srekel
Copy link

Srekel commented Apr 11, 2024

Just as a note, this was further made harder to fix due to the second parameter to b.dependency being anytype, making it pretty hard to know what I could pass in there. Relevant issue: #17198

@anticrisis
Copy link

I just ran into this too. I was following this example on how to define build.zig in a module intended to be imported as a dependency: https://github.com/ziglibs/known-folders/blob/master/build.zig.

What is the guidance on how to correctly define build.zig for a module dependency? Seems like it would be better to allow .target and .optimize to be defined by the parent. I'm new to Zig but don't quite understand why omitting those fields in the call to addModule would make it impossible for the parent to define them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error message This issue points out an error message that is unhelpful and should be improved. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Development

No branches or pull requests

5 participants