__cleanAttrs: Clean packages that don't expose all internals

doc/stdenv/stdenv.chapter.md

@@ -4,7 +4,9 @@ The standard build environment in the Nix Packages collection provides an enviro
 
 ## Using `stdenv` {#sec-using-stdenv}
 
-To build a package with the standard environment, you use the function `stdenv.mkDerivation`, instead of the primitive built-in function `derivation`, e.g.
+To build a package with the standard environment, you use the function `stdenv.mkDerivation`. It calls the built-in `derivation` function for you, and turns its result into a package attribute set.
+
+A minimal invocation looks as follows.
 
 ```nix
 stdenv.mkDerivation {
@@ -362,6 +364,42 @@ Unless set to `false`, some build systems with good support for parallel buildin
 
 ### Special variables {#special-variables}
 
+#### `__cleanAttrs` {#var-__cleanAttrs}
+
+By default, `mkDerivation` will expose its arguments in the returned package attribute set. This is unnecessary and leads to some confusion and doubt.
+
+New packages may pass `__cleanAttrs = true;` to `mkDerivation`, so that it will return a minimal set of package attributes, which package authors can extend via [`passthru`](#var-stdenv-passthru).
+
+Existing packages may be modified to pass `__cleanAttrs = "warn";`, so that the legacy attributes remain available, but they will print a helpful warning when they are accessed. Doing this is best avoided until the packaging function used supports recursively defined arguments like [`mkDerivation` does](#mkderivation-recursive-attributes).
+
+Benefits of `__cleanAttrs`:
+
+ - Users don't have to sift through unnecessary attributes when exploring a package in the `nix repl`.
+ - Users can confidently use non-standard attributes that a package provides.
+ - Package authors know which variables are intended for use by the builder. They can change the builder environment with confidence that they don't break consumers of their package.
+ - It is a little bit more efficient in terms of CPU cycles and memory use.
+
+##### What to do when warned {#warning-package-attr-impl-detail}
+
+If you encounter the warning `The attribute ... of package ... is an implementation detail`, you are invited to help us explicitly support ways in which a package attribute set may used, so that Nixpkgs contributors and users will be aware of your use case.
+
+Ideally the package can add support for whatever is the high level goal you are trying to achieve. This usually involves adding an attribute explicitly to the package attribute set using [`passthru`](#var-stdenv-passthru). Perhaps some commonly applied logic can be added to its value. The public attributes defined in `passthru` can make use of a [recursive package definition](#mkderivation-recursive-attributes) in order to access other parts of the `mkDerivation`-based package in a way that works with `overrideAttrs`.
+
+If you do not know why you got the warning, you may set environment variable `NIX_ABORT_ON_WARN=true` and pass `--show-trace` to Nix. This will cause an evaluation trace to be printed for the location of the warning. From this trace, you can usually derive which expression is responsible for getting the attribute. If you use the `nix-command` experimental feature, you also need to pass `--impure`.
+
+If the attribute can not reasonably be supported by the community, you may use the `internals` attribute to find the same value without the warning. We hope that this will be rare, and if you need `internals` to make some temporary hack bearable, it's perfectly reasonable to use it.
+
+#### `__structuredAttrs` {#var-__structuredAttrs}
+
+When `__structuredAttrs = true` is passed to `mkDerivation`, derivation attributes are serialized in JSON format and made available to the builder through a `.attrs.json` file. This will
+
+ - allow large values to be passed,
+ - eliminate the need for `passAsFile`,
+ - allow for finer grained settings such as [`outputChecks`](https://nixos.org/manual/nix/unstable/language/advanced-attributes.html?highlight=__struct#adv-attr-outputChecks),
+ - turn non-nested arrays into bash arrays rather than a concatenated string.
+
+See [`__structuredAttrs` in the Nix manual](https://nixos.org/manual/nix/unstable/language/advanced-attributes.html#adv-attr-structuredAttrs).
+
 #### `passthru` {#var-stdenv-passthru}
 
 This is an attribute set which can be filled with arbitrary values. For example:

lib/customisation.nix

@@ -1,5 +1,10 @@
 { lib }:
-
+let
+  inherit (lib)
+    isAttrs
+    isString
+    ;
+in
 rec {
 
 
@@ -209,10 +214,19 @@ rec {
       outputToAttrListElement = outputName:
         { name = outputName;
           value = commonAttrs // {
-            inherit (drv.${outputName}) type outputName;
+            type = drv.${outputName}.type or "derivation";
+            inherit outputName;
             outputSpecified = true;
-            drvPath = assert condition; drv.${outputName}.drvPath;
-            outPath = assert condition; drv.${outputName}.outPath;
+            drvPath = assert condition;
+              drv.${outputName}.drvPath or (
+                assert isString drv.drvPath;
+                drv.drvPath
+              );
+            outPath = assert condition;
+              drv.${outputName}.outPath or (
+                assert isString drv.${outputName};
+                drv.${outputName}
+              );
           } //
             # TODO: give the derivation control over the outputs.
             #       `overrideAttrs` may not be the only attribute that needs
@@ -238,7 +252,9 @@ rec {
       outputs = drv.outputs or ["out"];
 
       commonAttrs =
-        { inherit (drv) name system meta; inherit outputs; }
+        { inherit (drv) name meta; inherit outputs;
+          ${if drv?system then "system" else null} = drv.system;
+        }
         // lib.optionalAttrs (drv._hydraAggregate or false) {
           _hydraAggregate = true;
           constituents = map hydraJob (lib.flatten drv.constituents);

nixos/doc/manual/release-notes/rl-2305.section.md

@@ -165,6 +165,10 @@ In addition to numerous new and upgraded packages, this release has the followin
 
 - Pantheon now defaults to Mutter 42 and GNOME settings daemon 42, all Pantheon packages are now tracking elementary OS 7 updates.
 
+- `mkDerivation` does not have to leak implementation details anymore.
+
+  Passing [`__cleanAttrs = true`](https://nixos.org/manual/nixpkgs/unstable/#var-__cleanAttrs) to `mkDerivation` returns a minimal set of package attributes that can be extended via [`passthru`](https://nixos.org/manual/nixpkgs/unstable/#var-stdenv-passthru). This reduces confusion for package users and allows for confident use of non-standard attributes provided by a package. It also gives package authors more confidence in modifying the builder environment without breaking consumers of their package, and it is slightly more efficient.
+
 - The module for the application firewall `opensnitch` got the ability to configure rules. Available as [services.opensnitch.rules](#opt-services.opensnitch.rules)
 
 - The module `usbmuxd` now has the ability to change the package used by the daemon. In case you're experiencing issues with `usbmuxd` you can try an alternative program like `usbmuxd2`. Available as [services.usbmuxd.package](#opt-services.usbmuxd.package)

pkgs/applications/misc/hello/default.nix

@@ -2,7 +2,6 @@
 , lib
 , stdenv
 , fetchurl
-, nixos
 , testers
 , hello
 }:
@@ -20,16 +19,9 @@ stdenv.mkDerivation (finalAttrs: {
 
   passthru.tests = {
     version = testers.testVersion { package = hello; };
-
-    invariant-under-noXlibs =
-      testers.testEqualDerivation
-        "hello must not be rebuilt when environment.noXlibs is set."
-        hello
-        (nixos { environment.noXlibs = true; }).pkgs.hello;
+    run = callPackage ./test.nix { hello = finalAttrs.finalPackage; };
   };
 
-  passthru.tests.run = callPackage ./test.nix { hello = finalAttrs.finalPackage; };
-
   meta = with lib; {
     description = "A program that produces a familiar, friendly greeting";
     longDescription = ''

pkgs/applications/misc/hello/salve-mundi.nix

@@ -0,0 +1,98 @@
+/*
+  This file showcases various techniques and features of Nixpkgs, whereas the
+  regular hello serves as more of an introductory package definition.
+  Salve mundi is latin for hello world.
+*/
+{ callPackage
+, lib
+, stdenv
+, fetchurl
+, nixos
+, testers
+, salve-mundi
+, makeWrapper
+, runCommand
+}:
+
+stdenv.mkDerivation (finalAttrs: {
+  pname = "salve-mundi";
+  version = "2.12.1";
+
+  src = fetchurl {
+    url = "mirror://gnu/hello/hello-${finalAttrs.version}.tar.gz";
+    sha256 = "sha256-jZkUKv2SV28wsM18tCqNxoCZmLxdYH2Idh9RLibH2yA=";
+  };
+
+  doCheck = true;
+
+
+  /* Wrap the program */
+
+  postInstall = lib.optionalString (finalAttrs.passthru.greeting != null) ''
+    wrapProgram $out/bin/hello --append-flags --greeting=${lib.escapeShellArg (lib.escapeShellArg finalAttrs.passthru.greeting)}
+  '';
+  nativeBuildInputs = [ makeWrapper ];
+  passthru.greeting = "SALVE MUNDI";
+
+
+  /* Tidy up the package attributes and make them useful */
+
+  __cleanAttrs = true;
+  passthru.exe = lib.getExe finalAttrs.finalPackage;
+  # We allow `hello.src` to be used in tests and examples, despite __cleanAttrs
+  passthru.src = finalAttrs.src;
+
+
+  /* Strict deps enforce a separation of concerns that also benefits cross compilation
+     and, for shells, shell completions support via nativeBuildInputs */
+
+  strictDeps = true;
+
+
+  /* A fairly extensive suite of extra tests that we like to hold either for
+     the package in the Nixpkgs package set, or even for all possible overrides
+     of the package. */
+
+  passthru.tests = {
+    version = testers.testVersion { package = salve-mundi; };
+
+    # We use Nixpkgs attributes instead of `finalAttrs.finalPackage` here
+    # because overriding is not supported. Running the test on an overridden
+    # finalPackage wouldn't work, and is a bit unnecessary anyway.
+    invariant-under-noXlibs =
+      testers.testEqualDerivation
+        "hello must not be rebuilt when environment.noXlibs is set."
+        salve-mundi
+        (nixos { environment.noXlibs = true; }).pkgs.salve-mundi;
+
+    run = runCommand "salve-mundi-test-run" {
+      nativeBuildInputs = [ finalAttrs.finalPackage ];
+    } ''
+      diff -U3 --color=auto <(hello) <(echo 'SALVE MUNDI')
+      touch $out
+    '';
+
+    restore-default-greeting = callPackage ./test.nix {
+      hello = finalAttrs.finalPackage.overrideAttrs (o: {
+        passthru = o.passthru // {
+          greeting = null;
+        };
+      });
+    };
+  };
+
+  meta = with lib; {
+    description = "A showcase of Nixpkgs features that produces an unfamiliar, archaic greeting";
+    longDescription = ''
+      GNU Hello is a program that prints "Hello, world!" when you run it.
+      It is fully customizable. This package proves it, and showcases some
+      of the fancier things you can do with Nixpkgs.
+    '';
+    mainProgram = "hello";
+    homepage = "https://www.gnu.org/software/hello/manual/";
+    changelog = "https://git.savannah.gnu.org/cgit/hello.git/plain/NEWS?h=v${finalAttrs.version}";
+    license = licenses.gpl3Plus;
+    maintainers = [ maintainers.roberth ];
+    platforms = platforms.all;
+  };
+})

pkgs/stdenv/generic/make-derivation.nix

@@ -161,6 +161,11 @@ let
 # but for anything complex, be prepared to debug if enabling.
 , __structuredAttrs ? config.structuredAttrsByDefault or false
 
+# Opt-in for those who want a clear separation between attributes that should
+# be used and those that are internal. Also expected to perform slightly better
+# when applied widely.
+, __cleanAttrs ? false
+
 , env ? { }
 
 , ... } @ attrs:
@@ -288,6 +293,7 @@ else let
        "nativeCheckInputs" "nativeInstallCheckInputs"
        "__darwinAllowLocalNetworking"
        "__impureHostDeps" "__propagatedImpureHostDeps"
+       "__cleanAttrs"
        "sandboxProfile" "propagatedSandboxProfile"]
        ++ lib.optional (__structuredAttrs || envIsExportable) "env"))
     // (lib.optionalAttrs (attrs ? name || (attrs ? pname && attrs ? version)) {
@@ -542,11 +548,37 @@ else let
         "The ‘env’ attribute set can only contain derivation, string, boolean or integer attributes. The ‘${n}’ attribute is of type ${builtins.typeOf v}."; v)
       env;
 
+  drvAttrs = derivationArg // lib.optionalAttrs envIsExportable checkedEnv;
+  strict = builtins.derivationStrict drvAttrs;
+  outputName = lib.head outputs;
+
+  name = attrs.name or attrs.pname;
+
+  # begin __cleanAttrs helpers
+
+  whatNow =
+    (
+      if meta.maintainers != []
+      then "You may work with the maintainer(s) of ${name}: ${lib.concatMapStringsSep ", " (m: m.github or m.name or m.email) meta.maintainers} (and/or the community), to add explicit support for your use case."
+      else "You may work with the community to add explicit support for your use case to ${name}."
+    ) +
+    " See https://nixos.org/manual/nixpkgs/unstable/#warning-package-attr-impl-detail"
+    ;
+
+  warnCleanAttrs = lib.mapAttrs (k: lib.warn "The attribute ${lib.strings.escapeNixIdentifier k} of package ${name} ${if k == "passthru" then "is" else "seems to be"} an implementation detail and may be removed from the package attribute set in the future. ${whatNow}");
+
+  warnCleanDrvAttrs = lib.warn "The attribute drvAttrs of package ${name} is an implementation detail and may be removed from the package attribute set in the future. ${whatNow}";
+
+  maybeWarnCleanAttrs = if __cleanAttrs == "warn" then warnCleanAttrs else x: x;
+
+  # end __cleanAttrs helpers
+
 in
 
 lib.extendDerivation
   validity.handled
-  ({
+  (
+    lib.optionalAttrs (__cleanAttrs != true) (maybeWarnCleanAttrs {
      # A derivation that always builds successfully and whose runtime
      # dependencies are the original derivations build time dependencies
      # This allows easy building and distributing of all derivations
@@ -577,14 +609,37 @@ lib.extendDerivation
        disallowedReferences = [ ];
        disallowedRequisites = [ ];
      });
-
-     inherit meta passthru overrideAttrs;
-   } //
+     inherit passthru;
+   })
+   // {
+     inherit meta overrideAttrs;
+   }
    # Pass through extra attributes that are not inputs, but
    # should be made available to Nix expressions using the
    # derivation (e.g., in assertions).
-   passthru)
-  (derivation (derivationArg // lib.optionalAttrs envIsExportable checkedEnv));
+   // passthru)
+  (
+      (
+        if __cleanAttrs == true
+        then builtins.intersectAttrs { version = null; } drvAttrs
+        else if __cleanAttrs == "warn"
+        then warnCleanAttrs drvAttrs
+        else drvAttrs
+      )
+      // builtins.listToAttrs (map (outputName: {
+          name = outputName;
+          value = strict.${outputName};
+        }) outputs)
+      // {
+        type = "derivation";
+        inherit outputName outputs;
+        inherit (strict) drvPath;
+        inherit (drvAttrs) name;
+        ${if __cleanAttrs != false then "internals" else "drvAttrs"} = if __cleanAttrs == "warn" then warnCleanDrvAttrs drvAttrs else drvAttrs;
+        ${if __cleanAttrs == "warn" then "drvAttrs" else null} = warnCleanDrvAttrs drvAttrs;
+        outPath = strict.${outputName};
+      }
+  );
 
 in
   fnOrAttrs: