ziglang / zig

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

ability to override packages locally #20180

Open andrewrk opened 5 months ago

andrewrk commented 5 months ago

20178 is not technically a prerequisite for this, but the two proposals would work well together.

It would be nice if you could do something like this:

$ cd my-zig-project
$ git clone git://example.com/zlib/ .zig-cache/p/$(zig pkg-hash zlib)

or

$ cd my-zig-project
$ ln -s $HOME/path-to-local-zlib-checkout/ .zig-cache/p/$(zig pkg-hash zlib)

then you could make edits to .zig-cache/p/<pkg hash> that would override the global packages, as well as make it easy to interact with the upstream source control of the dependency.

Crucially, these directories are typically not tracked by source control, making the workflow convenient to test a patch to a dependency.

When building with local overrides, a warning should be issued:

$ zig build
warning: overriding 'zlib' dependency with './zig-cache/p/zlib-1.3.1-3-IQwAAPXlgi9M'

When building in --system mode, it ignores local overrides with a warning:

$ zig build --system /usr/lib/zig/p
warning: ignoring local override './zig-cache/p/zlib-1.3.1-3-IQwAAPXlgi9M' in system mode

zig pkg-hash --list would list all the immediate dependency packages and their hashes:

$ zig pkg-hash --list
libz    zlib-1.3.1-3-IQwAAPXlgi9M
libvorbis    libvorbis-1.3.8-3-NQ8kAD5eWxrE

Then you could drill down into the sub-dependencies:

$ zig pkg-hash --list libvorbis
libvorbis.libz    zlib-1.3.1-3-IQwAAPXlgi9M

Or maybe --list would just print the whole tree.

Perhaps another subcommand could do the path calculation, so even if you're in a subdirectory, it would give you the local override path:

andy@ark:~/my-project/build$ zig lop libvorbis.libz
../.zig-cache/p/zlib-1.3.1-3-IQwAAPXlgi9M
andy@ark:~/my-project/build$ zig lop libvorbis.libztypo
error: no dependency package named 'libvorbis.libztypo'
info: use `zig pkg-hash --list` to browse dependency tree

This would be a way to quickly find out the command line to pass to e.g. git clone.

Another part of this workflow would be finding out the list of overrides that have mismatching hashes. Perhaps:

Related:

scottredig commented 5 months ago

This last week I've been working on a small game using https://github.com/scottredig/zig-javascript-bridge/ where this would have been helpful, and have some thoughts which may be relevant here:

Firstly, problems started after I did a release of zjb and found that the release was broken. I have examples in the zjb project which build correctly, however it turns out that I improperly exposed an artifact from build.zig. So slightly tangential to this issue, but what is exposed from std.Build.dependency is hard to test from within. (maybe the example should have its own build.zig, despite being in the same git repro?) Doing a release only to realize it was completely broken wasn't great.

My solution was to temporarily change the game's build.zig.zon to reference my local copy of zjb. With this proposal, using a local override would be done instead (so the game commit history doesn't assume my local project organization).

From there when I was using zjb to make a game in practice, I have found several features needed to be added (as I expected would happen). My cycle has been to add the feature to zjb, test it with the game, and when ready commit both.

However this has been small feedback loop. Therefor what I haven't done is gone back to zjb and made a release for every change I've made. Instead I'm waiting to find all of the changes I'm going to find to be in, and then do a single release.

The issue I've found with my approach, and would be an issue with this proposal as well, is this:

When working in a tight iteration loop, where one package is having direct influence on changes to another package, a tradeoff needs to be made: Either the dependency needs to have a semantic version release after every change made to it, or the project which imports the dependency will have a range of commits where history is broken.

I think in a more ideal world, the zig package manager would let me directly reference the desired commit in the dependency. Then the process would be:

  1. Add local override.
  2. Make changes in both the dependency and the project.
  3. Commit dependency.
  4. Update project's zon to reference commit made in 3, and commit project.
  5. Repeat steps 2-4 until no more changes are needed.
  6. Semantic version release of dependency.
  7. Update project to use semantic version and commit.

It also wouldn't surprised me if I'm missing something about versioning with the package manager. I am quite new to using it.

BratishkaErik commented 5 months ago

Do I understand it correctly:

zig pkg-hash --list

Perhaps it would make sense to add additional --source or --origin parameter that will add third "source" column, which will show where package is taken from, I think it will be helpful to troubleshoot these patches. Something like:

libz      zlib-1.3.1-3-IQwAAPXlgi9M         local override
libvorbis libvorbis-1.3.8-3-NQ8kAD5eWxrE    global cache (or system dir if in system mode)

Inspired by command git config --list --show-scope:

system rebase.autosquash=true
system credential.helper=helper-selector
global core.editor=emacs
local core.symlinks=false
local core.ignorecase=true 

(example is from https://batsov.com/articles/2021/11/14/display-git-configuration/)

andrewrk commented 5 months ago

I think in a more ideal world, the zig package manager would let me directly reference the desired commit in the dependency. Then the process would be:

Note that this is possible via something like this:

            .url = "https://github.com/scottredig/zig-javascript/bridge/archive/502361981484f80ed11cbb66eee5be496a4aefa8.tar.gz",

For non-GitHub URLs the equivalent git+https:// scheme works and can reference individual commits.

andrewrk commented 5 months ago

@BratishkaErik Yes you understand correctly. --system <dir> provides <dir> as the one and only location where packages are used. Do I understand correctly that this is generally the desired behavior from Linux distro maintainers?

scottredig commented 5 months ago

Tangential to my post above: I would prefer that instead of .zig-cache/p/$(zig pkg-hash zlib) containing the dependency, .zig-cache/p/$(zig pkg-hash zlib).txt contain a path to a local copy of the dependency. (txt extension irrelevant to proposal) The file contents being only the file path with no other formatting would be the simplest.

This would have a few advantages:

  1. No need to perform a local clone if you already have the project.
  2. Any setup the user has for working with the repo (eg, editor having project local settings) can be used instead needing to do setup again.
  3. Works better when you're primarily doing work on the dependency, but want to quickly test a project which uses the dependency /before/ committing.
  4. Multiple projects using the dependency can be testing in one go.
andrewrk commented 5 months ago

A text file containing only a path is extremely similar to a symlink. Why reinvent symlinks? All 4 of your points work the same with actual symlinks.

BratishkaErik commented 5 months ago

@BratishkaErik Yes you understand correctly. --system <dir> provides <dir> as the one and only location where packages are used. Do I understand correctly that this is generally the desired behavior from Linux distro maintainers?

Yes for 99% of cases. If, for some reasons, package:

  1. has ".zig-cache" dir with override added to source,
  2. and the patches are package-dependent,
  3. and the patch is too intrusive to be applied to dependency supplied by distro before building this package,

maintainers can always:

  1. Copy system dir to another location and replace dependency with patched version here,
  2. or introduce another package like libfoobar-downstream_name-patched and use it as a dependency in their package manager.

I think situation where all three conditions are met will be extremely rare. "1-st + 2-nd condition only" should be more often, but in this case it's likely this patch should be upstreamed, for benefits of each party (distros, upstream and downstream)

andrewrk commented 5 months ago

has ".zig-cache" dir with override added to source,

Note that this will never be the case. The .zig-cache directory must be excluded from source control, and must not be included in a package.

BratishkaErik commented 5 months ago

has ".zig-cache" dir with override added to source,

Note that this will never be the case. The .zig-cache directory must be excluded from source control, and must not be included in a package.

Must as in "Zig must return error if this will be the case" or must as in "Authors must refrain from this, but no hard checks"? Because if this is the second, chances are very small but still not zero, and downstream still has opportunity to commit specifically ".zig-cache/p/dep-1.2.3-sizedhash" folder and ".gitignore" everything else from ".zig-cache". Maybe most distros will have policy to reject these packages, but I can't find out in this proposal if Zig will also check ".gitignore".

scottredig commented 5 months ago

A text file containing only a path is extremely similar to a symlink. Why reinvent symlinks? All 4 of your points work the same with actual symlinks.

Fair point. I was mostly thinking "I want to do .path = "../zig-javascript-bridge/", in build.zig.zon, but just locally and with a warning."

A file containing the path would be consistent on different OSs, which would be an advantage for any tooling.

There is some friction on Windows worth considering:

Which is all pretty stupid (on par for Windows). All that said, if the proposal goes through as is, it's nothing I wouldn't just deal with.

andrewrk commented 5 months ago
  • It requires admin or switching the computer into dev mode (which requires admin). I don't think anything else I've done with Zig has required admin.

Yeah, this is something I've been extremely careful with. I would draw two very different categories here:

  1. Depending on symlinks in order for zig to work, or even for some optional tooling to work. I veto this every time someone proposes it or makes a pull request that accidentally does this.
  2. Creating general-purpose tooling that operates correctly with symlinks when they are used. This is the category we are in. Notice that to take advantage of this feature, you would not need symlinks. You could copy-paste folders instead, for example, or just have an extra version control checkout. The zig process isn't ever the party that would create a symlink. But if you wanted symlinks - which it seems like you are at least subconsciously expressing 2 comments above - in your workflow, then you can add them to your workflow, by modifying your development environment to support that desired workflow.

Final note, for the Windows use case specifically it would be nice to see some IDE integrations, for example, a UI that lets you quickly select a dependency and start patching it locally using this mechanism.

scottredig commented 5 months ago

Good distinction of categories. No further notes.

mnemnion commented 5 months ago

has ".zig-cache" dir with override added to source,

Note that this will never be the case. The .zig-cache directory must be excluded from source control, and must not be included in a package.

Must as in "Zig must return error if this will be the case" or must as in "Authors must refrain from this, but no hard checks"?

My interpretation of this is "zig will always treat .zig_cache as ephemeral, and will never exhibit behavior which requires the cache to be in source control, for any reason", for what that's worth.

dweiller commented 4 months ago

One issue that can be encountered when using local overrides is that a dependency gets updated, but you forget to update the path of a local override so it gets ignored as it no longer matches the hash in build.zig.zon. I'm not sure how much of an issue this will be, but I did manage to do this already while using #20348. I haven't been able to come up with a reliable way to handle this so I'd be interested if anyone has some ideas.

Since subdependencies could have name clashes (and infact upgrading a dependency could cause the sub-dep name to change), I don't think there is a way to have a local override work across an upgrade without manually updating the path of the override as we can't rely on the name of a (sub)dependency.

I think the best we can do is have an warning if a directory in .zig-cache/p/ is named like a valid hash but does not get used as a local override. Something like

$ zig build
warning: unused local override .zig-cache/p/zlib-1.3.1-3-IQwAAPXlgi9M

This is a similar idea to the unused decl error: if you had an override and it doesn't get picked up by zig build something is mostly likely wrong and you either want to delete the override or rename it to a new hash.

buzmeg commented 2 months ago

Would this all not be better specified in the build.zig.zon as something like ".dependencies_override"? That way you could specify both the dependency being overridden and its source with hash as well as the substitute dependency source and its own hash.

This would duck the whole symlink vs. directory issue as you can specify the source by reusing the mechanisms already present in the build.zig.zon. It would also be something you could check the presence of for various build types as well as capture the intent of the builder.

I'm not particularly wedded to having this in the build.zig.zon. I'm happy to have any solution for this. I just suspect that tinkering with .zig-cache isn't going to be sufficient when used in container or CI/CD scenarios where people don't necessarily have access to the underlying filesystem.

If people start using more dependencies and those chains get deeper, it's also inevitable that some of those dependencies will be broken and we need some way to help them out. Generally I bump into this when I upgrade a Zig version where the compilation of some packaged and wrapped C library breaks even though everything down to it is perfectly fine.

dweiller commented 2 months ago

Would this all not be better specified in the build.zig.zon as something like ".dependencies_override"? That way you could specify both the dependency being overridden and its source with hash as well as the substitute dependency source and its own hash.

As I see it, this defeats the purpose of having a local override. The point is that it is local and not something that makes it into version control. If your project needs to depend on a different version of a package you just change the package used in the build.zig.zon, possibly to a vendored fork that lives in your source tree.

I just suspect that tinkering with .zig-cache isn't going to be sufficient when used in container or CI/CD scenarios where people don't necessarily have access to the underlying filesystem.

If people start using more dependencies and those chains get deeper, it's also inevitable that some of those dependencies will be broken and we need some way to help them out. Generally I bump into this when I upgrade a Zig version where the compilation of some packaged and wrapped C library breaks even though everything down to it is perfectly fine.

The idea is not to have these overrides be something that makes it to CI/CD or even be used in any version of a project, but rather to ease development workflow when you want to patch dependencies and check that those changes will work with your project. You would then either upstream those changes to the dependency, or fork/vendor the dependency and update your build.zig.zon to point the fork.

buzmeg commented 2 months ago

The idea is not to have these overrides be something that makes it to CI/CD or even be used in any version of a project

Then what is the solution when you do want it to be something that goes into source control?

Upstream dependencies may be on cadence that doesn't match yours (6 or 12 months, for example). Without an override, if you have two deep dependencies that, for example, insist on specific but differing versions of Zig, you're stuck making forks of every single repository involved in the chain due to the fact that the transitive hashes will mismatch once you make a fix at the bottom. With a ".dependencies_override" you only need to fork and fix one repository rather than the whole chain.

Dependencies break and have bugs. That's just life, and we can't control it. What we can control is how hard it is to fix the problem when it occurs.

This probably isn't a big deal for the moment since Zig doesn't have that many projects with long dependency chains. However, if we want a robust ecosystem (and I assume we do!), this is going to be something that needs to be addressed.

This isn't theoretical. I just tripped over it. In my instance, I simply excised the chain and went back to importing the C directly. It's unfortunate as I lost the convenience of the Zig wrappers but now I don't have to worry about the dependency chain at all. This isn't pique--the library was only being passively maintained so removing it is probably the right call.

However, if I had an easy ability to override a single module, I probably wouldn't have spent the work to remove it. So, not having an override actually means that it was less work to wipe out the dependency rather than maintaining the connection which might result in upstream contributions.

It's an unusual problem. And I think it's the fact that we are matching "hashes" (this very specific version) rather than "semver" (this version or newer) that exacerbates the issue.

andrewrk commented 2 months ago

Then what is the solution when you do want it to be something that goes into source control?

I thought there was an issue open for this but I didn't find it when I went looking just now, so I'll open one soon. But this use case will be solved with a way to explicitly override any package in the entire dependency tree using build.zig.zon configuration.

One use case is for ephemeral experimenting (override using .zig-cache file system paths), the other use case is for committing the overrides to source control (override using build.zig.zon configuration).