ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.37k stars 2.51k forks source link

package manager: prevent confusing copy+paste error with package `.hash` entries #16679

Open slimsag opened 1 year ago

slimsag commented 1 year ago

Zig Version

0.11.0

Steps to Reproduce and Observed Behavior

  1. Start with any existing Zig project that uses the package manager (you must have ran zig build previously, as in normal development)
  2. Go to add a new dependency, and copy+paste a section of build.zig.zon like this:
        .mach_earcut = .{
            .url = "https://pkg.machengine.org/mach-earcut/fa2d4962181d812960838435f95916c95f7c9132.tar.gz",
            .hash = "1220da97fa7b8ffb9576aa3e2346b5e82d93f3d1fa67f30cc5a5afe4b401008d40c7",
        },

to create your new entry:

+        .mach_newdep = .{
+            .url = "https://pkg.machengine.org/mach-newdep/ed8129635f9592d4962181d16c95f7c913208384.tar.gz",
             .hash = "1220da97fa7b8ffb9576aa3e2346b5e82d93f3d1fa67f30cc5a5afe4b401008d40c7",
+        },

If you forgot to delete the .hash, or otherwise invalidate it, then zig build will not complain about the hash being wrong, but rather you'll get very confusing errors because .mach_newdep will actually be pointing to .mach_earcut (the hash you copied) in your ~/.cache/zig/p folder.

This seems like a footgun that many users will encounter, I've personally had ~5 people run into this. Because the hashes in the compiler errors obfuscate which dependency actually lives inside that cache directory, it can lead to some very confusing error messages (e.g. b.dependency reporting an artifact doesn't exist.

Expected Behavior

Some way to avoid the copy+paste footgun here.

mitchellh commented 1 year ago

I've also experienced this. I completely understand why this is happening, especially as a Nix user I really get it. But I agree that it does lead to some head scratching periodically before saying "oh yeah, duh."

rohlem commented 1 year ago

Might not be worth it, but a crazy idea to fix this would be to invert the hierarchy to mirror our actual caching strategy (so the top-level entry becomes the hash instead of the name):

 .@"1220da97fa7b8ffb9576aa3e2346b5e82d93f3d1fa67f30cc5a5afe4b401008d40c7" = .{
    .name = "mach_earcut",
    .url = "https://pkg.machengine.org/mach-earcut/fa2d4962181d812960838435f95916c95f7c9132.tar.gz",
},

Then the .zon parser can complain on duplicate keys (I assume it does? it definitely should), and the error and fix would be relatively understandable/obvious to users. (I guess generally speaking my idea was that exposing the underlying structure to the users would help them understand/track down unexpected behavior. As-is it's not obvious to me that a field "hash" is essentially used as sole identification criterion, for both identity+version of a package.)

mitchellh commented 1 year ago

Then the .zon parser can complain on duplicate keys (I assume it does? it definitely should), and the error and fix would be blatantly obvious to users.

The parser could just as easily complain about duplicate hashes in a single zon file without it being the keys

rohlem commented 1 year ago

The parser could just as easily complain about duplicate hashes

I assume that would be after parsing, but the build system could complain, that's true.

drujensen commented 1 year ago

I'm new to zig and I'm a bit sad the packages are not installed in the project like a libs or deps directory instead of the ~/.cache/zig directory.

This would remove the need for hashes and allow you to quickly view the dependency code in your editor instead of trying to find the code in a cached hash directory. It also makes it dead simple to cleanup and pull down the latest dependencies by removing this local directory. It also prevents collisions with other zig projects since the deps are locally cached instead of using a shared cache.

Crystal language package manager does this and it makes development so much easier. It also supports pulling from github using tags or branches. You can add the libs or deps directory in a .gitignore to avoid conflicts with git.

marler8997 commented 1 year ago

The solution I propose is to maintain a set of "verified url/hash pairings" somewhere inside zig's global cache. Zig doesn't use any url hash until after it's been verified locally. When zig encounters a new URL/hash pair, it verifies the pairing is correct before adding it to the "trusted store". This allows zig to detect erroneous mismatches and report them to the user. Breaking zig's URL/hash correctness would now require mucking with Zig's internal trusted store instead of making an innocent mistake modifying build.zig.zon. This maintains the benefit we get from relying on "trusted hashes" (no need for network access to verify URL contents) and avoids the footgun.

karlseguin commented 9 months ago

A couple examples of users running into this issue:

https://github.com/karlseguin/http.zig/issues/29

https://github.com/karlseguin/pg.zig/issues/4#issuecomment-1879746313