From 75b2c7bf50fe70d03a2df253c0668884cd336be3 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Wed, 9 Feb 2022 13:57:40 +0100
Subject: [PATCH 1/9] lib: Add encapsulate

Creates objects that have overlay-based private attrs in their closure.

(cherry picked from commit 0067cf4fc2ff48a4cf59e961c1917e9bf2faec36)
---
 lib/default.nix      |  2 +-
 lib/fixed-points.nix | 91 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/lib/default.nix b/lib/default.nix
index 4345c48f3bc4c..8becb79f23479 100644
--- a/lib/default.nix
+++ b/lib/default.nix
@@ -80,7 +80,7 @@ let
       genericClosure readFile;
     inherit (self.fixedPoints) fix fix' converge extends composeExtensions
       composeManyExtensions makeExtensible makeExtensibleWithCustomName
-      toExtension;
+      encapsulate toExtension;
     inherit (self.attrsets) attrByPath hasAttrByPath setAttrByPath
       getAttrFromPath attrVals attrNames attrValues getAttrs catAttrs filterAttrs
       filterAttrsRecursive foldlAttrs foldAttrs collect nameValuePair mapAttrs
diff --git a/lib/fixed-points.nix b/lib/fixed-points.nix
index 1de5351d95aac..223c43e18615f 100644
--- a/lib/fixed-points.nix
+++ b/lib/fixed-points.nix
@@ -451,6 +451,97 @@ rec {
       }
     );
 
+
+  /*
+    Creates an overridable attrset with encapsulation.
+
+    This is like `makeExtensible`, but only the `public` attribute of the fixed
+    point is returned.
+
+    Synopsis:
+
+        r = encapsulate (final@{extend, ...}: {
+
+          # ... private attributes for `final` ...
+
+          public = {
+            # ... returned attributes for r, in terms of `final` ...
+            inherit extend; # optional, don't invoke too often; see below
+          };
+        })
+
+        s = r.extend (final: previous: {
+
+          # ... updates to private attributes ...
+
+          # optionally
+          public = previous.public // {
+            # ... updates to public attributes ...
+          };
+        })
+
+    = Performance
+
+    The `extend` function evaluates the whole fixed point all over, reusing
+    no "intermediate results" from the existing object.
+    This is necessary, because `final` has changed.
+    So the cost is quadratic; O(n^2) where n = number of chained invocations.
+    This has consequences for interface design.
+    Although enticing, `extend` is not suitable for directly implementing "fluent interfaces", where the caller makes many calls to `extend` via domain-specific "setters" or `with*` functions.
+    Fluent interfaces can not be implemented efficiently in Nix and have very little to offer over attribute sets in terms of usability.*
+
+    Example:
+
+        # cd nixpkgs; nix repl lib
+
+        nix-repl> multiplier = encapsulate (self: {
+          a = 1;
+          b = 1;
+          public = {
+            r = self.a * self.b;
+
+            # Publishing extend makes the attrset open for any kind of change.
+            inherit (self) extend;
+
+            # Instead, or additionally, you can add domain-specific functions.
+            # Offer a single method with multiple arguments, and not a
+            # "fluent interface" of a method per argument, because all extension
+            # functions are called for every `extend`. See the Performance section.
+            withParams = args@{ a ? null, b ? null }: # NB: defaults are not used
+              self.extend (self: super: args);
+
+          };
+        })
+
+        nix-repl> multiplier
+        { extend = «lambda»; r = 1; withParams =«lambda»; }
+
+        nix-repl> multiplier.withParams { a = 42; b = 10; }
+        { extend = «lambda»; r = 420; withParams =«lambda»; }
+
+        nix-repl> multiplier3 = multiplier.extend (self: super: {
+          c = 1;
+          public = super.public // {
+            r = super.public.r * self.c;
+          };
+        })
+
+        nix-repl> multiplier3.extend (self: super: { a = 2; b = 3; c = 10; })
+        { extend = «lambda»; r = 60; withParams =«lambda»; }
+
+    (*) Final note on Fluent APIs: While the asymptotic complexity can be fixed
+        by avoiding overlay extension or perhaps using it only at the end of the
+        chain only, one problem remains. Every method invocation has to produce
+        a new, immutable state value, which means copying the whole state up to
+        that point.
+
+  */
+  encapsulate = layerZero:
+    let
+      fixed = layerZero ({ extend = f: encapsulate (extends f layerZero); } // fixed);
+    in fixed.public;
+
+
   /**
     Convert to an extending function (overlay).
 

From d690a2290fbf424e605a9ba5d720549b4138c226 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 17 Mar 2024 22:46:23 +0100
Subject: [PATCH 2/9] WIP: add mkPackageWithDeps

This shows the `deps` pattern, merging the following into a
single fixpoint, from https://github.com/NixOS/nixpkgs/issues/273815

 - package function args (`this.deps`)
 - mkDerivation args (`this.setup`)
 - derivation args (`this.drvAttrs`)
 - package attributes (`this.public`)
---
 lib/customisation.nix                       |  12 +-
 pkgs/build-support/package/make-package.nix | 147 ++++++++++++++++++++
 pkgs/by-name/he/hello/package.nix           | 113 +++++++--------
 pkgs/stdenv/generic/default.nix             |   5 +
 pkgs/stdenv/generic/make-derivation.nix     |   1 +
 pkgs/top-level/all-packages.nix             |   6 +
 6 files changed, 224 insertions(+), 60 deletions(-)
 create mode 100644 pkgs/build-support/package/make-package.nix

diff --git a/lib/customisation.nix b/lib/customisation.nix
index be6585d5fa6e4..2257683c9197f 100644
--- a/lib/customisation.nix
+++ b/lib/customisation.nix
@@ -170,7 +170,17 @@ rec {
       in
       if isAttrs result then
         result
-        // {
+        # Overriding `overrideDerivation` and `overrideAttrs` is a hack:
+        # `callPackage` shouldn't have to be aware of those functions and then
+        # fix them up, but it has to because the package arguments aren't part of
+        # the package fixpoint in `mkDerivation`.
+        #
+        # If the original package defines its own `override` method, but not
+        # `overrideDerivation`, we can assume it's a callPackage-aware `mkPackage`
+        # call.
+        # Otherwise, it is a legacy package, for which we still need to perform
+        # the hacky fixup.
+        // optionalAttrs (! (result ? override) || (result ? overrideDerivation)) {
           override = overrideArgs;
           overrideDerivation = fdrv: overrideResult (x: overrideDerivation x fdrv);
           ${if result ? overrideAttrs then "overrideAttrs" else null} =
diff --git a/pkgs/build-support/package/make-package.nix b/pkgs/build-support/package/make-package.nix
new file mode 100644
index 0000000000000..fe8196780c40a
--- /dev/null
+++ b/pkgs/build-support/package/make-package.nix
@@ -0,0 +1,147 @@
+{ lib, callPackage, stdenv, config, ... }:
+let
+  checkMeta = import ../../stdenv/generic/check-meta.nix {
+    inherit lib config;
+    # Nix itself uses the `system` field of a derivation to decide where
+    # to build it. This is a bit confusing for cross compilation.
+    inherit (stdenv) hostPlatform;
+  };
+
+  baseLayer = this: {
+    pos = builtins.unsafeGetAttrPos "name" this;
+    # TODO: drvAttrs won't be available in RFC 92 dynamic derivations or multi-derivation packages.
+    validity = checkMeta.assertValidity { inherit (this.public) meta; attrs = this.drvAttrs // { inherit (this) meta; }; };
+
+    # In the repl, use :p pkg.help
+    # Since https://github.com/NixOS/nix/pull/10208
+    help = if this.deps == {} then ''
+      # Overriding dependencies
+
+      This package does not have overridable dependencies using the .override attribute.
+
+    '' else ''
+      # Overriding dependencies
+
+      This package allows its dependencies to be overridden, using the .override
+      attribute. For example:
+
+          pkg.override (old: { dep = f old.dep; })
+
+      Instead of dep, you may set the following attributes:
+
+          ${lib.concatMapStringsSep ", " lib.strings.escapeNixIdentifier (lib.attrNames this.deps)}
+    '';
+
+    deps = {};
+
+    public = builtins.intersectAttrs {
+      tests = null;
+      version = null;
+    } this // {
+      /*
+        The marker for a [package attribute set](https://nixos.org/manual/nix/stable/glossary.html#package-attribute-set).
+        The value is "derivation" for historical reasons.
+      */
+      type = "derivation";
+
+      # FIXME: this assumes this.drvAttrs, which is a bad dependency on the
+      #        derivation layer
+      meta = checkMeta.commonMeta {
+        inherit (this) validity pos;
+        attrs = this.drvAttrs // { inherit (this) meta; };
+        references = this.drvAttrs.nativeBuildInputs or []
+          ++ this.drvAttrs.buildInputs or []
+          ++ this.drvAttrs.propagatedNativeBuildInputs or []
+          ++ this.drvAttrs.propagatedBuildInputs or [];
+      };
+
+      inherit (this) help name;
+
+      override = f: this.extend (this: old: {
+        deps = old.deps // f old.deps;
+      });
+
+      overrideAttrs = f:
+        this.extend (this: old:
+          let
+            r = f (old.setup or {});
+            r' =
+              if lib.isFunction r
+              then
+                # f = finalAttrs: prevAttrs: ...
+                f
+                  (this.setup or {} // {
+                    finalPackage = this.public;
+                  })
+                  old.setup or {}
+              else r;
+          in
+          {
+            setup = old.setup // r';
+          }
+        );
+    };
+  };
+
+  layers.derivation = { stdenv, ... }: this: old:
+  let
+    outputs = lib.genAttrs (this.drvAttrs.outputs) (outputName:
+      this.public // {
+        outPath = assert this.validity.handled; this.drvOutAttrs.${outputName};
+        inherit outputName;
+        outputSpecified = true;
+      }
+    );
+  in
+  {
+    drvAttrs = stdenv.makeDerivationArgument (
+      (if this ? version then {
+        pname = this.name;
+        inherit (this) version;
+      }
+      else {
+        inherit (this) name;
+      }) // this.setup
+    );
+    drvOutAttrs = builtins.derivationStrict this.drvAttrs;
+    public =
+      old.public
+      // rec {
+        outPath = assert this.validity.handled; this.drvOutAttrs.${outputName};
+        outputName = lib.head this.drvAttrs.outputs;
+        # legacy attribute for single-drv packages
+        drvPath = assert this.validity.handled; this.drvOutAttrs.drvPath;
+      }
+      // outputs;
+  };
+
+  layers.withDeps = f: this: old:
+    let
+      spy = lib.setFunctionArgs (args: { inherit args; }) (lib.functionArgs f);
+      # TODO: make callPackage a parameter?
+      values = (callPackage spy { }).args;
+      old2 = old // {
+        deps = old.deps or {} // values;
+      };
+      r = f (lib.mapAttrs (name: value: this.deps.${name}) values);
+      r' = if lib.isList r then lib.composeManyExtensions r else r;
+    in
+      old2 //
+      r' this old2;
+      # lib.composeExtensions f ({ })
+
+  # TODO: layers.meson
+  # TODO: layers.cmake
+  # TODO: layers.pkg-config
+  # TODO: layers.<some language>
+
+  mkPackage = f: lib.encapsulate (lib.extends f baseLayer);
+  mkPackageWithDeps = f: mkPackage (layers.withDeps f);
+in
+{
+  inherit
+    layers
+    mkPackage
+    mkPackageWithDeps
+    ;
+}
diff --git a/pkgs/by-name/he/hello/package.nix b/pkgs/by-name/he/hello/package.nix
index 0590131913f46..79fec0faf5b70 100644
--- a/pkgs/by-name/he/hello/package.nix
+++ b/pkgs/by-name/he/hello/package.nix
@@ -1,59 +1,54 @@
-{
-  callPackage,
-  lib,
-  stdenv,
-  fetchurl,
-  nixos,
-  testers,
-  versionCheckHook,
-  hello,
-}:
-
-stdenv.mkDerivation (finalAttrs: {
-  pname = "hello";
-  version = "2.12.1";
-
-  src = fetchurl {
-    url = "mirror://gnu/hello/hello-${finalAttrs.version}.tar.gz";
-    hash = "sha256-jZkUKv2SV28wsM18tCqNxoCZmLxdYH2Idh9RLibH2yA=";
-  };
-
-  # The GNU Hello `configure` script detects how to link libiconv but fails to actually make use of that.
-  # Unfortunately, this cannot be a patch to `Makefile.am` because `autoreconfHook` causes a gettext
-  # infrastructure mismatch error when trying to build `hello`.
-  env = lib.optionalAttrs stdenv.hostPlatform.isDarwin {
-    NIX_LDFLAGS = "-liconv";
-  };
-
-  doCheck = true;
-
-  doInstallCheck = true;
-  nativeInstallCheckInputs = [
-    versionCheckHook
-  ];
-
-  # Give hello some install checks for testing purpose.
-  postInstallCheck = ''
-    stat "''${!outputBin}/bin/${finalAttrs.meta.mainProgram}"
-  '';
-
-  passthru.tests = {
-    version = testers.testVersion { package = hello; };
-  };
-
-  passthru.tests.run = callPackage ./test.nix { hello = finalAttrs.finalPackage; };
-
-  meta = {
-    description = "Program that produces a familiar, friendly greeting";
-    longDescription = ''
-      GNU Hello is a program that prints "Hello, world!" when you run it.
-      It is fully customizable.
-    '';
-    homepage = "https://www.gnu.org/software/hello/manual/";
-    changelog = "https://git.savannah.gnu.org/cgit/hello.git/plain/NEWS?h=v${finalAttrs.version}";
-    license = lib.licenses.gpl3Plus;
-    maintainers = with lib.maintainers; [ stv0g ];
-    mainProgram = "hello";
-    platforms = lib.platforms.all;
-  };
-})
+{ mkPackageWithDeps, layers }: mkPackageWithDeps ({ stdenv, fetchurl, testers, lib, callPackage, versionCheckHook }: [
+  (layers.derivation { inherit stdenv; })
+  (this: old: {
+    name = "hello";
+    version = "2.12.1";
+
+    setup = old.setup or {} // {
+      src = fetchurl {
+        url = "mirror://gnu/hello/hello-${this.version}.tar.gz";
+        hash = "sha256-jZkUKv2SV28wsM18tCqNxoCZmLxdYH2Idh9RLibH2yA=";
+      };
+
+      doCheck = true;
+      doInstallCheck = true;
+      nativeInstallCheckInputs = [
+        versionCheckHook
+      ];
+
+      # Give hello some install checks for testing purpose.
+      postInstallCheck = ''
+        stat "''${!outputBin}/bin/${this.meta.mainProgram}"
+      '';
+    };
+
+    # The GNU Hello `configure` script detects how to link libiconv but fails to actually make use of that.
+    # Unfortunately, this cannot be a patch to `Makefile.am` because `autoreconfHook` causes a gettext
+    # infrastructure mismatch error when trying to build `hello`.
+    env = lib.optionalAttrs stdenv.hostPlatform.isDarwin {
+      NIX_LDFLAGS = "-liconv";
+    };
+
+    public = old.public // {
+      tests = {
+        version = testers.testVersion { package = this.public; };
+
+        run = callPackage ./test.nix { hello = this.public; };
+      };
+    };
+
+    meta = {
+      description = "Program that produces a familiar, friendly greeting";
+      longDescription = ''
+        GNU Hello is a program that prints "Hello, world!" when you run it.
+        It is fully customizable.
+      '';
+      homepage = "https://www.gnu.org/software/hello/manual/";
+      changelog = "https://git.savannah.gnu.org/cgit/hello.git/plain/NEWS?h=v${this.version}";
+      license = lib.licenses.gpl3Plus;
+      maintainers = with lib.maintainers; [ stv0g ];
+      mainProgram = "hello";
+      platforms = lib.platforms.all;
+    };
+  })
+])
diff --git a/pkgs/stdenv/generic/default.nix b/pkgs/stdenv/generic/default.nix
index fe9843c6b1202..7720ad82ce939 100644
--- a/pkgs/stdenv/generic/default.nix
+++ b/pkgs/stdenv/generic/default.nix
@@ -63,6 +63,9 @@ let
       # This is convient to have as a parameter so the stdenv "adapters" work better
       mkDerivationFromStdenv ?
         stdenv: (import ./make-derivation.nix { inherit lib config; } stdenv).mkDerivation,
+
+      # TODO: unify with the above
+      makeDerivationArgumentFromStdenv ? stdenv: (import ./make-derivation.nix { inherit lib config; } stdenv).makeDerivationArgument
     }:
 
     let
@@ -201,6 +204,8 @@ let
 
       mkDerivation = mkDerivationFromStdenv stdenv;
 
+      makeDerivationArgument = makeDerivationArgumentFromStdenv stdenv;
+
       inherit fetchurlBoot;
 
       inherit overrides;
diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix
index 76c2cd3da4be1..65b3ff5c652a5 100644
--- a/pkgs/stdenv/generic/make-derivation.nix
+++ b/pkgs/stdenv/generic/make-derivation.nix
@@ -677,4 +677,5 @@ extendDerivation
 in
 {
   inherit mkDerivation;
+  inherit makeDerivationArgument;
 }
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index f3042065516bd..ed8adaa7fbc54 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -115,6 +115,12 @@ with pkgs;
       import ./pkg-config/defaultPkgConfigPackages.nix pkgs
     ) // { __attrsFailEvaluation = true; };
 
+  inherit (import ../build-support/package/make-package.nix { inherit callPackage config lib stdenv; })
+    layers
+    mkPackage
+    mkPackageWithDeps
+    ;
+
   ### Nixpkgs maintainer tools
 
   nix-generate-from-cpan = callPackage ../../maintainers/scripts/nix-generate-from-cpan.nix { };

From 2aef03b7ee54b103a09615ab2ca8e3bcca337a8d Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 14 Apr 2024 17:52:54 +0200
Subject: [PATCH 3/9] WIP: Allow non-pkgs deps (needs test)

Demo:

```diff
diff --git a/pkgs/by-name/he/hello/package.nix b/pkgs/by-name/he/hello/package.nix
index 0cf0ae493b72..69921e14ffc6 100644
--- a/pkgs/by-name/he/hello/package.nix
+++ b/pkgs/by-name/he/hello/package.nix
@@ -1,9 +1,13 @@
-{ mkPackageWithDeps, layers }: mkPackageWithDeps ({ stdenv, fetchurl, testers, lib, callPackage, nixos }: [
+{ mkPackageWithDeps, layers }: mkPackageWithDeps ({ stdenv, fetchurl, testers, lib, callPackage, nixos, haskellPackages, omnidoc }: [
   (layers.derivation { inherit stdenv; })
   (this: old: {
     name = "hello";
     version = "2.12.1";

+    deps = old.deps // {
+      omnidoc = haskellPackages.pandoc;
+    };
+
     setup = old.setup or {} // {
       src = fetchurl {
         url = "mirror://gnu/hello/hello-${this.version}.tar.gz";
@@ -24,6 +28,10 @@
       run = callPackage ./test.nix { hello = this.public; };
     };

+    public = old.public // {  # passthru, just to show that it works
+      inherit omnidoc;
+    };
+
     meta = with lib; {
       description = "A program that produces a familiar, friendly greeting";
       longDescription = ''
```
---
 pkgs/build-support/package/make-package.nix | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/pkgs/build-support/package/make-package.nix b/pkgs/build-support/package/make-package.nix
index fe8196780c40a..80d6e42f7a5f7 100644
--- a/pkgs/build-support/package/make-package.nix
+++ b/pkgs/build-support/package/make-package.nix
@@ -117,13 +117,21 @@ let
 
   layers.withDeps = f: this: old:
     let
-      spy = lib.setFunctionArgs (args: { inherit args; }) (lib.functionArgs f);
+      fargs = lib.functionArgs f;
+      spy = lib.setFunctionArgs
+        (args: { inherit args; })
+        (lib.mapAttrs
+          (_k: _v:
+            # say we have a default, so that unknown attrs aren't a problem and
+            # they can be defined by a subsequent layer.
+            true)
+          fargs);
       # TODO: make callPackage a parameter?
       values = (callPackage spy { }).args;
       old2 = old // {
         deps = old.deps or {} // values;
       };
-      r = f (lib.mapAttrs (name: value: this.deps.${name}) values);
+      r = f (lib.mapAttrs (name: value: this.deps.${name}) fargs);
       r' = if lib.isList r then lib.composeManyExtensions r else r;
     in
       old2 //

From 3908faf34f4e5d961b7d081d74ea9262c7efd3ca Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Tue, 14 Jan 2025 05:43:23 +0100
Subject: [PATCH 4/9] Make layers overridable

This also makes the old `mkPackageWithDeps` style the default, and
improves error reporting a bit.
---
 pkgs/build-support/package/make-package.nix | 69 +++++++++++++++++++--
 pkgs/by-name/he/hello/package.nix           |  2 +-
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/pkgs/build-support/package/make-package.nix b/pkgs/build-support/package/make-package.nix
index 80d6e42f7a5f7..15758a7002b04 100644
--- a/pkgs/build-support/package/make-package.nix
+++ b/pkgs/build-support/package/make-package.nix
@@ -61,6 +61,8 @@ let
         deps = old.deps // f old.deps;
       });
 
+      # FIXME: Use `toExtension f`?
+      # TODO: Support legacy attrs like passthru?
       overrideAttrs = f:
         this.extend (this: old:
           let
@@ -115,7 +117,7 @@ let
       // outputs;
   };
 
-  layers.withDeps = f: this: old:
+  layers.withDeps = f: externalArgs: this: old:
     let
       fargs = lib.functionArgs f;
       spy = lib.setFunctionArgs
@@ -127,11 +129,25 @@ let
             true)
           fargs);
       # TODO: make callPackage a parameter?
-      values = (callPackage spy { }).args;
+      values = (callPackage spy externalArgs).args;
       old2 = old // {
         deps = old.deps or {} // values;
+        inherit values;
       };
-      r = f (lib.mapAttrs (name: value: this.deps.${name}) fargs);
+      r =
+        f
+          (lib.mapAttrs
+            (name: hasDefault:
+              builtins.addErrorContext
+                "while evaluating the package function argument `${name}`"
+                (externalArgs.${name} or (
+                  this.deps.${name} or (
+                    throw "Dependency ${name} went missing from the package internal `deps` attribute. Did you forget to preserve previous deps? Write e.g. `deps = prev.deps // { ... }`"
+                  )
+                ))
+            )
+            fargs
+          );
       r' = if lib.isList r then lib.composeManyExtensions r else r;
     in
       old2 //
@@ -143,13 +159,54 @@ let
   # TODO: layers.pkg-config
   # TODO: layers.<some language>
 
-  mkPackage = f: lib.encapsulate (lib.extends f baseLayer);
-  mkPackageWithDeps = f: mkPackage (layers.withDeps f);
+  layers.noop = _this: _old: { };
+
+  mkPackageWith = {
+    /* these are not overridable by the layer implementations - not suited for `deps` */
+    externalDeps ? { inherit layers; }
+  }: f: lib.encapsulate (
+    this:
+      let baseLayer' = x: baseLayer x // {
+        /**
+          Extend the package layers with the given function.
+         */
+        extend = f: this.extend (this: old: {
+          userLayer = lib.composeExtensions old.userLayer f;
+        });
+      };
+      in
+      # The root of the mkPackage fixpoint is responsible for managing the deps,
+      # and combining the layers (without adding an extra fixpoint).
+      # Virtually all package logic happens in userLayer.
+      {
+        userLayer = final: prev:
+          this.externalDeps.layers.withDeps
+            f
+            (this.externalDeps // {
+              # Package attributes
+              inherit final prev;
+              # Dependencies
+              /**
+                Override the package dependencies that are not overridable by the individual layer implementations,
+                Notably, the `layers` attribute.
+               */
+              overrideExternalDeps = newDeps:
+                this.extend (this: old: {
+                  externalDeps = old.externalDeps // newDeps;
+                });
+            })
+            final
+            prev;
+        inherit externalDeps;
+        package = lib.extends this.userLayer baseLayer' this.package;
+        inherit (this.package) public;
+      }
+  );
+  mkPackage = mkPackageWith {};
 in
 {
   inherit
     layers
     mkPackage
-    mkPackageWithDeps
     ;
 }
diff --git a/pkgs/by-name/he/hello/package.nix b/pkgs/by-name/he/hello/package.nix
index 79fec0faf5b70..ae39b33edb015 100644
--- a/pkgs/by-name/he/hello/package.nix
+++ b/pkgs/by-name/he/hello/package.nix
@@ -1,4 +1,4 @@
-{ mkPackageWithDeps, layers }: mkPackageWithDeps ({ stdenv, fetchurl, testers, lib, callPackage, versionCheckHook }: [
+{ mkPackage }: mkPackage ({ stdenv, fetchurl, testers, layers, lib, callPackage, versionCheckHook }: [
   (layers.derivation { inherit stdenv; })
   (this: old: {
     name = "hello";

From c1875461a93730cc8d1f55afae82e29f0edb4259 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Tue, 14 Jan 2025 09:00:13 +0100
Subject: [PATCH 5/9] callPackage: Special treatment for mkPackage

The solution inside makeOverridable was fragile, and did not catch
the potential issue that the new code reports.

It's still not super pretty, but architecturally it won't get better
than this because `by-name/**/package.nix` is tied to `callPackage`.

Alternative solution: require a different file name, so that
`callPackage` can be avoided.
---
 lib/customisation.nix | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/lib/customisation.nix b/lib/customisation.nix
index 2257683c9197f..e1e45f973ddcc 100644
--- a/lib/customisation.nix
+++ b/lib/customisation.nix
@@ -170,17 +170,7 @@ rec {
       in
       if isAttrs result then
         result
-        # Overriding `overrideDerivation` and `overrideAttrs` is a hack:
-        # `callPackage` shouldn't have to be aware of those functions and then
-        # fix them up, but it has to because the package arguments aren't part of
-        # the package fixpoint in `mkDerivation`.
-        #
-        # If the original package defines its own `override` method, but not
-        # `overrideDerivation`, we can assume it's a callPackage-aware `mkPackage`
-        # call.
-        # Otherwise, it is a legacy package, for which we still need to perform
-        # the hacky fixup.
-        // optionalAttrs (! (result ? override) || (result ? overrideDerivation)) {
+        // {
           override = overrideArgs;
           overrideDerivation = fdrv: overrideResult (x: overrideDerivation x fdrv);
           ${if result ? overrideAttrs then "overrideAttrs" else null} =
@@ -311,7 +301,22 @@ rec {
 
     in
     if missingArgs == { } then
-      makeOverridable f allArgs
+      if fargs?mkPackage then
+        if fargs != { mkPackage = false; } then
+          throw ''
+            callPackage: A package function that uses `mkPackage` must only have `{ mkPackage }:` as its arguments.
+            Any other dependencies should be taken from the `mkPackage` callback.
+            Otherwise, such dependencies will not be overridable.
+
+            To illustrate, a dependency `foo` can be retrieved with:
+            { mkPackage }: mkPackage ({ layers, foo }: [
+              ...
+            ])
+          ''
+        else
+          f allArgs
+      else
+        makeOverridable f allArgs
     # This needs to be an abort so it can't be caught with `builtins.tryEval`,
     # which is used by nix-env and ofborg to filter out packages that don't evaluate.
     # This way we're forced to fix such errors in Nixpkgs,

From 70d979865ab10e80ab1042c39023bbd872393f84 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Wed, 15 Jan 2025 08:45:50 +0100
Subject: [PATCH 6/9] layers.buildNpmPackage: init

---
 .../node/build-npm-package/layer.nix          | 67 +++++++++++++++++++
 pkgs/build-support/package/make-package.nix   |  2 +
 2 files changed, 69 insertions(+)
 create mode 100644 pkgs/build-support/node/build-npm-package/layer.nix

diff --git a/pkgs/build-support/node/build-npm-package/layer.nix b/pkgs/build-support/node/build-npm-package/layer.nix
new file mode 100644
index 0000000000000..ff465f543b924
--- /dev/null
+++ b/pkgs/build-support/node/build-npm-package/layer.nix
@@ -0,0 +1,67 @@
+/**
+  `mkPackage` layer for building NPM packages.
+ */
+{
+  lib,
+  stdenv,
+  fetchNpmDeps,
+  buildPackages,
+  nodejs,
+  cctools,
+}:
+
+let
+  # The fetcher needs the unpack related attributes
+  fetchInherited = {
+    src = null;
+    srcs = null;
+    sourceRoot = null;
+    prePatch = null;
+    patches = null;
+    postPatch = null;
+    patchFlags = null;
+  };
+in
+this: old:
+let inherit (this) deps name version; in {
+  deps = {
+    inherit (deps.nodejs) fetchNpmDeps;
+    npmHooks = buildPackages.npmHooks.override {
+      inherit (deps) nodejs;
+    };
+    inherit (deps.npmHooks) npmConfigHook npmBuildHook npmInstallHook;
+  } // old.deps;
+  /** Arguments for fetchNpmDeps */
+  npmFetch = {
+    name = "${name}-${version}-npm-deps";
+    hash = throw "Please specify npmFetch.hash in the package definition of ${name}";
+    forceEmptyCache = false;
+    forceGitDeps = false;
+    patchFlags = "";
+    postPatch = "";
+    prePatch = "";
+  } // builtins.intersectAttrs fetchInherited this.setup // old.npmFetch;
+  setup = old.setup or {} // {
+    inherit (deps) nodejs;
+    npmDeps = fetchNpmDeps this.npmFetch;
+    npmPruneFlags = old.setup.npmPruneFlags or this.setup.npmInstallFlags or [];
+    npmBuildScript = "build";
+    nativeBuildInputs =
+      old.setup.nativeBuildInputs
+      ++ [
+        deps.nodejs
+        deps.npmConfigHook
+        deps.npmBuildHook
+        deps.npmInstallHook
+        deps.nodejs.python
+      ]
+      ++ lib.optionals stdenv.hostPlatform.isDarwin [ cctools ];
+    buildInputs = old.setup.buildInputs or [] ++ [ deps.nodejs ];
+    strictDeps = true;
+    # Stripping takes way too long with the amount of files required by a typical Node.js project.
+    dontStrip = old.dontStrip or true;
+  };
+  meta = (old.meta or { }) // {
+    platforms = old.meta.platforms or deps.nodejs.meta.platforms;
+  };
+}
diff --git a/pkgs/build-support/package/make-package.nix b/pkgs/build-support/package/make-package.nix
index 15758a7002b04..6a370f07c974d 100644
--- a/pkgs/build-support/package/make-package.nix
+++ b/pkgs/build-support/package/make-package.nix
@@ -161,6 +161,8 @@ let
 
   layers.noop = _this: _old: { };
 
+  layers.buildNpmPackage = callPackage ../node/build-npm-package/layer.nix { };
+
   mkPackageWith = {
     /* these are not overridable by the layer implementations - not suited for `deps` */
     externalDeps ? { inherit layers; }

From 5cbca85b9b3d74b7da01f8390792cd7a10dccb2a Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Wed, 15 Jan 2025 08:49:17 +0100
Subject: [PATCH 7/9] netlify-cli: Use mkPackage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This allows the all-packages.nix entry to be removed
when the package goes to pkgs/by-name 🎉
---
 pkgs/development/web/netlify-cli/default.nix | 77 ++++++++++----------
 pkgs/top-level/all-packages.nix              |  4 +-
 2 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/pkgs/development/web/netlify-cli/default.nix b/pkgs/development/web/netlify-cli/default.nix
index e5f3686842005..4187c61ecfe64 100644
--- a/pkgs/development/web/netlify-cli/default.nix
+++ b/pkgs/development/web/netlify-cli/default.nix
@@ -1,43 +1,44 @@
-{
-  buildNpmPackage,
-  callPackage,
-  fetchFromGitHub,
-  lib,
-  nix-update-script,
-  nodejs,
-  pkg-config,
-  vips,
-}:
+{ mkPackage }: mkPackage (
+{ lib, layers, stdenv, fetchFromGitHub, nix-update-script, pkg-config, vips, callPackage, testers, nodejs_20, ... }:
+[
+  (layers.derivation { inherit stdenv; })
+  (this: old: {
+    name = "netlify-cli";
+    version = "17.38.0";
 
-buildNpmPackage rec {
-  pname = "netlify-cli";
-  version = "17.38.0";
+    deps = old.deps // {
+      nodejs = nodejs_20;
+    };
 
-  src = fetchFromGitHub {
-    owner = "netlify";
-    repo = "cli";
-    tag = "v${version}";
-    hash = "sha256-fK+Z6bqnaqSYXgO0lUbGALZeCiAnvMd6LkMSH7JB7J8=";
-  };
+    npmFetch.hash = "sha256-oFt+l8CigOtm3W5kiT0kFsqKLOJB9ggfiFQgUU5xQ1I=";
 
-  npmDepsHash = "sha256-oFt+l8CigOtm3W5kiT0kFsqKLOJB9ggfiFQgUU5xQ1I=";
+    setup = {
+      buildInputs = [ vips ];
+      nativeBuildInputs = [ pkg-config ];
+      src = fetchFromGitHub {
+        owner = "netlify";
+        repo = "cli";
+        tag = "v${this.version}";
+        hash = "sha256-fK+Z6bqnaqSYXgO0lUbGALZeCiAnvMd6LkMSH7JB7J8=";
+      };
+    };
 
-  inherit nodejs;
+    public = old.public // {
+      tests = {
+        version = testers.testVersion { package = this.public; };
+        run = callPackage ./test.nix { netlify-cli = this.public; };
+      };
+      updateScript = nix-update-script { };
+    };
 
-  buildInputs = [ vips ];
-  nativeBuildInputs = [ pkg-config ];
-
-  passthru = {
-    tests.test = callPackage ./test.nix { };
-    updateScript = nix-update-script { };
-  };
-
-  meta = {
-    description = "Netlify command line tool";
-    homepage = "https://github.com/netlify/cli";
-    changelog = "https://github.com/netlify/cli/blob/v${version}/CHANGELOG.md";
-    license = lib.licenses.mit;
-    maintainers = with lib.maintainers; [ roberth ];
-    mainProgram = "netlify";
-  };
-}
+    meta = {
+      description = "Netlify command line tool";
+      homepage = "https://github.com/netlify/cli";
+      changelog = "https://github.com/netlify/cli/blob/v${this.version}/CHANGELOG.md";
+      license = lib.licenses.mit;
+      maintainers = with lib.maintainers; [ roberth ];
+      mainProgram = "netlify";
+    };
+  })
+  layers.buildNpmPackage
+])
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index ed8adaa7fbc54..08fed3a80bc7d 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -4357,9 +4357,7 @@ with pkgs;
     };
   });
 
-  netlify-cli = callPackage ../development/web/netlify-cli {
-    nodejs = nodejs_20;
-  };
+  netlify-cli = callPackage ../development/web/netlify-cli { };
 
   netpbm = callPackage ../tools/graphics/netpbm { };
 

From a2c1e46a66d9a155aa9a7a1cad7de5fd7cd1dafb Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Wed, 15 Jan 2025 08:53:16 +0100
Subject: [PATCH 8/9] mkPackage: Expose fixpoint root as .internals

Giving no access at all would be really annoying for debugging.

Unlike `mkDerivation`, which makes no distinction between intentional
and internal package attributes, this gives a signal that when you're
using it, you are more likely to have to change your code at some point.
(besides keeping the public package attrs _relevant_)
---
 pkgs/build-support/package/make-package.nix | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pkgs/build-support/package/make-package.nix b/pkgs/build-support/package/make-package.nix
index 6a370f07c974d..8b19e1ee344bd 100644
--- a/pkgs/build-support/package/make-package.nix
+++ b/pkgs/build-support/package/make-package.nix
@@ -61,6 +61,8 @@ let
         deps = old.deps // f old.deps;
       });
 
+      internals = this;
+
       # FIXME: Use `toExtension f`?
       # TODO: Support legacy attrs like passthru?
       overrideAttrs = f:

From b88cfe5f7f864427daacde3922b3cc4a596c8daa Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Wed, 15 Jan 2025 09:07:16 +0100
Subject: [PATCH 9/9] callPackage: Add comment about callPackage/mkPackage
 usage

---
 lib/customisation.nix | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/customisation.nix b/lib/customisation.nix
index e1e45f973ddcc..0227fafe253c0 100644
--- a/lib/customisation.nix
+++ b/lib/customisation.nix
@@ -303,6 +303,10 @@ rec {
     if missingArgs == { } then
       if fargs?mkPackage then
         if fargs != { mkPackage = false; } then
+          # If your file is e.g. an attrset with a package and something else,
+          # use `{ callPackage }: { foo = callPackage ({ mkPackage}: ...); ... }`
+          # Explaining that in the error message would be too verbose and confusing.
+          # TODO: link to docs
           throw ''
             callPackage: A package function that uses `mkPackage` must only have `{ mkPackage }:` as its arguments.
             Any other dependencies should be taken from the `mkPackage` callback.