From 04248f606f4ebfe298bfbfed3680a2fd5dacf796 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 25 Feb 2020 21:21:04 +0100 Subject: [PATCH 01/11] rustPlatform: increase build-speed of `checkPhase` for rust-packages When running `cargo test --release`, the artifacts from `buildPhase` will be reused here. Previously, most of the stuff had to be recompiled without optimizations. --- pkgs/build-support/rust/default.nix | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkgs/build-support/rust/default.nix b/pkgs/build-support/rust/default.nix index 93770d71a10d..6cece0048333 100644 --- a/pkgs/build-support/rust/default.nix +++ b/pkgs/build-support/rust/default.nix @@ -162,22 +162,17 @@ 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 ''; - checkPhase = args.checkPhase or '' + checkPhase = args.checkPhase or (let + argstr = "${stdenv.lib.optionalString (buildType == "release") "--release"} --target ${rustTarget} --frozen"; + in '' runHook preCheck - echo "Running cargo cargo test -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}" - cargo test -- ''${checkFlags} ''${checkFlagsArray+"''${checkFlagsArray[@]}"} + echo "Running cargo cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}" + cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+"''${checkFlagsArray[@]}"} runHook postCheck - ''; + ''); doCheck = args.doCheck or true; @@ -187,6 +182,13 @@ 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 \ From e32c005772c9f820780453628eb9b7cd9a51223e Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 24 Mar 2020 18:32:59 +0100 Subject: [PATCH 02/11] rustPlatform: don't install test executables This is done by gathering all binaries to install before running the checkPhase. --- pkgs/build-support/rust/default.nix | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/pkgs/build-support/rust/default.nix b/pkgs/build-support/rust/default.nix index 6cece0048333..9f11919974a8 100644 --- a/pkgs/build-support/rust/default.nix +++ b/pkgs/build-support/rust/default.nix @@ -165,6 +165,17 @@ stdenv.mkDerivation (args // { runHook postBuild ''; + postBuild = args.postBuild or "" + '' + + # 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 (let argstr = "${stdenv.lib.optionalString (buildType == "release") "--release"} --target ${rustTarget} --frozen"; in '' @@ -191,11 +202,7 @@ stdenv.mkDerivation (args // { 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\)" \ From f236714d65baa74404e5e25a24b6208134971752 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 6 May 2020 23:25:01 +0200 Subject: [PATCH 03/11] rustPlatform: fix log --- pkgs/build-support/rust/default.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/build-support/rust/default.nix b/pkgs/build-support/rust/default.nix index 9f11919974a8..b7d19ccd4a0a 100644 --- a/pkgs/build-support/rust/default.nix +++ b/pkgs/build-support/rust/default.nix @@ -180,7 +180,7 @@ stdenv.mkDerivation (args // { argstr = "${stdenv.lib.optionalString (buildType == "release") "--release"} --target ${rustTarget} --frozen"; in '' runHook preCheck - echo "Running cargo cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}" + echo "Running cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}" cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+"''${checkFlagsArray[@]}"} runHook postCheck ''); From 736462d995435ad14540f8137fddbd467db63a0a Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 13 May 2020 01:15:23 +0200 Subject: [PATCH 04/11] rustPlatform: make it possible to override the profile for `cargo test` --- pkgs/build-support/rust/default.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/build-support/rust/default.nix b/pkgs/build-support/rust/default.nix index aa5edc1f5bf7..e79e902bcdad 100644 --- a/pkgs/build-support/rust/default.nix +++ b/pkgs/build-support/rust/default.nix @@ -28,6 +28,7 @@ , meta ? {} , target ? null , cargoVendorDir ? null +, checkType ? buildType , ... } @ args: assert cargoVendorDir == null -> cargoSha256 != "unset"; @@ -191,7 +192,7 @@ stdenv.mkDerivation (args // { ''; checkPhase = args.checkPhase or (let - argstr = "${stdenv.lib.optionalString (buildType == "release") "--release"} --target ${rustTarget} --frozen"; + argstr = "${stdenv.lib.optionalString (checkType == "release") "--release"} --target ${rustTarget} --frozen"; in '' runHook preCheck echo "Running cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}" From 6b23cfe6894ab86ddf9b62944f39d616f993ff55 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 13 May 2020 01:28:24 +0200 Subject: [PATCH 05/11] rustPlatform: add `buildAndTestSubdir`-argument There are several tarballs (such as the `rust-lang/rust`-source) with a `Cargo.toml` at root and several sub-packages (with their own Cargo.toml) without using workspaces[1]. In such a case it's needed to move into a subdir to only build the specified sub-package (e.g. `rustfmt` or `rsl`), however the artifacts are at `/target` in the root-dir of the build environment. This breaks the build since `buildRustPackage` searches for executables in `target` (which is at the build-env's root) at the end of the `buildPhase`. With the optional `buildAndTestSubdir`-argument, the builder moves into the specified subdir using `pushd`/`popd` during `buildPhase` and `checkPhase`. Also moved the logic to find executables and libs to the end of the `buildPhase` from a custom `postBuild`-hook to fix packages with custom `build`/`install`-procedures such as `uutils-coreutils`. [1] https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html --- pkgs/build-support/rust/default.nix | 12 ++++++++++-- pkgs/development/compilers/rust/cargo.nix | 3 +-- pkgs/development/compilers/rust/clippy.nix | 3 +-- pkgs/development/compilers/rust/rls/default.nix | 5 ++--- pkgs/development/compilers/rust/rustfmt.nix | 9 +-------- .../development/tools/rust/rust-analyzer/generic.nix | 4 +--- 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/pkgs/build-support/rust/default.nix b/pkgs/build-support/rust/default.nix index e79e902bcdad..d3e13b913f38 100644 --- a/pkgs/build-support/rust/default.nix +++ b/pkgs/build-support/rust/default.nix @@ -29,6 +29,12 @@ , 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"; @@ -162,6 +168,7 @@ stdenv.mkDerivation (args // { ''; buildPhase = with builtins; args.buildPhase or '' + ${stdenv.lib.optionalString (buildAndTestSubdir != null) "pushd ${buildAndTestSubdir}"} runHook preBuild ( @@ -178,9 +185,8 @@ stdenv.mkDerivation (args // { ) runHook postBuild - ''; - postBuild = args.postBuild or "" + '' + ${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 @@ -194,10 +200,12 @@ stdenv.mkDerivation (args // { 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 test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+''${checkFlagsArray[@]}}" cargo test ${argstr} -- ''${checkFlags} ''${checkFlagsArray+"''${checkFlagsArray[@]}"} runHook postCheck + ${stdenv.lib.optionalString (buildAndTestSubdir != null) "popd"} ''); doCheck = args.doCheck or true; diff --git a/pkgs/development/compilers/rust/cargo.nix b/pkgs/development/compilers/rust/cargo.nix index 65614b9480e7..dfea7f6c8ef6 100644 --- a/pkgs/development/compilers/rust/cargo.nix +++ b/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; diff --git a/pkgs/development/compilers/rust/clippy.nix b/pkgs/development/compilers/rust/clippy.nix index 4857b587847e..0546ad9bac1a 100644 --- a/pkgs/development/compilers/rust/clippy.nix +++ b/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; diff --git a/pkgs/development/compilers/rust/rls/default.nix b/pkgs/development/compilers/rust/rls/default.nix index 4cf507fbf5c7..a050be7c2bfd 100644 --- a/pkgs/development/compilers/rust/rls/default.nix +++ b/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 diff --git a/pkgs/development/compilers/rust/rustfmt.nix b/pkgs/development/compilers/rust/rustfmt.nix index f8ed0bce2e0d..66a18f40ad42 100644 --- a/pkgs/development/compilers/rust/rustfmt.nix +++ b/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"; diff --git a/pkgs/development/tools/rust/rust-analyzer/generic.nix b/pkgs/development/tools/rust/rust-analyzer/generic.nix index de755ec17ff5..ae6ad80cdd9a 100644 --- a/pkgs/development/tools/rust/rust-analyzer/generic.nix +++ b/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"; From 0d7f889607e97c73a3b73ff79311c4fd8cc72aab Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 13 May 2020 01:38:15 +0200 Subject: [PATCH 06/11] cargo-deb: fix build --- pkgs/tools/package-management/cargo-deb/default.nix | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkgs/tools/package-management/cargo-deb/default.nix b/pkgs/tools/package-management/cargo-deb/default.nix index c6e8b4803ccf..96ef0eef8c5a 100644 --- a/pkgs/tools/package-management/cargo-deb/default.nix +++ b/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"; From e49f3a47604b2c18ac2e0aa9afe7bf786d61e34b Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 13 May 2020 22:19:16 +0200 Subject: [PATCH 07/11] nym: fix tests A lot of tests are using `debug_assert!` which isn't available in release-mode. --- pkgs/applications/networking/nym/default.nix | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/applications/networking/nym/default.nix b/pkgs/applications/networking/nym/default.nix index 6bb86c016ba3..1ff449eab0cc 100644 --- a/pkgs/applications/networking/nym/default.nix +++ b/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. From 068beb2c07788ea5fd8a47d0ef9d284b47df7dfc Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 13 May 2020 22:19:47 +0200 Subject: [PATCH 08/11] gnvim: fix build When running the default builder for Rust, the artifacts would be stored in `target//`, however the `install`-target expects the default structure (`target/`) of `cargo`-builds. When using the Makefile for building as well, the expected structure is created instead. --- pkgs/applications/editors/neovim/gnvim/default.nix | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkgs/applications/editors/neovim/gnvim/default.nix b/pkgs/applications/editors/neovim/gnvim/default.nix index e9f42d2b9b5e..3693ff322338 100644 --- a/pkgs/applications/editors/neovim/gnvim/default.nix +++ b/pkgs/applications/editors/neovim/gnvim/default.nix @@ -33,6 +33,10 @@ rustPlatform.buildRustPackage rec { EOF ''; + buildPhase = '' + make build + ''; + installPhase = '' make install PREFIX="${placeholder "out"}" ''; From 913dcddb6852e53e64cd6f6e7f998d6b6f57841e Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 13 May 2020 22:22:51 +0200 Subject: [PATCH 09/11] ripasso-cursive: fix tests Needed since we store artifacts in `target//`. --- pkgs/tools/security/ripasso/cursive.nix | 2 ++ pkgs/tools/security/ripasso/fix-tests.patch | 35 +++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 pkgs/tools/security/ripasso/fix-tests.patch diff --git a/pkgs/tools/security/ripasso/cursive.nix b/pkgs/tools/security/ripasso/cursive.nix index c8a55d3f397f..29229bff0028 100644 --- a/pkgs/tools/security/ripasso/cursive.nix +++ b/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" ]; diff --git a/pkgs/tools/security/ripasso/fix-tests.patch b/pkgs/tools/security/ripasso/fix-tests.patch new file mode 100644 index 000000000000..433ff933b1f7 --- /dev/null +++ b/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 ++} From 6574ba1946f3957fbf8bc1ba56850e26e15fa096 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 24 May 2020 18:37:34 +0200 Subject: [PATCH 10/11] rust*: add docs for testing packages See also https://discourse.nixos.org/t/rust-build-speed-improvements/7225 --- doc/languages-frameworks/rust.section.md | 66 ++++++++++++++++++++++ nixos/doc/manual/release-notes/rl-2009.xml | 18 +++++- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/doc/languages-frameworks/rust.section.md b/doc/languages-frameworks/rust.section.md index cec3373cbee6..066633b53c43 100644 --- a/doc/languages-frameworks/rust.section.md +++ b/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 after that, +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. In that case, the mode for `checkPhase` can be changed like this: + +```nix +rustPlatform.buildRustPackage { + /* ... */ + checkType = "debug"; +} +``` + +#### 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 since we use `cargo --target` during build. This means that +the artifacts +[are stored in `target//release/`](https://doc.rust-lang.org/cargo/guide/build-cache.html) +rather than `target/release/`. + +This can only be circumvented by patching the affected tests accordingly. + +#### Disabling package-tests + +In some cases it's necessary to disable the tests (which can be done by declaring +`doCheck = false;`) which is fine in the following cases: + +* 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 heavy network usage), it's also fine + disable tests. + +There are obviously some other corner-cases where it's sensible to disable tests, +those aren't hard-rules, in the end this is a case-by-case decision. + +Please check however if it's possible to disable a problematic subset of the +test-suite and leave comment which explains why that's needed. + +### Building a package in the `debug` mode + +By default, `buildRustPackage` will use the `release`-mode for building. If a package +should be built in the `debug`-mode however, it can be configured like this: + +```nix +rustPlatform.buildRustPackage { + /* ... */ + buildType = "debug"; +} +``` + +Obviously, the `checkPhase` will be ran in `debug`-mode as well in this case. + +### Custom `build`/`install`-procedures + +Some packages may use custom scripts for building/installing, e.g. with a `Makefile`. +In that case it's recommended to always override the `build-`/`install-`/`checkPhase`. + +Otherwise, it may be possible that one of the internal steps fails because of the +modified directory structure of `target/`. + ## Compiling Rust crates using Nix instead of Cargo ### Simple operation diff --git a/nixos/doc/manual/release-notes/rl-2009.xml b/nixos/doc/manual/release-notes/rl-2009.xml index 744be530be75..afd3b5040982 100644 --- a/nixos/doc/manual/release-notes/rl-2009.xml +++ b/nixos/doc/manual/release-notes/rl-2009.xml @@ -354,9 +354,21 @@ php.override { - - The default output of buildGoPackage is now $out instead of $bin. - + + The default output of buildGoPackage is now $out instead of $bin. + + + + + Packages built using buildRustPackage now use the release + mode for the checkPhase by default. + + + Please note that rust-packages utilizing a custom build/install-procedure + (e.g. by using a Makefile) or test-suites that rely on the + structure in the target/-directory may break because of that. + For further information, please read the rust-section in the nixpkgs manual. + From 59e8e7a129c1f9f657139b8554e1adf41c26de40 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 31 May 2020 20:47:32 +0200 Subject: [PATCH 11/11] rust: improve docs Co-authored-by: cole-h Co-authored-by: asymmetric --- doc/languages-frameworks/rust.section.md | 54 +++++++++++----------- nixos/doc/manual/release-notes/rl-2009.xml | 10 ++-- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/doc/languages-frameworks/rust.section.md b/doc/languages-frameworks/rust.section.md index 066633b53c43..7e5cff8ff607 100644 --- a/doc/languages-frameworks/rust.section.md +++ b/doc/languages-frameworks/rust.section.md @@ -79,11 +79,12 @@ pkgs.rustPlatform.buildRustPackage { 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 after that, -the tests will be ran in the `release`-mode by default. +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. In that case, the mode for `checkPhase` can be changed like this: +`release` mode. For these situations, the mode for `checkPhase` can be changed like +so: ```nix rustPlatform.buildRustPackage { @@ -92,37 +93,37 @@ rustPlatform.buildRustPackage { } ``` -#### Tests relying on the structure of the `target/`-directory +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`. -Some tests may rely on the structure of the `target/`-directory. Those tests -are likely to fail since we use `cargo --target` during build. This means that +#### 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//release/`](https://doc.rust-lang.org/cargo/guide/build-cache.html) -rather than `target/release/`. +[are stored in `target//release/`](https://doc.rust-lang.org/cargo/guide/build-cache.html), +rather than in `target/release/`. -This can only be circumvented by patching the affected tests accordingly. +This can only be worked around by patching the affected tests accordingly. #### Disabling package-tests -In some cases it's necessary to disable the tests (which can be done by declaring -`doCheck = false;`) which is fine in the following cases: +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 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). -* If tests are highly impure (e.g. due to heavy network usage), it's also fine - disable tests. +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. -There are obviously some other corner-cases where it's sensible to disable tests, -those aren't hard-rules, in the end this is a case-by-case decision. +However, please check if it's possible to disable a problematic subset of the +test suite and leave a comment explaining your reasoning. -Please check however if it's possible to disable a problematic subset of the -test-suite and leave comment which explains why that's needed. +### Building a package in `debug` mode -### Building a package in the `debug` mode - -By default, `buildRustPackage` will use the `release`-mode for building. If a package -should be built in the `debug`-mode however, it can be configured like this: +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 { @@ -131,15 +132,14 @@ rustPlatform.buildRustPackage { } ``` -Obviously, the `checkPhase` will be ran in `debug`-mode as well in this case. +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 that case it's recommended to always override the `build-`/`install-`/`checkPhase`. +In these cases, it's recommended to override the `buildPhase`/`installPhase`/`checkPhase`. -Otherwise, it may be possible that one of the internal steps fails because of the -modified directory structure of `target/`. +Otherwise, some steps may fail because of the modified directory structure of `target/`. ## Compiling Rust crates using Nix instead of Cargo diff --git a/nixos/doc/manual/release-notes/rl-2009.xml b/nixos/doc/manual/release-notes/rl-2009.xml index afd3b5040982..04b0802ad91f 100644 --- a/nixos/doc/manual/release-notes/rl-2009.xml +++ b/nixos/doc/manual/release-notes/rl-2009.xml @@ -360,14 +360,14 @@ php.override { - Packages built using buildRustPackage now use the release + Packages built using buildRustPackage now use release mode for the checkPhase by default. - Please note that rust-packages utilizing a custom build/install-procedure - (e.g. by using a Makefile) or test-suites that rely on the - structure in the target/-directory may break because of that. - For further information, please read the rust-section in the nixpkgs manual. + Please note that Rust packages utilizing a custom build/install procedure + (e.g. by using a Makefile) or test suites that rely on the + structure of the target/ directory may break due to those assumptions. + For further information, please read the Rust section in the Nixpkgs manual.