-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
Rust Application Migration to fetchCargoTarball Implementation #79975
Comments
The readme was nice to discuss in the implementation PR, but now that this is merged it's better to have an issue that can be linked against in PRs and doesn't require further merges to update status. Ported with a status update in NixOS#79975
Changes the default fetcher in the Rust Platform to be the newer `fetchCargoTarball`, and changes every application using the current default to instead opt out. This commit does not change any hashes or cause any rebuilds. Once integrated, we will start deleting the opt-outs and recomputing hashes. See NixOS#79975 for details.
Changes the default fetcher in the Rust Platform to be the newer `fetchCargoTarball`, and changes every application using the current default to instead opt out. This commit does not change any hashes or cause any rebuilds. Once integrated, we will start deleting the opt-outs and recomputing hashes. See #79975 for details.
This introduced a bug in the legacy fetcher, as found by @Ma27 : previously, it had a step where it would explicitly run Unfortunately, some Rust applications will run a custom build hook in cargo that writes to the vendor directory; these packages will fail without one of the following two fixes:
|
Includes some bugfixes/cleanups to the scripts and packaging, a run of the updater, a bump of the version, an upgrade to the newer cargo fetcher in NixOS#79975, and gets the web assembly portion to compile successfully. Fixes NixOS#75863
As mentioned in NixOS#79975, the default on `legacyCargoFetcher` if left unspecified is now `false`.
See inline comment and NixOS#79975 for details.
Resolves NixOS#80117 by using the newer fetcher; see NixOS#79975 for details. Would also be fixed by NixOS#80153 eventually, but we want to upgrade Rust packages either way, so might as well start with the broken ones!
Infra upgrade as part of NixOS#79975; no functional change expected.
For anyone who's looking at bumping a package, no need to recompute the #!/usr/bin/env zsh
set -euo pipefail
# Helper script for migrating a NixPkgs Rust pkg to the new cargoSha256
# verification. Run from the root of a NixPkgs git checkout.
if [ -z "$1" ]; then
echo "USAGE: $0 <attribute>"
echo "EXAMPLE: $0 ripgrep"
exit 1
fi
ATTR=$1
FNAME=$(EDITOR=ls nix edit -f . $ATTR)
section() {
echo "********************************************************************************"
echo "$ATTR: $1"
echo "********************************************************************************"
}
main() {
sed -i '/.*Delete this on next update.*/,/^$/d' $FNAME
section "Nuking cargoSha256 reference for $FNAME, then rebuilding"
sed -i 's|cargoSha256.*|cargoSha256 = "0000000000000000000000000000000000000000000000000000";|' $FNAME;
nix-build -A $ATTR 2>&1 | tee /tmp/nix-rust-logfile-$ATTR || true
actual=$(grep 'got:.*sha256:.*' /tmp/nix-rust-logfile-$ATTR | cut -d':' -f3-)
echo "Build of $ATTR determined that cargoSha256 should be $actual"
sed -i "s|cargoSha256.*|cargoSha256 = \"$actual\";|" $FNAME
section "Rebuilding with updated hash, expecting a pass:"
nix-build -A $ATTR || exit 1
section "Finished successfully!"
}
main |
Currently broken; see NixOS#79975 for details. Would also be fixed by NixOS#80153 eventually, but since we want to upgrade either way we might as well do so now.
This issue has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/steps-towards-even-more-pr-automation/5634/11 |
The attribute `verifyCargoDeps` is no longer defined in the rustPlatform helper; it is now the default and always on as part of the improvements in NixOS#79975
The attribute `verifyCargoDeps` is no longer defined in the rustPlatform helper; it is now the default and always on as part of the improvements in #79975
Infra upgrade as part of NixOS#79975; no functional change expected.
There are just three uses of the legacy fetcher left:
If someone could help with reviewing/merging, that'd be great. Once those three are gone, we can proceed with the deletion of the legacy infra and complete this ticket! |
We have now migrated every single Rust package in NixPkgs! This deletes the legacy fetcher, which is now unused. Resolves NixOS#79975
We have now migrated every single Rust package in NixPkgs! This deletes the legacy fetcher, which is now unused. Resolves #79975
The cargo hash differed from the cherry-picked one due to changes to fetchCargoTarball on the master branch NixOS#79975 On the master this happened here: eb11fea 1f03a34 This should not effect the actual build result.
The cargo hash differed from the cherry-picked one due to changes to fetchCargoTarball on the master branch NixOS#79975 On the master this happened here: eb11fea 1f03a34 This should not effect the actual build result.
What
We are making general infrastructure improvements to the cargo fetcher used to
produce the vendor package in Rust applications. This should not materially
change behavior, but will change the
cargoSha256
, so this is being rolled outcarefully to avoid disruption and merge conflicts.
Motivation
Currently
fetchcargo
will pull all of the cargo vendor deps into aattr.cargoDeps
package with a recursive sha256 hash. We are migrating to a newimplementation that will compress the cargoDeps into a tar.gz archive that gets
unpacked at the start of the build. This has been implemented and merged in
#78501
The new implementation has several advantages:
It takes up significantly less space on disk in-between builds in the nix store.
It plays nicely with hashed mirrors like tarballs.nixos.org, which only
substitute --flat hashes on single files (not recursive directory hashes).
This is important for internal reproducibility and air-gapped builders.
It's significantly faster with distributed builders, because in that scenario
the actual full src package is copied back and forth (unlike with a binary cache,
where it's compressed). This can be very slow.
It's consistent with how simple fetchurl src derivations work.
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. Moreover, in the
store it's harder to get confused with something that is just there as a
build-time dependency directory vs. an actual runtime package directory,
since the build-time src dependencies are flat tarballs.
It ensures that if a
patchPhase
makes modifications to thesrc
, thebuild/install must reference the patched sources in the build directory,
rather than the original input src package. This prevents issues like fzf: fix patch for vim plugin; enable tests; avoid direct $src layout dependency #79575
Additional Improvements
Changes to the
fetchcargo.nix
behavior that cause changes to thecargoSha256
are somewhat disruptive, so historically we've added conditionals to provide
backwards compatibility. We've now accumulated enough of these that it makes
sense to do a clean sweep updating hashes, and delete the conditionals in the
fetcher to simplify maintenance and implementation complexity. These
conditionals are:
When cargo vendors dependencies, it generates a config. Previously, we were
hard-coding our own config, but this fails if there are git dependencies. We
have conditional logic to sometimes copy the vendored cargo config in, and
sometimes not.
When users update the src package, they may forget to update the
cargoSha256
. We have an opt-in conditional flag to add theCargo.lock
into the vendor dir for inspection and compare at build-time, which catches
this mistake; but it defaults to false.
We will bring both of these features in as defaults in the new
fetchCargoTarball
implementation.Migration Plan and Current Status
(DONE in fetchcargo: use flat tar.gz file for vendored src instead of recursive hash dir #78501) Implement
fetchCargoTarball
as a separate, clean fetcherimplementation along-side
fetchcargo
. RenameverifyCargoDeps
(defaultfalse) to
legacyCargoFetcher
(default true), which switches the fetcherimplementation used. Replace
verifyCargoDeps = true;
withlegacyCargoFetcher = false;
in Rust applications.(DONE in treewide: change fetchCargoTarball default to opt-out #79978) Send a treewide Rust PR that sets
legacyCargoFetcher = true;
in all Rustapplications not using this (which is ~200 of them), with a note to
maintainers to delete if updating the package. Change the default in
buildRustPackage
to false.(ABANDONED) Backport both (1) and (2) to 20.03 so that other Rust backports can be done without issue. Note: per discussion below, we're not going to do this, and instead will just have maintainers re-compute the cargoSha256 when backporting Rust applications to 20.03. This will give it time to incubate and then sweep for 20.09.
(DONE) Go through all Rust src packages deleting the
legacyCargoFetcher = false;
line and re-computing the
cargoSha256
, merging as we go.(DONE in rust: remove legacy cargo fetcher #82901) Delete the
fetchcargo.nix
implementation entirely and all conditionals around it. Update the manual accordingly.The text was updated successfully, but these errors were encountered: