Merge #82342: rustPlatform: increase build-speed of checkPhase

...for rust-packages (into staging)

doc/languages-frameworks/rust.section.md

@@ -75,6 +75,72 @@ pkgs.rustPlatform.buildRustPackage {
 }
 ```
 
+### Running package tests
+
+When using `buildRustPackage`, the `checkPhase` is enabled by default and runs
+`cargo test` on the package to build. To make sure that we don't compile the
+sources twice and to actually test the artifacts that will be used at runtime, 
+the tests will be ran in the `release` mode by default.
+
+However, in some cases the test-suite of a package doesn't work properly in the
+`release` mode. For these situations, the mode for `checkPhase` can be changed like
+so:
+
+```nix
+rustPlatform.buildRustPackage {
+  /* ... */
+  checkType = "debug";
+}
+```
+
+Please note that the code will be compiled twice here: once in `release` mode
+for the `buildPhase`, and again in `debug` mode for the `checkPhase`.
+
+#### Tests relying on the structure of the `target/` directory
+
+Some tests may rely on the structure of the `target/` directory. Those tests
+are likely to fail because we use `cargo --target` during the build. This means that
+the artifacts
+[are stored in `target/<architecture>/release/`](https://doc.rust-lang.org/cargo/guide/build-cache.html),
+rather than in `target/release/`.
+
+This can only be worked around by patching the affected tests accordingly.
+
+#### Disabling package-tests
+
+In some instances, it may be necessary to disable testing altogether (with `doCheck = false;`):
+
+* If no tests exist -- the `checkPhase` should be explicitly disabled to skip
+  unnecessary build steps to speed up the build.
+* If tests are highly impure (e.g. due to network usage).
+
+There will obviously be some corner-cases not listed above where it's sensible to disable tests.
+The above are just guidelines, and exceptions may be granted on a case-by-case basis.
+
+However, please check if it's possible to disable a problematic subset of the
+test suite and leave a comment explaining your reasoning.
+
+### Building a package in `debug` mode
+
+By default, `buildRustPackage` will use `release` mode for builds. If a package
+should be built in `debug` mode, it can be configured like so:
+
+```nix
+rustPlatform.buildRustPackage {
+  /* ... */
+  buildType = "debug";
+}
+```
+
+In this scenario, the `checkPhase` will be ran in `debug` mode as well.
+
+### Custom `build`/`install`-procedures
+
+Some packages may use custom scripts for building/installing, e.g. with a `Makefile`.
+In these cases, it's recommended to override the `buildPhase`/`installPhase`/`checkPhase`.
+
+Otherwise, some steps may fail because of the modified directory structure of `target/`.
+
 ## Compiling Rust crates using Nix instead of Cargo
 
 ### Simple operation

nixos/doc/manual/release-notes/rl-2009.xml

@@ -474,9 +474,21 @@ systemd.services.nginx.serviceConfig.ReadWritePaths = [ "/var/www" ];
     </para>
    </listitem>
    <listitem>
-   <para>
-     The default output of <literal>buildGoPackage</literal> is now <literal>$out</literal> instead of <literal>$bin</literal>.
-   </para>
+    <para>
+      The default output of <literal>buildGoPackage</literal> is now <literal>$out</literal> instead of <literal>$bin</literal>.
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+     Packages built using <literal>buildRustPackage</literal> now use <literal>release</literal>
+     mode for the <literal>checkPhase</literal> by default.
+    </para>
+    <para>
+     Please note that Rust packages utilizing a custom build/install procedure
+     (e.g. by using a <filename>Makefile</filename>) or test suites that rely on the
+     structure of the <filename>target/</filename> directory may break due to those assumptions.
+     For further information, please read the Rust section in the Nixpkgs manual.
+    </para>
    </listitem>
    <listitem>
    <para>

pkgs/applications/editors/neovim/gnvim/default.nix

@@ -33,6 +33,10 @@ rustPlatform.buildRustPackage rec {
     EOF
   '';
 
+  buildPhase = ''
+    make build
+  '';
+
   installPhase = ''
     make install PREFIX="${placeholder "out"}"
   '';

pkgs/applications/networking/nym/default.nix

@@ -24,6 +24,8 @@ rustPlatform.buildRustPackage rec {
 
   buildInputs = [ openssl ];
 
+  checkType = "debug";
+
   /*
   Nym's test presence::converting_mixnode_presence_into_topology_mixnode::it_returns_resolved_ip_on_resolvable_hostname tries to resolve nymtech.net.
   Since there is no external DNS resolution available in the build sandbox, we point cargo and its children (that's what we remove the 'unsetenv' call for) to a hosts file in which we statically resolve nymtech.net.

pkgs/build-support/rust/default.nix

@@ -28,6 +28,13 @@
 , meta ? {}
 , target ? null
 , cargoVendorDir ? null
+, checkType ? buildType
+
+# Needed to `pushd`/`popd` into a subdir of a tarball if this subdir
+# contains a Cargo.toml, but isn't part of a workspace (which is e.g. the
+# case for `rustfmt`/etc from the `rust-sources).
+# Otherwise, everything from the tarball would've been built/tested.
+, buildAndTestSubdir ? null
 , ... } @ args:
 
 assert cargoVendorDir == null -> cargoSha256 != "unset";
@@ -161,6 +168,7 @@ stdenv.mkDerivation (args // {
   '';
 
   buildPhase = with builtins; args.buildPhase or ''
+    ${stdenv.lib.optionalString (buildAndTestSubdir != null) "pushd ${buildAndTestSubdir}"}
     runHook preBuild
 
     (
@@ -176,22 +184,29 @@ stdenv.mkDerivation (args // {
         --frozen ${concatStringsSep " " cargoBuildFlags}
     )
 
-    # rename the output dir to a architecture independent one
-    mapfile -t targets < <(find "$NIX_BUILD_TOP" -type d | grep '${releaseDir}$')
-    for target in "''${targets[@]}"; do
-      rm -rf "$target/../../${buildType}"
-      ln -srf "$target" "$target/../../"
-    done
-
     runHook postBuild
+
+    ${stdenv.lib.optionalString (buildAndTestSubdir != null) "popd"}
+
+    # This needs to be done after postBuild: packages like `cargo` do a pushd/popd in
+    # the pre/postBuild-hooks that need to be taken into account before gathering
+    # all binaries to install.
+    bins=$(find $releaseDir \
+      -maxdepth 1 \
+      -type f \
+      -executable ! \( -regex ".*\.\(so.[0-9.]+\|so\|a\|dylib\)" \))
   '';
 
-  checkPhase = args.checkPhase or ''
+  checkPhase = args.checkPhase or (let
+    argstr = "${stdenv.lib.optionalString (checkType == "release") "--release"} --target ${rustTarget} --frozen";
+  in ''
+    ${stdenv.lib.optionalString (buildAndTestSubdir != null) "pushd ${buildAndTestSubdir}"}
     runHook preCheck
-    echo "Running cargo cargo test -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}"
-    cargo test -- ''${checkFlags} ''${checkFlagsArray+"''${checkFlagsArray[@]}"}
+    echo "Running cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}"
+    cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+"''${checkFlagsArray[@]}"}
     runHook postCheck
-  '';
+    ${stdenv.lib.optionalString (buildAndTestSubdir != null) "popd"}
+  '');
 
   doCheck = args.doCheck or true;
 
@@ -201,13 +216,16 @@ stdenv.mkDerivation (args // {
 
   installPhase = args.installPhase or ''
     runHook preInstall
+
+    # rename the output dir to a architecture independent one
+    mapfile -t targets < <(find "$NIX_BUILD_TOP" -type d | grep '${releaseDir}$')
+    for target in "''${targets[@]}"; do
+      rm -rf "$target/../../${buildType}"
+      ln -srf "$target" "$target/../../"
+    done
     mkdir -p $out/bin $out/lib
 
-    find $releaseDir \
-      -maxdepth 1 \
-      -type f \
-      -executable ! \( -regex ".*\.\(so.[0-9.]+\|so\|a\|dylib\)" \) \
-      -print0 | xargs -r -0 cp -t $out/bin
+    xargs -r cp -t $out/bin <<< $bins
     find $releaseDir \
       -maxdepth 1 \
       -regex ".*\.\(so.[0-9.]+\|so\|a\|dylib\)" \

pkgs/development/compilers/rust/cargo.nix

@@ -9,8 +9,7 @@ rustPlatform.buildRustPackage {
 
   # the rust source tarball already has all the dependencies vendored, no need to fetch them again
   cargoVendorDir = "vendor";
-  preBuild = "pushd src/tools/cargo";
-  postBuild = "popd";
+  buildAndTestSubdir = "src/tools/cargo";
 
   passthru.rustc = rustc;
 

pkgs/development/compilers/rust/clippy.nix

@@ -5,8 +5,7 @@ rustPlatform.buildRustPackage {
 
   # the rust source tarball already has all the dependencies vendored, no need to fetch them again
   cargoVendorDir = "vendor";
-  preBuild = "pushd src/tools/clippy";
-  postBuild = "popd";
+  buildAndTestSubdir = "src/tools/clippy";
 
   # changes hash of vendor directory otherwise
   dontUpdateAutotoolsGnuConfigScripts = true;

pkgs/development/compilers/rust/rls/default.nix

@@ -10,8 +10,9 @@ rustPlatform.buildRustPackage {
   dontUpdateAutotoolsGnuConfigScripts = true;
 
   cargoVendorDir = "vendor";
+  buildAndTestSubdir = "src/tools/rls";
+
   preBuild = ''
-    pushd src/tools/rls
     # client tests are flaky
     rm tests/client.rs
   '';
@@ -28,8 +29,6 @@ rustPlatform.buildRustPackage {
 
   doCheck = true;
 
-  preInstall = "popd";
-
   doInstallCheck = true;
   installCheckPhase = ''
     $out/bin/rls --version

pkgs/development/compilers/rust/rustfmt.nix

@@ -6,8 +6,7 @@ rustPlatform.buildRustPackage rec {
 
   # the rust source tarball already has all the dependencies vendored, no need to fetch them again
   cargoVendorDir = "vendor";
-  preBuild = "pushd src/tools/rustfmt";
-  preInstall = "popd";
+  buildAndTestSubdir = "src/tools/rustfmt";
 
   # changes hash of vendor directory otherwise
   dontUpdateAutotoolsGnuConfigScripts = true;
@@ -17,12 +16,6 @@ rustPlatform.buildRustPackage rec {
   # As of 1.0.0 and rustc 1.30 rustfmt requires a nightly compiler
   RUSTC_BOOTSTRAP = 1;
 
-  # we run tests in debug mode so tests look for a debug build of
-  # rustfmt. Anyway this adds nearly no compilation time.
-  preCheck = ''
-    cargo build
-  '';
-
   meta = with stdenv.lib; {
     description = "A tool for formatting Rust code according to style guidelines";
     homepage = "https://github.com/rust-lang-nursery/rustfmt";

pkgs/development/tools/rust/rust-analyzer/generic.nix

@@ -15,9 +15,7 @@ rustPlatform.buildRustPackage {
     inherit rev sha256;
   };
 
-  preBuild = "pushd crates/rust-analyzer";
-  # Do not checking other crates in checkPhase.
-  preInstall = "popd";
+  buildAndTestSubdir = "crates/rust-analyzer";
 
   cargoBuildFlags = lib.optional useJemalloc "--features=jemalloc";
 

pkgs/tools/package-management/cargo-deb/default.nix

@@ -2,7 +2,9 @@
 , lib
 , fetchFromGitHub
 , rustPlatform
-, Security }:
+, rust
+, Security
+}:
 
 rustPlatform.buildRustPackage rec {
   pname = "cargo-deb";
@@ -19,6 +21,13 @@ rustPlatform.buildRustPackage rec {
 
   cargoSha256 = "1vqnnqn6rzkdi239bh3lk7gaxr7w6v3c4ws4ya1ah04g6v9hkzlw";
 
+  checkType = "debug";
+
+  preCheck = ''
+    substituteInPlace tests/command.rs \
+      --replace 'target/debug' "target/${rust.toRustTarget stdenv.buildPlatform}/debug"
+  '';
+
   meta = with lib; {
     description = "Generate Debian packages from information in Cargo.toml";
     homepage = "https://github.com/mmstick/cargo-deb";

pkgs/tools/security/ripasso/cursive.nix

@@ -12,6 +12,8 @@ buildRustPackage rec {
     sha256 = "164da20j727p8l7hh37j2r8pai9sj402nhswvg0nrlgj53nr6083";
   };
 
+  patches = [ ./fix-tests.patch ];
+
   cargoSha256 = "1wpn67v0xmxhn1dgzhh1pwz1yc3cizmfxhpb7qv9b27ynx4486ji";
 
   cargoBuildFlags = [ "-p ripasso-cursive -p ripasso-man" ];

pkgs/tools/security/ripasso/fix-tests.patch

@@ -0,0 +1,35 @@
+diff --git a/src/pass/test.rs b/src/pass/test.rs
+index c980a2f..2e6c8cc 100644
+--- a/src/pass/test.rs
++++ b/src/pass/test.rs
+@@ -56,6 +56,7 @@ fn populate_password_list_small_repo() {
+     base_path.pop();
+     base_path.pop();
+     base_path.pop();
++    base_path.pop();
+     base_path.push("testres");
+ 
+     let mut password_dir: PathBuf = base_path.clone();
+@@ -84,6 +85,7 @@ fn populate_password_list_repo_with_deleted_files() {
+     base_path.pop();
+     base_path.pop();
+     base_path.pop();
++    base_path.pop();
+     base_path.push("testres");
+ 
+     let mut password_dir: PathBuf = base_path.clone();
+@@ -112,6 +114,7 @@ fn populate_password_list_directory_without_git() {
+     base_path.pop();
+     base_path.pop();
+     base_path.pop();
++    base_path.pop();
+     base_path.push("testres");
+ 
+     let mut password_dir: PathBuf = base_path.clone();
+@@ -149,4 +152,4 @@ fn parse_signing_keys_empty() {
+     let result = PasswordStore::parse_signing_keys(&None).unwrap();
+ 
+     assert_eq!(result.len(), 0);
+-}
+\ No newline at end of file
++}