Incremental fixes, refactor Zcu.File

[mlugg]

See commit messages.

There is one breaking change here, which is that @import("non_existent_module") is now a compile error even if the import is never analyzed. This is going to fail CI at the moment, because Aro currently relies on the old behavior of accepting these imports.

I've reverted that breaking change, at least for now. There is still one very minor breaking change, which is that the contract of std.Build.Cache.Manifest.addFilePostContents has changed.

[NicoElbers]

In one of my project I currently use non existant modules to have optional depencencies. Simplified slightly below:

// build.zig
const use_jemalloc = b.option(bool, "use_jemalloc", "test jemalloc") orelse false;

const exe_opts = b.addOptions();
exe_opts.addOption(bool, "use_jemalloc", use_jemalloc);

const exe_mod = b.createModule(.{
    // ...
});

exe_mod.addOptions("build_opts", exe_opts);

// the jemalloc source code may not be downloaded, or maybe exe_mod wants to stay libc free
if (use_jemalloc) {
    const jemalloc_mod = b.createModule(.{
        // ...
    });

    exe_mod.addImport("use_jemalloc", jemalloc_mod);
}

// main.zig
const functions = [_]Contructor{} ++
    if (build_opts.use_jemalloc) @import("jemalloc").constructor else .{};

const build_opts = @import("build_opts");

// ...

If I understand correctly, after this PR this code will no longer compile when use_jemalloc is false, as jemalloc_mod is never created. Is there a proper way to do optional dependencies? or am I doing something too weird.

[mlugg]

In that case, I'd suggest just adding the module in the build script regardless of use_jemalloc. It doesn't do any harm; the build option will still stop it from actually being analyzed.

There could be other cases I've not thought of where this is more of a problem, though. If anyone has any, feel free to ask here, and we can see if this language change is problematic.

[NicoElbers]

It may be useful to label this PR as breaking, so more people are inclined to check it out.

[vesim987]

@mlugg

There could be other cases I've not thought of where this is more of a problem, though. If anyone has any, feel free to ask here, and we can see if this language change is problematic.

Whats about bigger dependencies like zigwin32, or packages that cannot be extracted on systems with case-insensitive fs?

[dweiller]

In one of my project I currently use non existant modules to have optional depencencies. Simplified slightly below:

// build.zig
const use_jemalloc = b.option(bool, "use_jemalloc", "test jemalloc") orelse false;

const exe_opts = b.addOptions();
exe_opts.addOption(bool, "use_jemalloc", use_jemalloc);

const exe_mod = b.createModule(.{
    // ...
});

exe_mod.addOptions("build_opts", exe_opts);

// the jemalloc source code may not be downloaded, or maybe exe_mod wants to stay libc free
if (use_jemalloc) {
    const jemalloc_mod = b.createModule(.{
        // ...
    });

    exe_mod.addImport("use_jemalloc", jemalloc_mod);
}

// main.zig
const functions = [_]Contructor{} ++
    if (build_opts.use_jemalloc) @import("jemalloc").constructor else .{};

const build_opts = @import("build_opts");

// ...

If I understand correctly, after this PR this code will no longer compile when use_jemalloc is false, as jemalloc_mod is never created. Is there a proper way to do optional dependencies? or am I doing something too weird.

Does the compiler actually analyse that @import? I would have assumed that it never checks whether jemalloc in the import table (when it's not) since build_opts.use_jemalloc is comptime known.

[mlugg]

Whats about bigger dependencies like zigwin32

Optional lazy dependencies seems like a compelling argument in favor of status quo to me.

@andrewrk, what do you think? The question is about something like this:

const foo = @import("foo");
const build_options = @import("build_options");

pub fn main() void {
    if (build_options.use_foo) {
        foo.doStuff();
    }
}

...where the module foo comes from a package in your build.zig.zon which is declared with .lazy = true and only fetched if you pass -Duse-foo (which also controls build_options.use_foo). Right now, on master, this works; on this branch it is a compile error, because the @import("foo") is of a non-existent module when you don't pass -Duse-foo.

I think this clearly needs to be a supported use case, so I see two options:

• Revert to status-quo behavior, where "module not found" errors only occur if an import is analyzed
• Require people using this pattern to add a reference to an "empty" module (this may require extending the build system to make it more ergonomic

Personally I'm in favour of option 1 (making this PR non-breaking); while it's unfortunate that you could end up with unused imports of non-existent modules, option 2 feels like quite a bit of friction to make this work well. It also makes the error for referencing foo when !build_options.use_foo more confusing (e.g. it'd be root source file struct 'foo' has no member named 'doStuff' rather than no module named 'foo' available within module 'root').

[mlugg]

I've reverted the breaking change discussed above; I think it did more harm than good. At the very least, it can go through the proposal process.

There is still one very minor breaking change here (I honestly don't think anyone will hit it), which is that the contract of std.Build.Cache.Manifest.addFilePostContents has changed slightly.