lib: Split checkMkDerivationFor into multiple functions

lib/default.nix

@@ -14,6 +14,41 @@ rec {
       in
         lib.toUpper head + tail;
 
+  # Attaches reports to a package’s attribute set.
+  addReports =
+    originalDrv:
+    reports:
+
+    lib.recursiveUpdate originalDrv {
+      __nixpkgs-hammering-state = {
+        reports = originalDrv.__nixpkgs-hammering-state.reports or [] ++ reports;
+      };
+    };
+
+  # Creates a function based on the original one, that, when called on
+  # one of the requested packages, runs a check on the arguments passed to it
+  # and then returns the original result enriched with the reports.
+  wrapFunctionWithChecks =
+    originalFunction:
+    namePositions:
+    check:
+
+    args:
+
+    let
+      originalDrv = originalFunction args;
+      namePosition =
+        let
+          pnamePosition = builtins.unsafeGetAttrPos "pname" args;
+        in
+          if pnamePosition != null then pnamePosition else builtins.unsafeGetAttrPos "name" args;
+    in
+      if builtins.elem namePosition namePositions
+      then
+        addReports originalDrv (lib.filter ({ cond, ... }: cond) (check args originalDrv))
+      else
+        originalDrv;
+
   # Creates an overlay that replaces stdenv.mkDerivation with a function that,
   # for packages with locations of name attribute matching one of the namePositions,
   # checks the attribute set passed as argument to mkDerivation.
@@ -29,24 +64,7 @@ rec {
 
     {
       stdenv = prev.stdenv // {
-        mkDerivation = drv:
-          let
-            originalDrv = prev.stdenv.mkDerivation drv;
-            namePosition =
-              let
-                pnamePosition = builtins.unsafeGetAttrPos "pname" drv;
-              in
-                if pnamePosition != null then pnamePosition else builtins.unsafeGetAttrPos "name" drv;
-          in
-            if builtins.elem namePosition namePositions
-            then
-              lib.recursiveUpdate originalDrv {
-                __nixpkgs-hammering-state = {
-                  reports = originalDrv.__nixpkgs-hammering-state.reports or [] ++ lib.filter ({ cond, ... }: cond) (check drv);
-                };
-              }
-            else
-              originalDrv;
+        mkDerivation = wrapFunctionWithChecks prev.stdenv.mkDerivation namePositions check;
       };
     };
 }

overlays/attribute-ordering.nix

@@ -27,11 +27,11 @@ let
         builtins.listToAttrs
       ];
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     let
-      getAttrLine = attr: (builtins.unsafeGetAttrPos attr drv).line;
+      getAttrLine = attr: (builtins.unsafeGetAttrPos attr drvArgs).line;
 
-      drvAttrs = builtins.sort (a: b: getAttrLine a < getAttrLine b) (builtins.attrNames drv);
+      drvAttrs = builtins.sort (a: b: getAttrLine a < getAttrLine b) (builtins.attrNames drvArgs);
 
       knownDrvAttrs = builtins.filter (attr: preferredOrdering ? "${attr}") drvAttrs;
 
@@ -48,8 +48,8 @@ let
             The ${lib.optionalString (fstInfo.group != null) "${fstInfo.group}, including the "}attribute “${fst}” should preferably come before ${lib.optionalString (sndInfo.group != null) "${sndInfo.group}’ "}“${snd}” attribute in the expression.
           '';
           locations = [
-            (builtins.unsafeGetAttrPos fst drv)
-            (builtins.unsafeGetAttrPos snd drv)
+            (builtins.unsafeGetAttrPos fst drvArgs)
+            (builtins.unsafeGetAttrPos snd drvArgs)
           ];
         }
       );

overlays/build-tools-in-build-inputs.nix

@@ -15,16 +15,16 @@ let
     "pkg-config"
   ];
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     (map
       (tool: {
         name = "build-tools-in-build-inputs";
-        cond = lib.elem prev.${tool} (drv.buildInputs or [ ]);
+        cond = lib.elem prev.${tool} (drvArgs.buildInputs or [ ]);
         msg = ''
           ${tool} is a build tool so it likely goes to `nativeBuildInputs`, not `buildInputs`.
         '';
         locations = [
-          (builtins.unsafeGetAttrPos "buildInputs" drv)
+          (builtins.unsafeGetAttrPos "buildInputs" drvArgs)
         ];
       })
       buildTools

overlays/explicit-phases.nix

@@ -15,16 +15,16 @@ let
     "install"
   ];
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     (map
       (phase: {
         name = "explicit-phases";
-        cond = drv ? "${phase}Phase";
+        cond = drvArgs ? "${phase}Phase";
         msg = ''
           It is a good idea to avoid overriding `${phase}Phase` when possible.
         '';
         locations = [
-          (builtins.unsafeGetAttrPos "${phase}Phase" drv)
+          (builtins.unsafeGetAttrPos "${phase}Phase" drvArgs)
         ];
       })
       phases

overlays/fixup-phase.nix

@@ -8,15 +8,15 @@ let
   inherit (prev) lib;
   inherit (import ../lib { inherit lib; }) checkMkDerivationFor;
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     lib.singleton {
       name = "fixup-phase";
-      cond = drv ? fixupPhase;
+      cond = drvArgs ? fixupPhase;
       msg = ''
         `fixupPhase` should not be overridden, use `postFixup` instead.
       '';
       locations = [
-        (builtins.unsafeGetAttrPos "fixupPhase" drv)
+        (builtins.unsafeGetAttrPos "fixupPhase" drvArgs)
       ];
     };
 

overlays/meson-cmake.nix

@@ -8,15 +8,15 @@ let
   inherit (prev) lib;
   inherit (import ../lib { inherit lib; }) checkMkDerivationFor;
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     lib.singleton {
       name = "meson-cmake";
-      cond = lib.elem prev.meson (drv.nativeBuildInputs or [ ]) && lib.elem prev.cmake (drv.nativeBuildInputs or [ ]);
+      cond = lib.elem prev.meson (drvArgs.nativeBuildInputs or [ ]) && lib.elem prev.cmake (drvArgs.nativeBuildInputs or [ ]);
       msg = ''
         Meson uses CMake as a fallback dependency resolution method and it likely is not necessary here. The message about cmake not being found is purely informational.
       '';
       locations = [
-        (builtins.unsafeGetAttrPos "nativeBuildInputs" drv)
+        (builtins.unsafeGetAttrPos "nativeBuildInputs" drvArgs)
       ];
     };
 

overlays/missing-phase-hooks.nix

@@ -15,20 +15,20 @@ let
     "install"
   ];
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     (map
       (phase:
         let
-          preMissing = builtins.match ".*runHook pre${capitalize phase}.*"  drv."${phase}Phase" == null;
-          postMissing = builtins.match ".*runHook post${capitalize phase}.*" drv."${phase}Phase" == null;
+          preMissing = builtins.match ".*runHook pre${capitalize phase}.*"  drvArgs."${phase}Phase" == null;
+          postMissing = builtins.match ".*runHook post${capitalize phase}.*" drvArgs."${phase}Phase" == null;
         in {
           name = "missing-phase-hooks";
-          cond = drv ? "${phase}Phase" && drv."${phase}Phase" != null && (preMissing || postMissing);
+          cond = drvArgs ? "${phase}Phase" && drvArgs."${phase}Phase" != null && (preMissing || postMissing);
           msg = ''
             `${phase}Phase` should probably contain ${lib.optionalString preMissing "`runHook pre${capitalize phase}`"}${lib.optionalString (preMissing && postMissing) " and "}${lib.optionalString postMissing "`runHook post${capitalize phase}`"}.
           '';
           locations = [
-            (builtins.unsafeGetAttrPos "${phase}Phase" drv)
+            (builtins.unsafeGetAttrPos "${phase}Phase" drvArgs)
           ];
         }
       )

overlays/patch-phase.nix

@@ -8,15 +8,15 @@ let
   inherit (prev) lib;
   inherit (import ../lib { inherit lib; }) checkMkDerivationFor;
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     lib.singleton {
       name = "patch-phase";
-      cond = drv ? patchPhase;
+      cond = drvArgs ? patchPhase;
       msg = ''
         `patchPhase` should not be overridden, use `postPatch` instead.
       '';
       locations = [
-        (builtins.unsafeGetAttrPos "patchPhase" drv)
+        (builtins.unsafeGetAttrPos "patchPhase" drvArgs)
       ];
     };
 

overlays/unclear-gpl.nix

@@ -21,16 +21,16 @@ let
     "lgpl3"
   ];
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     (map
       (license: {
         name = "unclear-gpl";
-        cond = lib.elem lib.licenses.${license} (lib.toList (drv.meta.license or []));
+        cond = lib.elem lib.licenses.${license} (lib.toList (drvArgs.meta.license or []));
         msg = ''
           `${license}` is a deprecated license, check if project uses `${license}Plus` or `${license}Only` and change `meta.license` accordingly.
         '';
         locations = [
-          (builtins.unsafeGetAttrPos "license" drv.meta)
+          (builtins.unsafeGetAttrPos "license" drvArgs.meta)
         ];
       })
       licenses

overlays/unnecessary-parallel-building.nix

@@ -8,19 +8,19 @@ let
   inherit (prev) lib;
   inherit (import ../lib { inherit lib; }) checkMkDerivationFor;
 
-  checkDerivation = drv:
+  checkDerivation = drvArgs: drv:
     lib.singleton {
       name = "unnecessary-parallel-building";
       cond =
         let
-          inputs = drv.nativeBuildInputs or [ ] ++ drv.propagatedBuildInputs or [ ] ++ drv.nativeBuildInputs or [ ];
+          inputs = drvArgs.nativeBuildInputs or [ ] ++ drvArgs.propagatedBuildInputs or [ ] ++ drvArgs.nativeBuildInputs or [ ];
         in
-          (lib.elem prev.meson inputs || lib.elem prev.cmake inputs || lib.elem prev.qt5.qmake inputs) && drv.enableParallelBuilding or false;
+          (lib.elem prev.meson inputs || lib.elem prev.cmake inputs || lib.elem prev.qt5.qmake inputs) && drvArgs.enableParallelBuilding or false;
       msg = ''
         Meson, CMake and qmake already set `enableParallelBuilding = true` by default so it is not necessary.
       '';
       locations = [
-        (builtins.unsafeGetAttrPos "enableParallelBuilding" drv)
+        (builtins.unsafeGetAttrPos "enableParallelBuilding" drvArgs)
       ];
     };