Skip to content

package manager: transitive dependencies must be listed in the top-level build.zig.zon is painful #17135

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

Closed
emidoots opened this issue Sep 13, 2023 · 1 comment · Fixed by #17164
Labels
bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@emidoots
Copy link

emidoots commented Sep 13, 2023

Zig Version

0.12.0-dev.21+ac95cfe44

Steps to Reproduce and Observed Behavior

Use any package that has transitive dependencies via the package manager.


Mach, mach-core, and mach-glfw all have a handful or two of dependencies. They're logical bits of code that we've extracted into independent Zig packages, actually useful on their own, not an npm-encroaching situation. This should be good for us and the ecosystem as a whole.

But today when users import e.g. mach-glfw via the package manager following our guide, we have to tell them to make sure they meticulously copy all the dependencies from our build.zig.zon file into their own, and keep those up-to-date / correct with what we expect. This is because the Zig package manager today requires all transitive dependencies be specified in the build.zig.zon file.

Whether this is an intentional design decision or not is unclear to me.

But it is very tedious, error prone, and annoying for our users to have to manually copy this informationn over to their own build.zig.zon file. I am regularly seeing 1-3 people per week file issues and ask questions that ultimately boil down to 'I copied the dependencies wrong' or 'I didn't know I had to copy them'. We've added docs around this, created starter projects, etc. to try and reduce the annoyance on our side - but it's still painful overall for our users.

This issue is two questions:

  1. Is this an intentional design decision, intended to e.g. encourage less dependencies?
  2. Is an easier way to add transitive dependencies to your build.zig.zon in scope?

Without at least #2 solved, I think many package authors will be encouraged to reduce dependencies not by actually having less of them.. but rather by 'shoving multiple dependencies into one' and pretending it's one

Expected Behavior

1 & 2 above.

@emidoots emidoots added the bug Observed behavior contradicts documented or intended behavior label Sep 13, 2023
@mlugg
Copy link
Member

mlugg commented Sep 13, 2023

Is this an intentional design decision?

No. (so Q2 not relevant)

Are you sure you're not just hitting the actual bug which ended up existing in #16354?

mlugg added a commit to mlugg/zig that referenced this issue Sep 15, 2023
…cept

The new `@depedencies` module contains generated code like the
following:

```zig
pub const root_deps = [_]struct { []const u8, []const u8 }{
    .{ "foo", "abc123" },
};

pub const packages = struct {
    pub const abc123 = struct {
        pub const build_root = "/home/mlugg/.cache/zig/blah/abc123";
        pub const build_zig = @import("abc123");
        pub const deps = [_]struct { []const u8, []const u8 }{
            .{ "bar", "abc123" },
            .{ "name", "ghi789" },
        };
    };
};
```

Each package contains a build root string, the build.zig import, and a
mapping from dependency names to package hashes. There is also such a
mapping for the root package dependencies.

Resolves: ziglang#16354
Resolves: ziglang#17135
mlugg added a commit to mlugg/zig that referenced this issue Sep 15, 2023
…cept

The new `@depedencies` module contains generated code like the
following (where strings like "abc123" represent hashes):

```zig
pub const root_deps = [_]struct { []const u8, []const u8 }{
    .{ "foo", "abc123" },
};

pub const packages = struct {
    pub const abc123 = struct {
        pub const build_root = "/home/mlugg/.cache/zig/blah/abc123";
        pub const build_zig = @import("abc123");
        pub const deps = [_]struct { []const u8, []const u8 }{
            .{ "bar", "abc123" },
            .{ "name", "ghi789" },
        };
    };
};
```

Each package contains a build root string, the build.zig import, and a
mapping from dependency names to package hashes. There is also such a
mapping for the root package dependencies.

In theory, we could now remove the `dep_prefix` field from `std.Build`,
since its main purpose is now handled differently. I believe this is a
desirable goal, as it doesn't really make sense to assign a single FQN
to any package (because it may appear in many different places in the
package hierarchy). This commit does not remove that field, as it's used
non-trivially in a few places in the build runner and compiler tests:
this will be a future enhancement.

Resolves: ziglang#16354
Resolves: ziglang#17135
mlugg added a commit to mlugg/zig that referenced this issue Sep 15, 2023
…cept

The new `@depedencies` module contains generated code like the
following (where strings like "abc123" represent hashes):

```zig
pub const root_deps = [_]struct { []const u8, []const u8 }{
    .{ "foo", "abc123" },
};

pub const packages = struct {
    pub const abc123 = struct {
        pub const build_root = "/home/mlugg/.cache/zig/blah/abc123";
        pub const build_zig = @import("abc123");
        pub const deps = [_]struct { []const u8, []const u8 }{
            .{ "bar", "abc123" },
            .{ "name", "ghi789" },
        };
    };
};
```

Each package contains a build root string, the build.zig import, and a
mapping from dependency names to package hashes. There is also such a
mapping for the root package dependencies.

In theory, we could now remove the `dep_prefix` field from `std.Build`,
since its main purpose is now handled differently. I believe this is a
desirable goal, as it doesn't really make sense to assign a single FQN
to any package (because it may appear in many different places in the
package hierarchy). This commit does not remove that field, as it's used
non-trivially in a few places in the build runner and compiler tests:
this will be a future enhancement.

Resolves: ziglang#16354
Resolves: ziglang#17135
mlugg added a commit to mlugg/zig that referenced this issue Sep 15, 2023
…cept

The new `@depedencies` module contains generated code like the
following (where strings like "abc123" represent hashes):

```zig
pub const root_deps = [_]struct { []const u8, []const u8 }{
    .{ "foo", "abc123" },
};

pub const packages = struct {
    pub const abc123 = struct {
        pub const build_root = "/home/mlugg/.cache/zig/blah/abc123";
        pub const build_zig = @import("abc123");
        pub const deps = [_]struct { []const u8, []const u8 }{
            .{ "bar", "abc123" },
            .{ "name", "ghi789" },
        };
    };
};
```

Each package contains a build root string, the build.zig import, and a
mapping from dependency names to package hashes. There is also such a
mapping for the root package dependencies.

In theory, we could now remove the `dep_prefix` field from `std.Build`,
since its main purpose is now handled differently. I believe this is a
desirable goal, as it doesn't really make sense to assign a single FQN
to any package (because it may appear in many different places in the
package hierarchy). This commit does not remove that field, as it's used
non-trivially in a few places in the build runner and compiler tests:
this will be a future enhancement.

Resolves: ziglang#16354
Resolves: ziglang#17135
andrewrk pushed a commit that referenced this issue Sep 15, 2023
…cept

The new `@depedencies` module contains generated code like the
following (where strings like "abc123" represent hashes):

```zig
pub const root_deps = [_]struct { []const u8, []const u8 }{
    .{ "foo", "abc123" },
};

pub const packages = struct {
    pub const abc123 = struct {
        pub const build_root = "/home/mlugg/.cache/zig/blah/abc123";
        pub const build_zig = @import("abc123");
        pub const deps = [_]struct { []const u8, []const u8 }{
            .{ "bar", "abc123" },
            .{ "name", "ghi789" },
        };
    };
};
```

Each package contains a build root string, the build.zig import, and a
mapping from dependency names to package hashes. There is also such a
mapping for the root package dependencies.

In theory, we could now remove the `dep_prefix` field from `std.Build`,
since its main purpose is now handled differently. I believe this is a
desirable goal, as it doesn't really make sense to assign a single FQN
to any package (because it may appear in many different places in the
package hierarchy). This commit does not remove that field, as it's used
non-trivially in a few places in the build runner and compiler tests:
this will be a future enhancement.

Resolves: #16354
Resolves: #17135
@andrewrk andrewrk added this to the 0.12.0 milestone Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants