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>
This commit is contained in:
deliciouslytyped 2021-06-14 15:06:23 +02:00 committed by sterni
parent 204eb98e85
commit a71e906e3a
3 changed files with 122 additions and 2 deletions

View file

@ -116,7 +116,7 @@ rec {
, checkPhase ? "" # syntax checks, e.g. for scripts
}:
runCommand name
{ inherit text executable;
{ inherit text executable checkPhase;
passAsFile = [ "text" ];
# Pointless to do this on a remote machine.
preferLocalBuild = true;
@ -132,7 +132,7 @@ rec {
echo -n "$text" > "$n"
fi
${checkPhase}
eval "$checkPhase"
(test -n "$executable" && chmod +x "$n") || true
'';

View file

@ -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"
''

View file

@ -52,6 +52,7 @@ with pkgs;
cuda = callPackage ./cuda { };
trivial = callPackage ../build-support/trivial-builders/test.nix {};
trivial-overriding = callPackage ../build-support/trivial-builders/test-overriding.nix {};
writers = callPackage ../build-support/writers/test.nix {};
}