about summary refs log tree commit diff
path: root/pkgs/build-support/trivial-builders
diff options
context:
space:
mode:
authordeliciouslytyped <47436522+deliciouslytyped@users.noreply.github.com>2021-06-14 15:06:23 +0200
committersterni <sternenseemann@systemli.org>2021-06-18 01:39:59 +0200
commita71e906e3a0bec9c5fece94262e96de83e58c1f3 (patch)
tree0e8dd8f98769a0d32ca3704cb9983acc389d255d /pkgs/build-support/trivial-builders
parent204eb98e85679f3da6190048730434c3e62bfb90 (diff)
trivial-builders: refactor writeTextFile to be overridable
This fixes #126344, specifically with the goal of enabling overriding the
checkPhase argument. See `design notes` at the end for details.

This allows among other things, enabling bash extension for the `checkPhase`.
Previously using such bash extensions was prohibited by the `writeShellScript`
code because there was no way to enable the extension in the checker.

As an example:

```nix
(writeShellScript "foo" ''
  shopt -s extglob
  echo @(foo|bar)
'').overrideAttrs (old: {
  checkPhase = ''
    # use subshell to preserve outer environment
    (
      export BASHOPTS
      shopt -s extglob
      ${old.checkPhase}
    )
  '';
})
```

This commit also adds tests for this feature to `pkgs/tests/default.nix`,
under `trivial-overriding`. The test code is located at
`pkgs/build-support/trivial-builders/test-overriding.nix`.

Design notes:
-------------

Per discussion with @sternenseemann, the original approach of just wrapping
`writeTextFile` in `makeOverridable` had the issue that combined with `callPackage`
in the following form, would shadow the `.override` attribute of the `writeTextFile`:

```nix
with import <nixpkgs>;
callPackage ({writeShellScript}: writeShellScript "foo" "echo foo")
```

A better approach can be seen in this commit, where `checkPhase` is moved
from an argument of `writeTextFile`, which is substituted into `buildCommand`,
into an `mkDerivation` argument, which is substituted from the environment
and `eval`-ed. (see the source)

This way we can simple use `.overideAttrs` as usual, and this also makes
`checkPhase` a bit more conformant to `mkDerivation` naming, with respect to
phases generally being overridable attrs.

Co-authored-by: sterni <sternenseemann@systemli.org>
Co-authored-by: Naïm Favier <n@monade.li>
Diffstat (limited to 'pkgs/build-support/trivial-builders')
-rw-r--r--pkgs/build-support/trivial-builders/test-overriding.nix119
1 files changed, 119 insertions, 0 deletions
diff --git a/pkgs/build-support/trivial-builders/test-overriding.nix b/pkgs/build-support/trivial-builders/test-overriding.nix
new file mode 100644
index 0000000000000..ddd5dc0507525
--- /dev/null
+++ b/pkgs/build-support/trivial-builders/test-overriding.nix
@@ -0,0 +1,119 @@
+# Check that overriding works for trivial-builders like
+# `writeShellScript` via `overrideAttrs`. This is useful
+# to override the `checkPhase`, e. g. when you want
+# to enable extglob in `writeShellScript`.
+#
+# Run using `nix-build -A tests.trivial-overriding`.
+{ lib
+, runtimeShell
+, runCommand
+, callPackage
+, writeShellScript
+, writeTextFile
+, writeShellScriptBin
+}:
+
+let
+  extglobScript = ''
+    shopt -s extglob
+    touch success
+    echo @(success|failure)
+    rm success
+  '';
+
+  # Reuse the old `checkPhase` of `writeShellScript`, but enable extglob.
+  allowExtglob = old: {
+    checkPhase = ''
+      # make sure we don't change the settings for
+      # the rest of the derivation's build
+      (
+        export BASHOPTS
+        shopt -s extglob
+        ${old.checkPhase}
+      )
+    '';
+  };
+
+  # Run old checkPhase, but only succeed if it fails.
+  # This HACK is required because we can't introspect build failures
+  # in nix: With `assertFail` we want to make sure that the default
+  # `checkPhase` would fail if extglob was used in the script.
+  assertFail = old: {
+    # write old checkPhase into a shell script, so we can check for
+    # the phase to fail even though we have `set -e`.
+    checkPhase = ''
+      if source ${writeShellScript "old-check-phase" old.checkPhase} 2>/dev/null; then
+        exit 1
+      fi
+    '';
+  };
+
+  simpleCase = case:
+    writeShellScript "test-trivial-overriding-${case}" extglobScript;
+
+  callPackageCase = case: callPackage (
+    { writeShellScript }:
+    writeShellScript "test-trivial-callpackage-overriding-${case}" extglobScript
+  ) { };
+
+  binCase = case:
+    writeShellScriptBin "test-trivial-overriding-bin-${case}" extglobScript;
+
+  # building this derivation would fail without overriding
+  textFileCase = writeTextFile {
+    name = "test-trivial-overriding-text-file";
+    checkPhase = "false";
+    text = ''
+      #!${runtimeShell}
+      echo success
+    '';
+    executable = true;
+  };
+
+  mkCase = f: type: isBin:
+    let
+      drv = (f type).overrideAttrs
+        (if type == "succ" then allowExtglob else assertFail);
+    in if isBin then "${drv}/bin/${drv.name}" else drv;
+
+  writeTextOverrides = {
+    # Enabling globbing in checkPhase
+    simpleSucc = mkCase simpleCase "succ" false;
+    # Ensure it's possible to fail; in this case globbing is not enabled.
+    simpleFail = mkCase simpleCase "fail" false;
+    # Do the same checks after wrapping with callPackage
+    # to make sure callPackage doesn't mess with the override
+    callpSucc = mkCase callPackageCase "succ" false;
+    callpFail = mkCase callPackageCase "fail" false;
+    # Do the same check using `writeShellScriptBin`
+    binSucc = mkCase binCase "succ" true;
+    binFail = mkCase binCase "fail" true;
+    # Check that we can also override plain writeTextFile
+    textFileSuccess = textFileCase.overrideAttrs (_: {
+      checkPhase = "true";
+    });
+  };
+
+  # `runTest` forces nix to build the script of our test case and
+  # run its `checkPhase` which is our main interest. Additionally
+  # it executes the script and thus makes sure that extglob also
+  # works at run time.
+  runTest = script:
+    let
+      name = script.name or (builtins.baseNameOf script);
+    in writeShellScript "run-${name}" ''
+      if [ "$(${script})" != "success" ]; then
+        echo "Failed in ${script}"
+        exit 1
+      fi
+    '';
+in
+
+runCommand "test-writeShellScript-overriding" {
+  passthru = { inherit writeTextOverrides; };
+} ''
+  ${lib.concatMapStrings (test: ''
+      ${runTest test}
+    '') (lib.attrValues writeTextOverrides)}
+  touch "$out"
+''