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

LibExeObjStep.addBuildOption doesn't import the type #3127

Open
SamTebbs33 opened this issue Aug 27, 2019 · 3 comments
Open

LibExeObjStep.addBuildOption doesn't import the type #3127

SamTebbs33 opened this issue Aug 27, 2019 · 3 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@SamTebbs33
Copy link
Contributor

addBuiltOption doesn't import the type passed.

build.zig

const Builder = @import("std").build.Builder;
const builtin = @import("builtin");
const Arch = builtin.Arch;

pub fn build(b: *Builder) void {
    const exe = b.addExecutable("foo", "foo.zig");
    exe.addBuildOption(Arch, "arch", builtin.arch);
    exe.install();
    b.default_step.dependOn(&exe.step);
}

foo.zig

const options = @import("zig-cache/foo_build_options.zig");
const warn = @import("std").debug.warn;

pub fn main() void {
    warn(@tagName(options.arch));
}
/home/sam/repos/zig/build/zig build-exe /home/sam/foo.zig --pkg-begin build_options /home/sam/zig-cache/foo_build_options.zig --pkg-end --cache-dir /home/sam/zig-cache --name foo --cache on
/home/sam/zig-cache/foo_build_options.zig:1:18: error: use of undeclared identifier 'Arch'
pub const arch = Arch{ .x86_64 = void };

If the type is not one of the built-in types (uX, bool etc.) then an import should be added.

@andrewrk andrewrk added this to the 0.6.0 milestone Aug 27, 2019
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Aug 27, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 11, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 17, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Nov 6, 2020
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
@courajs
Copy link
Contributor

courajs commented Oct 23, 2021

I've just added build option support for arrays/slices of primitives in #10016

But I've been trying to figure out whether there's a good way to support structs/unions in the general case.

  1. How can we know whether a type is from the standard library or not? I can only think either of a prefix check on @typeName, or some sort of reflection crawl of the std package, which I believe would cause everything to be semantically analyzed.
  2. What about arbitrary user-defined types? We could add a declaration for them, but what about anonymous types? Should we require a name for them?
  3. What about imports from other, non-std packages? Is/could this even be allowed in build.zig? If so, can we guarantee that the main project will see a value of the actual type from the aux package? (rather than some structurally identical but separate type)
  4. What about generics? How would we know the proper function to import, and what arguments to pass? Again, the only thing I can think of here is a string check of @typeName (ew), or using reflection to generate a declaration which seems it would break our type equality semantics.

@leecannon
Copy link
Contributor

Due to the way build options currently functions (a seperate package) it cant really handle user types defined within the program itself without the build option package having dependencies on all the packages the LibExeStep depends on.

If it did, having addBuildOption work for known types like Arch, Version, etc but requiring a seperate addBuildOptionExtended which takes a fully qualifed type would allow:

const Builder = @import("std").build.Builder;

const MyStruct = @import("src/lib.zig").MyStruct;

pub fn build(b: *Builder) void {
    const exe = b.addExecutable("foo", "foo.zig");
    exe.addBuildOptionExtended(MyStruct, "option", .{}, "@import(\"root\").MyStruct");
    exe.install();
    b.default_step.dependOn(&exe.step);
}

which would result something like: pub const option: @import("root").MyStruct = .{}; although it should probably result in fully initalizing all fields to be the same as the instance passed to addBuildOptionExtended

@courajs
Copy link
Contributor

courajs commented Oct 23, 2021

Would that run into problems as src/lib.zig is now being compiled twice, with potentially different build configuration? For example when cross-compiling, I assume build.zig is compiled natively, then the build.zig configures the main build to compile for a different target. The types might be different based on that.
Similarly, it wouldn’t have access to any of the other build options - but there’s probably no reasonable way around that

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. standard library This issue involves writing Zig code for the standard library. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Development

No branches or pull requests

4 participants