Our existing approach with setting mkDerivation attributes to null does not work

In the past, I have seen (and recommended myself) to set mkDerivation attributes (e.g. for cmakeFlags) to null to avoid causing unneeded rebuilds. For example:

  cmakeFlags = if stdenv.isDarwin then [
    "-DCMAKE_SKIP_BUILD_RPATH=OFF"
  ] else null;

using else null instead of else [].

However, I just found out that does not work at all with this common pattern:

cmakeFlags = (args.cmakeFlags or []) ++ [ "-DSTUFF..." ];

because (null or []) ++ [] is null ++ [] which fails with

value is null while a list was expected

An example is to try to biuld the following simple package:

$ nix-shell -p pkgsStatic.murmur
while evaluating the attribute 'cmakeFlags' of the derivation 'libtiff-4.1.0-x86_64-unknown-linux-musl' at pkgs/stdenv/generic/make-derivation.nix:198:11:
value is null while a list was expected, at pkgs/stdenv/adapters.nix:63:22

What do we do about this? What should we recommend right now in PRs?

Can we change the build process so that [] for somethingFlags does not cause the eval to change, the same way as null does?

3 Likes

I’m not sure if I understand the issue.

Can you share an example of something that would be rebuild with the attribute beeing a list but not when it is null?

I always thought that the exact value of attributes isn’t causing any rebuilds, but differences in attributes values compared to what is already there and known.

1 Like

Previously, there was no cmakeFlags attribute. OP added one for Darwin but did not want to cause rebuilds for Linux. It works because attributes containing null are not included in the produced derivation file:

$ nix-instantiate -I nixpkgs=channel:nixos-unstable -E 'with import <nixpkgs> {}; stdenv.mkDerivation { name = "foo"; cmakeFlags = []; }'
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/bh3y6wxzs6jpj9j4z146qmxgshqabxjw-foo.drv
$ nix show-derivation /nix/store/bh3y6wxzs6jpj9j4z146qmxgshqabxjw-foo.drv | rg cmake
      "cmakeFlags": "",
$ nix-instantiate -I nixpkgs=channel:nixos-unstable -E 'with import <nixpkgs> {}; stdenv.mkDerivation { name = "foo"; cmakeFlags = null; }'
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/42aym08amdhzkszaijc14svqk0f83vyp-foo.drv
$ nix show-derivation /nix/store/42aym08amdhzkszaijc14svqk0f83vyp-foo.drv | rg cmake
$ 

Personally, in this case, I would just include the flag on all platforms as I value consistency over avoiding rebuilds. But for flags that we need to include conditionally, I guess there is no way other than filtering known attributes in stdenv.mkDerivation (not sure if there are not some attributes whose even empty presence is meaningful). Well we could also do stdenv.mkDerivation ({ … } // lib.optionalAttrs stdenv.isDarwin { cmakeFlags = [ … ]; }) but that is super ugly and people will forget to add parentheses.

3 Likes

I guess there is no way other than filtering known attributes in stdenv.mkDerivation (not sure if there are not some attributes whose even empty presence is meaningful).

This has occurred to me as well, and I’ve played around with this but always get stuck. I would expect something like this to work:

--- a/pkgs/stdenv/generic/default.nix
+++ b/pkgs/stdenv/generic/default.nix
@@ -148,9 +148,13 @@ let
       # to correct type of machine.
       inherit (hostPlatform) system;
 
-      inherit (import ./make-derivation.nix {
-        inherit lib config stdenv;
-      }) mkDerivation;
+      mkDerivation = let
+        filterArgs = lib.filterAttrs (name: value:
+          name == "version" || name == "targetPrefix" || value != "");
+        mkDerivation' = (import ./make-derivation.nix {
+          inherit lib config stdenv;
+        }).mkDerivation;
+      in attrs: mkDerivation' (filterArgs attrs);

(version and targetPrefix are two such attrs with meaningful emptiness)

I think this is able to build stdenv, but lots of evaluation errors pop up for other packages, usually the dreaded infinite recursion encountered, at undefined position. Maybe someone else will be able to make more progress than me.

1 Like

Okay, I understand. Wouldn’t then something like this help to solve both problems (well, introducing readability issues):

mkDerivation ({
  …
} // (mkIf isDarwin {
  cmakeFlags = [ … ];
}))

That’s probably the best solution when the rebuild must be avoided IMO, even though it’s more complex. Though there’s the potential issue that you can omit the parentheses, and it still evaluates but gives the wrong behavior. Might be a footgun.

$ nix-instantiate -E 'with import <nixpkgs> {}; stdenv.mkDerivation ({ name = "foo"; } // lib.optionalAttrs stdenv.isLinux { cmakeFlags = []; })'
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/48ycr2r4zxqjnb2fjc2hqx02h562f39p-foo.drv
$ nix-instantiate -E 'with import <nixpkgs> {}; stdenv.mkDerivation { name = "foo"; } // lib.optionalAttrs stdenv.isLinux { cmakeFlags = []; }'
warning: you did not specify '--add-root'; the result might be removed by the garbage collector
/nix/store/2lqg85d899w391d7n4argh17khfrk6lp-foo.drv