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
fetchCargoTarball: backport to 20.03 #79981
fetchCargoTarball: backport to 20.03 #79981
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. We should have that.
Looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of my league here as far as knowledge of proceedings, but it seems fine to me.
I'll defer merge to those who can do such in an informed manner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before we backport this to a release branch, can you please check out if/why this patch breaks custom build commands for crates in the buildPhase
? (Example: https://hydra.nixos.org/build/112753750, already fixed in ce84052 by using the new fetcher).
Ugh, thanks for the catch (and fix on spotifyd). The systemic fix unfortunately requires another rebuild-the-world for all the legacy Rust fetches, so I sent the PR with the fix to staging. Fortunately, it's the legacy implementation that's broken, and the new one that works, so a "workaround" is to do the upgrade work . . . details here: #79975 (comment) Is there a way to filer Hydra on master to show all failing Rust packages? |
Please use |
…e hash dir This has several advantages: 1. It takes up less space on disk in-between builds in the nix store. 2. It uses less space in the binary cache for vendor derivation packages. 3. It uses less network traffic downloading from the binary cache. 4. It plays nicely with hashed mirrors like tarballs.nixos.org, which only substitute --flat hashes on single files (not recursive directory hashes). 5. It's consistent with how simple `fetchurl` src derivations work. 6. It provides a stronger abstraction between input src-package and output package, e.g., it's harder to accidentally depend on the src derivation at runtime by referencing something like `${src}/etc/index.html`. Likewise, in the store it's harder to get confused with something that is just there as a build-time dependency vs. a runtime dependency, since the build-time src dependencies are tarred up. Disadvantages are: 1. It takes slightly longer to untar at the start of a build. As currently implemented, this attaches the compacted vendor.tar.gz feature as a rider on `verifyCargoDeps`, since both of them are relatively newly implemented behavior that change the `cargoSha256`. If this PR is accepted, I will push forward the remaining rust packages with a series of treewide PRs to update the `cargoSha256`s. (cherry picked from commit 2115a20)
Cherry-picked from PR NixOS#80153 (cherry picked from commit 4f8921bd7fa3744a8c2b9cfa475fb102e53d0230)
4f8921b
to
faa46f4
Compare
Aha, I was wondering how you got the nice (cherry-picked from ...) metadata; noted for the future! Reset to |
did you want to cherry-pick the upgrade fixes as part of this? |
The second commit includes the bugfix at an infra level, which means the application bugfix PRs are unnecessary to backport. For example, this was red on master, but is now green, after the second commit was merged to master: #80219 This means we don't need to backport all of the individual application changes, except when there's some real change that's actually needed (like bumping a version, security fix, etc.). Aside from this PR, the only other change that I'd like to pre-emptively backport is this one, which changes the default cargo fetcher implementation to match master, so that the As with all of these, the only purpose of backporting is to make it easier for maintainers to cherry-pick Rust applications, since otherwise they'd have to re-compute the |
Per suggestion, let's wait until #80262 makes its way through staging and over to master, which fixes custom patches for Cargo.lock files. As it currently stands, the implementation works fine for anything that has a Cargo.lock file out of the gate, but there are ~20 packages in Nix that use custom crate mutations outside of the |
With 20.09 not that far away I'd say no. This is a no-op functionally to the end-user, but it's a rebuild-the-rust-world and a ton of sha changes in the release branch, as well as an API change mid-stable release for those consuming the nix exprs directly. I'd do something like this as a work around:
where |
Rather than |
Thanks. That's effectively what I already did. Just took a little while to
track down this cause.
…On Wed, Jul 22, 2020, 11:00 PM Benjamin Hipple ***@***.***> wrote:
With 20.09 not that far away I'd say no. This is a no-op functionally to
the end-user, but it's a rebuild-the-rust-world and a ton of sha changes in
the release branch, as well as an API change mid-stable release for those
consuming the nix exprs directly.
I'd do something like this as a work around:
cargoSha256 = if <condition> then X else Y;
where <condition> is something like switching on system.stateVersion ==
20.03.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#79981 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACNZYIY3F2S3GGQP4EFTHPDR46RTZANCNFSM4KUJ3E3A>
.
|
Cherry-picked from PR #78501, minus the README.md migration/design doc, which I'm deleting.
This causes no functional change, but does recompile all Rust applications. It's
necessary to get the new fetcher on the 20.03 branch in order to backport
updated Rust packages in the future.
See #79975 for details.
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)