about summary refs log tree commit diff
diff options
context:
space:
mode:
authorBenjamin Hipple <bhipple@protonmail.com>2020-01-12 11:21:23 -0500
committerBenjamin Hipple <bhipple@protonmail.com>2020-02-10 10:17:29 -0500
commit2115a2037cb8337ccb09798c175733f1fc747ee3 (patch)
treeae36678ae1dfb15b8c9cd74229cb369a60ae7fe8
parentd9eb897edd8dfb7c65d8069197e3db5eeb537d69 (diff)
fetchcargo: use flat tar.gz file for vendored src instead of recursive 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.
-rw-r--r--doc/languages-frameworks/rust.section.md21
-rw-r--r--pkgs/applications/editors/hexdino/default.nix4
-rw-r--r--pkgs/applications/version-management/git-and-tools/git-workspace/default.nix4
-rw-r--r--pkgs/build-support/rust/README.md45
-rw-r--r--pkgs/build-support/rust/default.nix41
-rw-r--r--pkgs/build-support/rust/fetchCargoTarball.nix81
-rw-r--r--pkgs/development/compilers/rust/default.nix8
-rw-r--r--pkgs/development/tools/documentation/mdsh/default.nix4
-rw-r--r--pkgs/tools/misc/broot/default.nix4
-rw-r--r--pkgs/tools/misc/wagyu/default.nix4
-rw-r--r--pkgs/tools/package-management/nix-du/default.nix4
-rw-r--r--pkgs/tools/security/fido2luks/default.nix4
-rw-r--r--pkgs/tools/system/tre-command/default.nix4
13 files changed, 190 insertions, 38 deletions
diff --git a/doc/languages-frameworks/rust.section.md b/doc/languages-frameworks/rust.section.md
index 3332dff1eb078..6f0ec7c051445 100644
--- a/doc/languages-frameworks/rust.section.md
+++ b/doc/languages-frameworks/rust.section.md
@@ -37,7 +37,7 @@ rustPlatform.buildRustPackage rec {
   };
 
   cargoSha256 = "17ldqr3asrdcsh4l29m3b5r37r5d0b3npq1lrgjmxb6vlx6a36qh";
-  verifyCargoDeps = true;
+  legacyCargoFetcher = false;
 
   meta = with stdenv.lib; {
     description = "A fast line-oriented regex search tool, similar to ag and ack";
@@ -59,12 +59,19 @@ When the `Cargo.lock`, provided by upstream, is not in sync with the
 added in `cargoPatches` will also be prepended to the patches in `patches` at
 build-time.
 
-When `verifyCargoDeps` is set to `true`, the build will also verify that the
-`cargoSha256` is not out of date by comparing the `Cargo.lock` file in both the
-`cargoDeps` and `src`. Note that this option changes the value of `cargoSha256`
-since it also copies the `Cargo.lock` in it. To avoid breaking
-backward-compatibility this option is not enabled by default but hopefully will
-be in the future.
+Setting `legacyCargoFetcher` to `false` enables the following behavior:
+
+1. The `Cargo.lock` file is copied into the cargo vendor directory.
+2. At buildtime, `buildRustPackage` will ensure that the `src` and `cargoSha256`
+   are consistent. This avoids errors where one but not the other is updated.
+3. The builder will compress the vendored cargo src directory into a tar.gz file
+   for storage after vendoring, and decompress it before the build. This saves
+   disk space and enables hashed mirrors for Rust dependencies.
+
+Note that this option changes the value of `cargoSha256`, so it is currently
+defaulted to `false`. When updating a Rust package, please set it to `true`;
+eventually we will default this to true and update the remaining Rust packages,
+then delete the option from all individual Rust package expressions.
 
 ### Building a crate for a different target
 
diff --git a/pkgs/applications/editors/hexdino/default.nix b/pkgs/applications/editors/hexdino/default.nix
index eee5a69655583..91f048320a723 100644
--- a/pkgs/applications/editors/hexdino/default.nix
+++ b/pkgs/applications/editors/hexdino/default.nix
@@ -11,8 +11,8 @@ rustPlatform.buildRustPackage {
     sha256 = "11mz07735gxqfamjcjjmxya6swlvr1p77sgd377zjcmd6z54gwyf";
   };
 
-  cargoSha256 = "0qa8ypp5a7sf1gic482zh3i6s94w6k6bgmk5ynfvwi7g49ql7c4z";
-  verifyCargoDeps = true;
+  cargoSha256 = "06ghcd4j751mdkzwb88nqwk8la4zdb137y0iqrkpykkfx0as43x3";
+  legacyCargoFetcher = false;
 
   buildInputs = [ ncurses ];
 
diff --git a/pkgs/applications/version-management/git-and-tools/git-workspace/default.nix b/pkgs/applications/version-management/git-and-tools/git-workspace/default.nix
index 2ceac55bf5ca0..61e3e1e3eef0a 100644
--- a/pkgs/applications/version-management/git-and-tools/git-workspace/default.nix
+++ b/pkgs/applications/version-management/git-and-tools/git-workspace/default.nix
@@ -15,9 +15,9 @@ rustPlatform.buildRustPackage rec {
     sha256 = "0pl5z0gx2ypkrgq7vj1cxj5iwj06vcd06x3b3nh0g7w7q7nl8pr4";
   };
 
-  cargoSha256 = "0jbsz7r9n3jcgb9sd6pdjwzjf1b35qpfqw8ba8fjjmzfvs9qn6ld";
+  cargoSha256 = "1z4cb7rcb7ldj16xxynrjh4hg872rj39rbbp0vy15kdp3ifyi466";
 
-  verifyCargoDeps = true;
+  legacyCargoFetcher = false;
 
   buildInputs = with stdenv; lib.optional isDarwin Security;
 
diff --git a/pkgs/build-support/rust/README.md b/pkgs/build-support/rust/README.md
new file mode 100644
index 0000000000000..0e0ddb9648de9
--- /dev/null
+++ b/pkgs/build-support/rust/README.md
@@ -0,0 +1,45 @@
+# Updated fetchCargo behavior
+
+Changes to the `fetchcargo.nix` behavior that cause changes to the `cargoSha256`
+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:
+
+1. 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.
+
+2. When a user updates the src package, they may forget to update the
+   `cargoSha256`. We have an opt-in conditional flag to add the `Cargo.lock`
+   into the vendor dir for inspection and compare at build-time, but it defaults
+   to false.
+
+3. We were previously vendoring into a directory with a recursive hash, but
+   would like to vendor into a compressed tar.gz file instead, for the reasons
+   specified in the git commit message adding this feature.
+
+
+## Migration plan
+
+1. (DONE in this PR) Implement `fetchCargoTarball` as a separate, clean fetcher
+   implementation along-side `fetchcargo`. Rename `verifyCargoDeps` (default
+   false) to `legacyCargoFetcher` (default true), which switches the fetcher
+   implementation used. Replace `verifyCargoDeps = true;` with
+   `legacyCargoFetcher = false;` in Rust applications.
+
+2. Send a treewide Rust PR that sets `legacyCargoFetcher = true;` in all Rust
+   applications 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.
+
+3. Go through all Rust src packages deleting the `legacyCargoFetcher = false;`
+   line and re-computing the `cargoSha256`, merging as we go.
+
+4. Delete the `fetchcargo.nix` implementation entirely and also remove:
+  - All overrides in application-level packages
+  - The `fetchcargo-default-config.toml` and conditionals around using it when
+    no `$CARGO_CONFIG` exists
+  - This README.md file
diff --git a/pkgs/build-support/rust/default.nix b/pkgs/build-support/rust/default.nix
index 4089436c0e0b8..ac0a8d3ae4643 100644
--- a/pkgs/build-support/rust/default.nix
+++ b/pkgs/build-support/rust/default.nix
@@ -1,4 +1,4 @@
-{ stdenv, cacert, git, rust, cargo, rustc, fetchcargo, buildPackages, windows }:
+{ stdenv, cacert, git, rust, cargo, rustc, fetchcargo, fetchCargoTarball, buildPackages, windows }:
 
 { name ? "${args.pname}-${args.version}"
 , cargoSha256 ? "unset"
@@ -14,13 +14,13 @@
 , cargoUpdateHook ? ""
 , cargoDepsHook ? ""
 , cargoBuildFlags ? []
-, # Set to true to verify if the cargo dependencies are up to date.
-  # This will change the value of cargoSha256.
-  verifyCargoDeps ? false
+  # Please set to true on any Rust package updates. Once all packages set this
+  # to true, we will delete and make it the default. For details, see the Rust
+  # section on the manual and ./README.md.
+, legacyCargoFetcher ? true
 , buildType ? "release"
 , meta ? {}
 , target ? null
-
 , cargoVendorDir ? null
 , ... } @ args:
 
@@ -28,20 +28,27 @@ assert cargoVendorDir == null -> cargoSha256 != "unset";
 assert buildType == "release" || buildType == "debug";
 
 let
+
+  cargoFetcher = if legacyCargoFetcher
+                 then fetchcargo
+                 else fetchCargoTarball;
+
   cargoDeps = if cargoVendorDir == null
-    then fetchcargo {
+    then cargoFetcher {
         inherit name src srcs sourceRoot unpackPhase cargoUpdateHook;
-        copyLockfile = verifyCargoDeps;
         patches = cargoPatches;
         sha256 = cargoSha256;
       }
     else null;
 
+  # If we're using the modern fetcher that always preserves the original Cargo.lock
+  # and have vendored deps, check them against the src attr for consistency.
+  validateCargoDeps = cargoSha256 != "unset" && !legacyCargoFetcher;
+
   setupVendorDir = if cargoVendorDir == null
     then ''
       unpackFile "$cargoDeps"
-      cargoDepsCopy=$(stripHash $(basename $cargoDeps))
-      chmod -R +w "$cargoDepsCopy"
+      cargoDepsCopy=$(stripHash $cargoDeps)
     ''
     else ''
       cargoDepsCopy="$sourceRoot/${cargoVendorDir}"
@@ -54,9 +61,14 @@ let
   ccForHost="${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc";
   cxxForHost="${stdenv.cc}/bin/${stdenv.cc.targetPrefix}c++";
   releaseDir = "target/${rustTarget}/${buildType}";
+
+  # Fetcher implementation choice should not be part of the hash in final
+  # derivation; only the cargoSha256 input matters.
+  filteredArgs = builtins.removeAttrs args [ "legacyCargoFetcher" ];
+
 in
 
-stdenv.mkDerivation (args // {
+stdenv.mkDerivation (filteredArgs // {
   inherit cargoDeps;
 
   patchRegistryDeps = ./patch-registry-deps;
@@ -95,14 +107,13 @@ stdenv.mkDerivation (args // {
     ''}
     EOF
 
-    unset cargoDepsCopy
     export RUST_LOG=${logLevel}
-  '' + stdenv.lib.optionalString verifyCargoDeps ''
-    if ! diff source/Cargo.lock $cargoDeps/Cargo.lock ; then
+  '' + stdenv.lib.optionalString validateCargoDeps ''
+    if ! diff source/Cargo.lock $cargoDepsCopy/Cargo.lock ; then
       echo
       echo "ERROR: cargoSha256 is out of date"
       echo
-      echo "Cargo.lock is not the same in $cargoDeps"
+      echo "Cargo.lock is not the same in $cargoDepsCopy"
       echo
       echo "To fix the issue:"
       echo '1. Use "1111111111111111111111111111111111111111111111111111" as the cargoSha256 value'
@@ -112,6 +123,8 @@ stdenv.mkDerivation (args // {
 
       exit 1
     fi
+  '' + ''
+    unset cargoDepsCopy
   '' + (args.postUnpack or "");
 
   configurePhase = args.configurePhase or ''
diff --git a/pkgs/build-support/rust/fetchCargoTarball.nix b/pkgs/build-support/rust/fetchCargoTarball.nix
new file mode 100644
index 0000000000000..dff5d99da9eb3
--- /dev/null
+++ b/pkgs/build-support/rust/fetchCargoTarball.nix
@@ -0,0 +1,81 @@
+{ stdenv, cacert, git, cargo, python3 }:
+let cargo-vendor-normalise = stdenv.mkDerivation {
+  name = "cargo-vendor-normalise";
+  src = ./cargo-vendor-normalise.py;
+  nativeBuildInputs = [ python3.pkgs.wrapPython ];
+  dontUnpack = true;
+  installPhase = "install -D $src $out/bin/cargo-vendor-normalise";
+  pythonPath = [ python3.pkgs.toml ];
+  postFixup = "wrapPythonPrograms";
+  doInstallCheck = true;
+  installCheckPhase = ''
+    # check that ./fetchcargo-default-config.toml is a fix point
+    reference=${./fetchcargo-default-config.toml}
+    < $reference $out/bin/cargo-vendor-normalise > test;
+    cmp test $reference
+  '';
+  preferLocalBuild = true;
+};
+in
+{ name ? "cargo-deps"
+, src ? null
+, srcs ? []
+, patches ? []
+, sourceRoot
+, sha256
+, cargoUpdateHook ? ""
+, ...
+} @ args:
+stdenv.mkDerivation ({
+  name = "${name}-vendor.tar.gz";
+  nativeBuildInputs = [ cacert git cargo-vendor-normalise cargo ];
+
+  phases = "unpackPhase patchPhase buildPhase installPhase";
+
+  buildPhase = ''
+    # Ensure deterministic Cargo vendor builds
+    export SOURCE_DATE_EPOCH=1
+
+    if [[ ! -f Cargo.lock ]]; then
+        echo
+        echo "ERROR: The Cargo.lock file doesn't exist"
+        echo
+        echo "Cargo.lock is needed to make sure that cargoSha256 doesn't change"
+        echo "when the registry is updated."
+        echo
+
+        exit 1
+    fi
+
+    # Keep the original around for copyLockfile
+    cp Cargo.lock Cargo.lock.orig
+
+    export CARGO_HOME=$(mktemp -d cargo-home.XXX)
+    CARGO_CONFIG=$(mktemp cargo-config.XXXX)
+
+    ${cargoUpdateHook}
+
+    cargo vendor $name | cargo-vendor-normalise > $CARGO_CONFIG
+
+    # Add the Cargo.lock to allow hash invalidation
+    cp Cargo.lock.orig $name/Cargo.lock
+
+    # Packages with git dependencies generate non-default cargo configs, so
+    # always install it rather than trying to write a standard default template.
+    install -D $CARGO_CONFIG $name/.cargo/config;
+  '';
+
+  # Build a reproducible tar, per instructions at https://reproducible-builds.org/docs/archives/
+  installPhase = ''
+    tar --owner=0 --group=0 --numeric-owner --format=gnu \
+        --sort=name --mtime="@$SOURCE_DATE_EPOCH" \
+        -czf $out $name
+  '';
+
+  outputHashAlgo = "sha256";
+  outputHash = sha256;
+
+  impureEnvVars = stdenv.lib.fetchers.proxyImpureEnvVars;
+} // (builtins.removeAttrs args [
+  "name" "sha256" "cargoUpdateHook"
+]))
diff --git a/pkgs/development/compilers/rust/default.nix b/pkgs/development/compilers/rust/default.nix
index da3421a987dab..af7f63b6f8a00 100644
--- a/pkgs/development/compilers/rust/default.nix
+++ b/pkgs/development/compilers/rust/default.nix
@@ -25,12 +25,18 @@
       inherit rustc cargo;
     };
 
+    fetchCargoTarball = buildPackages.callPackage ../../../build-support/rust/fetchCargoTarball.nix {
+      inherit cargo;
+    };
+
+    # N.B. This is a legacy fetcher implementation that is being phased out and deleted.
+    # See ../../../build-support/rust/README.md for details.
     fetchcargo = buildPackages.callPackage ../../../build-support/rust/fetchcargo.nix {
       inherit cargo;
     };
 
     buildRustPackage = callPackage ../../../build-support/rust {
-      inherit rustc cargo fetchcargo;
+      inherit rustc cargo fetchcargo fetchCargoTarball;
     };
 
     rustcSrc = callPackage ./rust-src.nix {
diff --git a/pkgs/development/tools/documentation/mdsh/default.nix b/pkgs/development/tools/documentation/mdsh/default.nix
index 2ca2ef29dd26a..ddce5bc29a438 100644
--- a/pkgs/development/tools/documentation/mdsh/default.nix
+++ b/pkgs/development/tools/documentation/mdsh/default.nix
@@ -11,8 +11,8 @@ rustPlatform.buildRustPackage rec {
     sha256 = "1a9i6h8fzrrfzjyfxaps73lxgkz92k0bnmwbjbwdmiwci4qgi9ms";
   };
 
-  cargoSha256 = "0rarpzfigyxr6s0ba13z00kvnms29qkjfbfjkay72mb6xn7f1059";
-  verifyCargoDeps = true;
+  cargoSha256 = "1fxajh1n0qvcdas6w7dy3g92wilhfldy90pyk3779mrnh57fa6n5";
+  legacyCargoFetcher = false;
 
   meta = with stdenv.lib; {
     description = "Markdown shell pre-processor";
diff --git a/pkgs/tools/misc/broot/default.nix b/pkgs/tools/misc/broot/default.nix
index 45b26b2505589..df26423c63310 100644
--- a/pkgs/tools/misc/broot/default.nix
+++ b/pkgs/tools/misc/broot/default.nix
@@ -11,8 +11,8 @@ rustPlatform.buildRustPackage rec {
     sha256 = "13b1w9g68aj3r70w9bmrmdc772y959n77ajbdm2cpjs5f4kgfpak";
   };
 
-  cargoSha256 = "0vzpyymylzxjm613lf5xr6hd21ijkl3vwq4y6h1q3as41phw2sqb";
-  verifyCargoDeps = true;
+  cargoSha256 = "0zrwpmsrzwnjml0964zky8w222zmlargha3z0n6hf8cfshx23s4k";
+  legacyCargoFetcher = false;
 
   nativeBuildInputs = [ installShellFiles ];
 
diff --git a/pkgs/tools/misc/wagyu/default.nix b/pkgs/tools/misc/wagyu/default.nix
index d56d21b151664..53e0984952267 100644
--- a/pkgs/tools/misc/wagyu/default.nix
+++ b/pkgs/tools/misc/wagyu/default.nix
@@ -11,8 +11,8 @@ rustPlatform.buildRustPackage rec {
     sha256 = "1646j0lgg3hhznifvbkvr672p3yqlcavswijawaxq7n33ll8vmcn";
   };
 
-  cargoSha256 = "10b96l0b32zxq0xrnhivv3gihmi5y31rllbizv67hrg1axz095vn";
-  verifyCargoDeps = true;
+  cargoSha256 = "16d1b3pamkg29nq80n6cbzc4zl9z3cgfvdxjkr2z4xrnzmkn1ysi";
+  legacyCargoFetcher = false;
 
   meta = with lib; {
     description = "Rust library for generating cryptocurrency wallets";
diff --git a/pkgs/tools/package-management/nix-du/default.nix b/pkgs/tools/package-management/nix-du/default.nix
index 74543cef83b19..908f31b93abe0 100644
--- a/pkgs/tools/package-management/nix-du/default.nix
+++ b/pkgs/tools/package-management/nix-du/default.nix
@@ -9,8 +9,8 @@ rustPlatform.buildRustPackage rec {
     rev = "v${version}";
     sha256 = "149d60mid29s5alv5m3d7jrhyzc6cj7b6hpiq399gsdwzgxr00wq";
   };
-  cargoSha256 = "18kb4car5nzch3vpl6z1499silhs3fyn8c6xj3rzk94mm2m9srg4";
-  verifyCargoDeps = true;
+  cargoSha256 = "1a6svl89dcdb5fpvs2i32i6agyhl0sx7kkkw70rqr17fyzl5psai";
+  legacyCargoFetcher = false;
 
   doCheck = true;
   checkInputs = [ graphviz ];
diff --git a/pkgs/tools/security/fido2luks/default.nix b/pkgs/tools/security/fido2luks/default.nix
index 4682a09acf5c7..ea911e4673400 100644
--- a/pkgs/tools/security/fido2luks/default.nix
+++ b/pkgs/tools/security/fido2luks/default.nix
@@ -19,8 +19,8 @@ rustPlatform.buildRustPackage rec {
   buildInputs = [ cryptsetup ];
   nativeBuildInputs = [ pkg-config ];
 
-  cargoSha256 = "1i37k4ih6118z3wip2qh4jqk7ja2z0v1w8dri1lwqwlciqw17zi9";
-  verifyCargoDeps = true;
+  cargoSha256 = "0rp4f6xnwmvf3pv6h0qwsg01jrndf77yn67675ac39kxzmrzfy2f";
+  legacyCargoFetcher = false;
 
   meta = with stdenv.lib; {
     description = "Decrypt your LUKS partition using a FIDO2 compatible authenticator";
diff --git a/pkgs/tools/system/tre-command/default.nix b/pkgs/tools/system/tre-command/default.nix
index 6050b2a709ebc..b8925dcfa42eb 100644
--- a/pkgs/tools/system/tre-command/default.nix
+++ b/pkgs/tools/system/tre-command/default.nix
@@ -11,8 +11,8 @@ rustPlatform.buildRustPackage rec {
     sha256 = "1fazw2wn738iknbv54gv7qll7d4q2gy9bq1s3f3cv21cdv6bqral";
   };
 
-  cargoSha256 = "0m82zbi610zgvcza6n03xl80g31x6bfkjyrfxcxa6fyf2l5cj9pv";
-  verifyCargoDeps = true;
+  cargoSha256 = "1m3ccp5ncafkifg8sxyxczsg3ja1gvq8wmgni68bgzm2lwxh2qgw";
+  legacyCargoFetcher = false;
 
   meta = with stdenv.lib; {
     description = "Tree command, improved";