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

stage2: ensure builtin packages are always available #11867

Merged
merged 2 commits into from Jul 28, 2022

Conversation

nektro
Copy link
Contributor

@nektro nektro commented Jun 14, 2022

after a certain package depth, it appears access to the 'std' package is lost. with this fix building nektro/aquila reaches a new error

[nix-shell:~/other/dev/aquila]$ ~/other/dev/zig/zig-out/bin/zig build --prominent-compile-errors -fno-stage1
/home/meg/other/dev/aquila/.zigmod/deps/git/github.com/nektro/zig-zorm/src/sqlite3.zig:1:21: error: unable to open 'std': PackageNotFound
const std = @import("std");
                    ^
aquila...The step exited with error code 1
error: the build command failed with exit code 1

[nix-shell:~/other/dev/aquila]$ ~/other/dev/zig/zig-out/bin/zig build --prominent-compile-errors -fno-stage1
Semantic Analysis [2456] ensureTotalCapacityPrecise... thread 78535 panic: integer overflow
/home/meg/other/dev/zig/lib/std/mem.zig:3045:54: 0x2dd642a in std.mem.alignForwardGeneric (zig)
    return alignBackwardGeneric(T, addr + (alignment - 1), alignment);
                                                     ^
/home/meg/other/dev/zig/src/codegen/llvm.zig:2651:57: 0x30e8575 in codegen.llvm.DeclGen.lowerType (zig)
                    offset = std.mem.alignForwardGeneric(u64, offset, field_align);
                                                        ^
/home/meg/other/dev/zig/src/codegen/llvm.zig:2376:43: 0x30df435 in codegen.llvm.DeclGen.resolveGlobalDecl (zig)
        const llvm_type = try dg.lowerType(decl.ty);
                                          ^
/home/meg/other/dev/zig/src/codegen/llvm.zig:3740:39: 0x30ef167 in codegen.llvm.DeclGen.lowerDeclRefValue (zig)
            try self.resolveGlobalDecl(decl_index);
                                      ^
....

@ifreund
Copy link
Member

ifreund commented Jun 16, 2022

This doesn't feel like the right fix, why is the std package not found "after a certain package depth"?

@Vexu
Copy link
Member

Vexu commented Jun 17, 2022

std and builtin are only added to first level packages in

while (other_pkg_iter.next()) |pkg| {

Either that needs to be made recursive or builtin should also be handled here. Also is root intentionally omitted from other packages?

@ifreund
Copy link
Member

ifreund commented Jun 19, 2022

If I'm not mistaken, this change would make the code Vexu referenced in Compilation.zig unnecessary. It should probably be removed.

@ifreund
Copy link
Member

ifreund commented Jun 20, 2022

Also is root intentionally omitted from other packages?

It's used with the std package, so I think it is intended to be available outside of the root package.

@nektro
Copy link
Contributor Author

nektro commented Jun 21, 2022

just to be sure, 'root' is mod.main_pkg right?

@Vexu Vexu force-pushed the patch-2 branch 2 times, most recently from 20e70d2 to d8db350 Compare July 20, 2022 13:40
@Vexu Vexu requested a review from andrewrk July 20, 2022 13:40
@nektro nektro changed the title stage2: ensure 'std' is always available to @import stage2: ensure builtin packages are always available Jul 26, 2022
@nektro
Copy link
Contributor Author

nektro commented Jul 26, 2022

fail seems unrelated?

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

Let's let the CI run; I can do the typo fix upon merging.

src/main.zig Outdated
@@ -858,6 +858,12 @@ fn buildOutputType(
) catch |err| {
fatal("Failed to add package at path {s}: {s}", .{ pkg_path.?, @errorName(err) });
};

if (mem.eql(u8, pkg_name.?, "std") or mem.eql(u8, pkg_name.?, "root") or mem.eql(u8, pkg_name.?, "builtin")) {
fatal("unable to add package '{s}' -> '{s}': conflicts with builtin pacakge", .{ pkg_name.?, pkg_path.? });
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fatal("unable to add package '{s}' -> '{s}': conflicts with builtin pacakge", .{ pkg_name.?, pkg_path.? });
fatal("unable to add package '{s}' -> '{s}': conflicts with builtin package", .{ pkg_name.?, pkg_path.? });

@andrewrk andrewrk merged commit e6b3eae into ziglang:master Jul 28, 2022
@nektro nektro deleted the patch-2 branch July 28, 2022 22:30
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

4 participants