-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
ability to override packages locally #20180
Comments
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 My solution was to temporarily change the game's 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:
It also wouldn't surprised me if I'm missing something about versioning with the package manager. I am quite new to using it. |
Do I understand it correctly:
Perhaps it would make sense to add additional
Inspired by command
(example is from https://batsov.com/articles/2021/11/14/display-git-configuration/) |
Note that this is possible via something like this:
For non-GitHub URLs the equivalent |
@BratishkaErik Yes you understand correctly. |
Tangential to my post above: This would have a few advantages:
|
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. |
Yes for 99% of cases. If, for some reasons, package:
maintainers can always:
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) |
Note that this will never be the case. The |
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". |
Fair point. I was mostly thinking "I want to do 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. |
Yeah, this is something I've been extremely careful with. I would draw two very different categories here:
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. |
Good distinction of categories. No further notes. |
My interpretation of this is "zig will always treat |
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 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
This is a similar idea to the unused decl error: if you had an override and it doesn't get picked up by |
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. |
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
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 |
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. |
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 One use case is for ephemeral experimenting (override using |
#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:
or
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:
When building in
--system
mode, it ignores local overrides with a warning:zig pkg-hash --list
would list all the immediate dependency packages and their hashes:Then you could drill down into the sub-dependencies:
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:
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:
zig lop --clean
- delete all local overrides with mismatching hasheszig lop --clear
- delete all local overrideszig lop --status
- list all local overrides, their expected hashes, and actual hashes (when mismatching)Related:
The text was updated successfully, but these errors were encountered: