`runCommandWith`: Why is the `passAsFile` attribute defined so circuitously?

Help
1

That is, runCommandWith is defined thus:

(top part of `runCommandWith`) 
  runCommandWith =
    let
      defaultStdenv = stdenv;
    in
    { stdenv ? defaultStdenv
    , runLocal ? false
    , derivationArgs ? {}
    , name
    }: buildCommand:
    stdenv.mkDerivation ({
      enableParallelBuilding = true;
      inherit buildCommand name;

      passAsFile = [ "buildCommand" ]
        ++ (derivationArgs.passAsFile or []);
    }
    // (lib.optionalAttrs runLocal {
          preferLocalBuild = true;
          allowSubstitutes = false;
       })
    // builtins.removeAttrs derivationArgs [ "passAsFile" ]);

I think I get why the builtins.removeAttrs was needed: if derivationArgs would have simply been updated / merged in the end, that the previous passAsFile attribute would have been overwritten (if derivationArgs had a passAsFile attribute, that is).

nix-repl> inputAttributeSet = { a = [ 3 ]; r = "x"; s = "x"; }

nix-repl> :p ({ a = [ 1 2 ] ++ inputAttributeSet.a; b = 3; }
          // inputAttributeSet)
{ a = [ 3 ]; b = 3; r = "x"; s = "x"; }

nix-repl> :p ({ a = [ 1 2 ] ++ inputAttributeSet.a; b = 3; }
          // builtins.removeAttrs inputAttributeSet [ "a" ])
{ a = [ 1 2 3 ]; b = 3; r = "x"; s = "x"; }

1. Is there a reason why passAsFile isn’t simply defined in the end, once?

(top part of `runCommandWith`) 
  runCommandWith =
    let
      defaultStdenv = stdenv;
    in
    { stdenv ? defaultStdenv
    , runLocal ? false
    , derivationArgs ? {}
    , name
    }: buildCommand:
    stdenv.mkDerivation ({
      enableParallelBuilding = true;
      inherit buildCommand name;

    }
    // (lib.optionalAttrs runLocal {
          preferLocalBuild = true;
          allowSubstitutes = false;
       })
    // derivationArgs
    // { passAsFile = [ "buildCommand" ] ++ (derivationArgs.passAsFile or []); }
    );

That is:

nix-repl> :p ({  b = 3; }
          // inputAttributeSet
          // { a = [ 1 2 ] ++ inputAttributeSet.a; })
{ a = [ 1 2 3 ]; b = 3; r = "x"; s = "x"; }

This version looks more readable to me, but then again, I have a tendency to miss obvious things, and this part of Nixpkgs has a lot of complex stuff.

2

Correct.

There often are multiple ways to do things, in this case there are little reasons that could be recited for both variants probably. I think at the time of writing this I followed an established pattern of { … } // builtins.removeAttrs args [ … ] in other wrappers around stdenv.mkDerivation. While in this case you may argue that it is more readable written differently, especially when there’s more going on, it’s much better for readability to have all the custom defined attributes in a single block.

1 Like
3

Thank you for confirming that the two variants are identical in function, and for putting it in historical and architectural context! As for that pattern, I wasn’t aware of it (and, as far as I know, these are conventions that emerged over time and are not documented anywhere).

Readability is a subjective quality (e.g., my preferred style of indentation would probably make others frustrated), and looking up other examples of the “removeAttrs-in-the-end” pattern, I get it now; runCommandWith is just too simple to show its benefits.

I think these are good examples: