Merge pull request #49 from ipetkov/default-flags

Update default flags passed to cargo

CHANGELOG.md

@@ -6,6 +6,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## Unreleased
 
+### Changed
+* **Breaking**: the `--workspace` flag is no longer set for all cargo commands
+  by default. The previous behavior can be recovered by setting `cargoExtraArgs
+  = "--workspace";` in any derivation.
+* **Breaking**: the `$CARGO_PROFILE` environment variable can be used to specify
+  which cargo-profile all invocations use (by default `release` will be used).
+  Technically breaking if the default command was overridden for any derivation;
+  set `CARGO_PROFILE = null;` to avoid telling cargo to use a release build.
+* **Breaking**: `cargoTarpaulin` will use the release profile by default
+* All cargo invocations made during the build are automatically logged
+
 ## [0.5.1] - 2022-07-20
 
 ### Added

checks/compilesFresh.nix

@@ -7,7 +7,6 @@ src: expected: args:
 let
   runCargoAndCheckFreshness = cmd: extra: ''
     cargo ${cmd} \
-      --workspace \
       --release \
       --message-format json-diagnostic-short \
       ${extra} \

docs/API.md

@@ -86,15 +86,24 @@ to influence its behavior.
   - Default value: `"${cargoCheckCommand} ${cargoExtraArgs}\n${cargoBuildCommand} ${cargoExtraArgs}"`
 * `cargoBuildCommand`: A cargo (build) invocation to run during the derivation's build
   phase
-  - Default value: `"cargo build --workspace --release --all-targets"`
+  - Default value: `"cargo build --profile release --all-targets"`
+    * `CARGO_PROFILE` can be set on the derivation to alter which cargo profile
+      is selected; setting it to `null` will omit specifying a profile
+      altogether.
 * `cargoCheckCommand`: A cargo (check) invocation to run during the derivation's build
   phase (in order to cache additional artifacts)
-  - Default value: `"cargo build --workspace --release"`
+  - Default value: `"cargo build --profile release"`
+    * `CARGO_PROFILE` can be set on the derivation to alter which cargo profile
+      is selected; setting it to `null` will omit specifying a profile
+      altogether.
 * `cargoExtraArgs`: additional flags to be passed in the cargo invocation (e.g.
   enabling specific features)
 * `cargoTestCommand`: A cargo invocation to run during the derivation's check
   phase
-  - Default value: `"cargo test --workspace --release"`
+  - Default value: `"cargo test --profile release"`
+    * `CARGO_PROFILE` can be set on the derivation to alter which cargo profile
+      is selected; setting it to `null` will omit specifying a profile
+      altogether.
 * `cargoVendorDir`: A path (or derivation) of vendored cargo sources which can
   be consumed without network access. Directory structure should basically
   follow the output of `cargo vendor`.
@@ -148,7 +157,10 @@ log.
     file. The `cargoBuildLog` shell variable should point to this log.
 * `cargoBuildCommand`: A cargo invocation to run during the derivation's build
   phase
-  - Default value: `"cargo build --workspace --release"`
+  - Default value: `"cargo build --profile release"`
+    * `CARGO_PROFILE` can be set on the derivation to alter which cargo profile
+      is selected; setting it to `null` will omit specifying a profile
+      altogether.
 * `cargoExtraArgs`: additional flags to be passed in the cargo invocation (e.g.
   enabling specific features)
   - Default value: `""`
@@ -195,13 +207,19 @@ its behavior.
     set (with the respective default values)
 * `cargoBuildCommand`: A cargo invocation to run during the derivation's build
   phase
-  - Default value: `"cargo build --workspace --release"`
+  - Default value: `"cargo build --profile release"`
+    * `CARGO_PROFILE` can be set on the derivation to alter which cargo profile
+      is selected; setting it to `null` will omit specifying a profile
+      altogether.
 * `cargoExtraArgs`: additional flags to be passed in the cargo invocation (e.g.
   enabling specific features)
   - Default value: `""`
 * `cargoTestCommand`: A cargo invocation to run during the derivation's check
   phase
-  - Default value: `"cargo test --workspace --release"`
+  - Default value: `"cargo test --profile release"`
+    * `CARGO_PROFILE` can be set on the derivation to alter which cargo profile
+      is selected; setting it to `null` will omit specifying a profile
+      altogether.
 * `cargoVendorDir`: A path (or derivation) of vendored cargo sources which can
   be consumed without network access. Directory structure should basically
   follow the output of `cargo vendor`.
@@ -236,8 +254,11 @@ workspace.
 
 Except where noted below, all derivation attributes are delegated to
 `cargoBuild`, and can be used to influence its behavior.
-* `cargoBuildCommand` will be set to run `cargo clippy` for all targets in the
-  workspace.
+* `cargoBuildCommand` will be set to run `cargo clippy --profile release
+  --all-targets` for the workspace.
+  - `CARGO_PROFILE` can be set on the derivation to alter which cargo profile
+    is selected; setting it to `null` will omit specifying a profile
+    altogether.
 * `cargoExtraArgs` will have `cargoClippyExtraArgs` appended to it
   - Default value: `""`
 * `doCheck` is disabled
@@ -316,7 +337,10 @@ Except where noted below, all derivation attributes are delegated to
 `cargoBuild`, and can be used to influence its behavior.
 * `cargoArtifacts` will be instantiated via `buildDepsOnly` if not specified
   - `cargoTarpaulinExtraArgs` will be removed before delegating to `buildDepsOnly`
-* `cargoBuildCommand` will be set to run `cargo tarpaulin` in the workspace.
+* `cargoBuildCommand` will be set to run `cargo tarpaulin --profile release` in
+  the workspace.
+  - `CARGO_PROFILE` can be set on the derivation to alter which cargo profile is
+    selected; setting it to `null` will omit specifying a profile altogether.
 * `cargoExtraArgs` will have `cargoTarpaulinExtraArgs` appended to it
   - Default value: `""`
 * `doCheck` is disabled
@@ -527,6 +551,7 @@ environment variables during the build, you can bring them back via
 The `cargo` package is automatically appended as a native build input to any
 other `nativeBuildInputs` specified by the caller, along with the following
 hooks:
+* `cargoHelperFunctionsHook`
 * `configureCargoCommonVarsHook`
 * `configureCargoVendoredDepsHook`
 * `inheritCargoArtifactsHook`
@@ -725,6 +750,23 @@ lib.writeTOML "foo.toml" { foo.bar = "baz"; }
 
 ## Hooks
 
+### `cargoHelperFunctionsHook`
+
+Defines helper functions for internal use. It is probably not a great idea to
+depend on these directly as their behavior can change at any time, but it is
+worth documenting them just in case:
+
+* Defines a `cargo()` function which will immediately invoke the `cargo` command
+  found on the `$PATH` after echoing the exact arguments that were passed in.
+  Useful for automatically logging all cargo invocations to the log.
+* Defines a `cargoWithProfile()` function which will invoke `cargo` with the
+  provided arguments. If `$CARGO_PROFILE` is set, then `--profile
+  $CARGO_PROFILE` will be injected into the `cargo` invocation
+  - Note: a default value of `$CARGO_PROFILE` is set via
+    `configureCargoCommonVarsHook`. You can set `CARGO_PROFILE = "something"` in
+    your derivation to change which profile is used, or set `CARGO_PROFILE =
+    null;` to omit it altogether.
+
 ### `configureCargoCommonVarsHook`
 
 Defines `configureCargoCommonVars()` which will set various common cargo-related
@@ -734,6 +776,9 @@ incremental artifacts, etc. More specifically:
 * `CARGO_BUILD_JOBS` is set to `$NIX_BUILD_CORES` if not already defined
 * `CARGO_HOME` is set to `$PWD/.cargo-home` if not already defined.
   - The directory that `CARGO_HOME` points to will be created
+* `CARGO_PROFILE` is set to `release` if not already defined.
+  - Note that this is is used internally specify a cargo profile (e.g. `cargo
+    build --profile release`) and not something natively understood by cargo.
 * `RUST_TEST_THREADS` is set to `$NIX_BUILD_CORES` if not already defined
 
 **Automatic behavior:** runs as a post-patch hook

lib/buildDepsOnly.nix

@@ -4,10 +4,10 @@
 , vendorCargoDeps
 }:
 
-{ cargoBuildCommand ? "cargo build --workspace --release"
-, cargoCheckCommand ? "cargo check --workspace --release --all-targets"
+{ cargoBuildCommand ? "cargoWithProfile build"
+, cargoCheckCommand ? "cargoWithProfile check --all-targets"
 , cargoExtraArgs ? ""
-, cargoTestCommand ? "cargo test --workspace --release"
+, cargoTestCommand ? "cargoWithProfile test"
 , ...
 }@args:
 let

lib/buildPackage.nix

@@ -3,7 +3,7 @@
 , lib
 }:
 
-{ cargoBuildCommand ? "cargo build --workspace --release"
+{ cargoBuildCommand ? "cargoWithProfile build"
 , cargoExtraArgs ? ""
 , ...
 }@args:
@@ -13,7 +13,7 @@ let
     ${cargoBuildCommand} --message-format json-render-diagnostics ${cargoExtraArgs} >"$cargoBuildLog"
   '';
 
-  defaultInstallPhaseCommand = ''
+  installPhaseCommand = args.installPhaseCommand or ''
     if [ -n "$cargoBuildLog" -a -f "$cargoBuildLog" ]; then
       installFromCargoBuildLog "$out" "$cargoBuildLog"
     else
@@ -27,20 +27,12 @@ let
         2. ensure that cargo's build log is captured in a file and point $cargoBuildLog at it
         At a minimum, the latter option can be achieved with running:
             cargoBuildLog=$(mktemp cargoBuildLogXXXX.json)
-            cargo build --message-format json-render-diagnostics >"$cargoBuildLog"
+            cargo build --release --message-format json-render-diagnostics >"$cargoBuildLog"
       ''}
 
       false
     fi
   '';
-
-  installPhaseCommand =
-    if args ? installPhaseCommand
-    then ''
-      echo running: ${lib.strings.escapeShellArg args.installPhaseCommand}
-      ${args.installPhaseCommand}
-    ''
-    else defaultInstallPhaseCommand;
 in
 (cargoBuild args).overrideAttrs (old: {
   # NB: we use overrideAttrs here so that our extra additions here do not end up
@@ -53,8 +45,6 @@ in
 
   buildPhase = args.buildPhase or ''
     runHook preBuild
-    cargo --version
-    echo running: ${lib.strings.escapeShellArg buildPhaseCargoCommand}
     ${buildPhaseCargoCommand}
     runHook postBuild
   '';

lib/cargoBuild.nix

@@ -4,8 +4,8 @@
 , vendorCargoDeps
 }:
 
-{ cargoBuildCommand ? "cargo build --workspace --release"
-, cargoTestCommand ? "cargo test --workspace --release"
+{ cargoBuildCommand ? "cargoWithProfile build"
+, cargoTestCommand ? "cargoWithProfile test"
 , cargoExtraArgs ? ""
 , ...
 }@args:

lib/cargoClippy.nix

@@ -14,7 +14,7 @@ cargoBuild (args // {
   inherit cargoArtifacts;
   pnameSuffix = "-clippy";
 
-  cargoBuildCommand = "cargo clippy --workspace --release --all-targets";
+  cargoBuildCommand = "cargoWithProfile clippy --all-targets";
   cargoExtraArgs = "${cargoExtraArgs} ${cargoClippyExtraArgs}";
 
   nativeBuildInputs = (args.nativeBuildInputs or [ ]) ++ [ clippy ];

lib/cargoTarpaulin.nix

@@ -12,7 +12,7 @@ let
 in
 cargoBuild (args // {
   cargoArtifacts = args.cargoArtifacts or (buildDepsOnly args);
-  cargoBuildCommand = "cargo tarpaulin";
+  cargoBuildCommand = "cargoWithProfile tarpaulin";
   cargoExtraArgs = "${cargoExtraArgs} ${cargoTarpaulinExtraArgs}";
 
   doCheck = false;

lib/mkCargoDerivation.nix

@@ -1,4 +1,5 @@
 { cargo
+, cargoHelperFunctionsHook
 , configureCargoCommonVarsHook
 , configureCargoVendoredDepsHook
 , inheritCargoArtifactsHook
@@ -48,6 +49,7 @@ stdenv.mkDerivation (cleanedArgs // {
 
   nativeBuildInputs = (args.nativeBuildInputs or [ ]) ++ [
     cargo
+    cargoHelperFunctionsHook
     configureCargoCommonVarsHook
     configureCargoVendoredDepsHook
     inheritCargoArtifactsHook
@@ -58,21 +60,18 @@ stdenv.mkDerivation (cleanedArgs // {
   buildPhase = args.buildPhase or ''
     runHook preBuild
     cargo --version
-    echo running: ${lib.strings.escapeShellArg buildPhaseCargoCommand}
     ${buildPhaseCargoCommand}
     runHook postBuild
   '';
 
   checkPhase = args.checkPhase or ''
     runHook preCheck
-    echo running: ${lib.strings.escapeShellArg checkPhaseCargoCommand}
     ${checkPhaseCargoCommand}
     runHook postCheck
   '';
 
   installPhase = args.installPhase or ''
     runHook preInstall
-    echo running: ${lib.strings.escapeShellArg installPhaseCommand}
     ${installPhaseCommand}
     runHook postInstall
   '';

lib/vendorGitDeps.nix

@@ -20,7 +20,6 @@ let
   inherit (lib)
     concatMapStrings
     concatStrings
-    escapeShellArg
     groupBy
     hasPrefix
     last

pkgs/cargoHelperFunctions.sh

@@ -0,0 +1,14 @@
+# A shell wrapper which logs any `cargo` invocation
+cargo() {
+  # Run in a subshell to avoid polluting the environment
+  (
+    set -x
+    command cargo "$@"
+  )
+}
+
+# Injects `--profile $CARGO_PROFILE` into a particular cargo invocation
+# if the environment variable is set
+cargoWithProfile() {
+  cargo "${@:1:1}" ${CARGO_PROFILE:+--profile ${CARGO_PROFILE}} "${@:2}"
+}

pkgs/cargoHelperFunctionsHook.nix

@@ -0,0 +1,7 @@
+{ makeSetupHook
+}:
+
+makeSetupHook
+{
+  name = "cargoHelperFunctionsHook";
+} ./cargoHelperFunctions.sh

pkgs/configureCargoCommonVarsHook.sh

@@ -13,6 +13,10 @@ configureCargoCommonVars() {
   # while building with nix. Allow a declared-but-empty variable which will tell
   # cargo to honor the definition used in the build profile
   export CARGO_BUILD_INCREMENTAL=${CARGO_BUILD_INCREMENTAL-false}
+
+  # Used by `cargoWithProfile` to specify a cargo profile to use.
+  # Not exported since it is not natively understood by cargo.
+  CARGO_PROFILE=${CARGO_PROFILE:-release}
 }
 
 # NB: run after patching, but before other configure hooks so that we can set

pkgs/default.nix

@@ -9,6 +9,7 @@
 # etc. scopes).
 callPackage:
 {
+  cargoHelperFunctionsHook = callPackage ./cargoHelperFunctionsHook.nix { };
   configureCargoCommonVarsHook = callPackage ./configureCargoCommonVars.nix { };
   configureCargoVendoredDepsHook = callPackage ./configureCargoVendoredDeps.nix { };
   inheritCargoArtifactsHook = callPackage ./inheritCargoArtifacts.nix { };