From 4c398f9530af9844e2d33b3abebbb2128f6626d4 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 25 Jan 2021 14:34:54 +0100 Subject: [PATCH 1/2] lib: Split checkMkDerivationFor into multiple functions This will allow us to reuse it for other package sets like Python. --- lib/default.nix | 54 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/lib/default.nix b/lib/default.nix index dd5e34e..86e7482 100644 --- a/lib/default.nix +++ b/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)) + 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; }; }; } From f9e9674ddf0e6861d2009c25a3667aace1de67df Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 25 Jan 2021 20:47:48 +0100 Subject: [PATCH 2/2] Pass the original derivation to checkers This will allow us to do things like checking compatibility of interpreter of the original package. --- lib/default.nix | 2 +- overlays/attribute-ordering.nix | 10 +++++----- overlays/build-tools-in-build-inputs.nix | 6 +++--- overlays/explicit-phases.nix | 6 +++--- overlays/fixup-phase.nix | 6 +++--- overlays/meson-cmake.nix | 6 +++--- overlays/missing-phase-hooks.nix | 10 +++++----- overlays/patch-phase.nix | 6 +++--- overlays/unclear-gpl.nix | 6 +++--- overlays/unnecessary-parallel-building.nix | 8 ++++---- 10 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/default.nix b/lib/default.nix index 86e7482..eef3ca4 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -45,7 +45,7 @@ rec { in if builtins.elem namePosition namePositions then - addReports originalDrv (lib.filter ({ cond, ... }: cond) (check args)) + addReports originalDrv (lib.filter ({ cond, ... }: cond) (check args originalDrv)) else originalDrv; diff --git a/overlays/attribute-ordering.nix b/overlays/attribute-ordering.nix index 912fc05..0e30e5f 100644 --- a/overlays/attribute-ordering.nix +++ b/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) ]; } ); diff --git a/overlays/build-tools-in-build-inputs.nix b/overlays/build-tools-in-build-inputs.nix index 4bb779f..fe7d82d 100644 --- a/overlays/build-tools-in-build-inputs.nix +++ b/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 diff --git a/overlays/explicit-phases.nix b/overlays/explicit-phases.nix index 86860f0..c8d1107 100644 --- a/overlays/explicit-phases.nix +++ b/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 diff --git a/overlays/fixup-phase.nix b/overlays/fixup-phase.nix index 2b1bf85..bdf8ba6 100644 --- a/overlays/fixup-phase.nix +++ b/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) ]; }; diff --git a/overlays/meson-cmake.nix b/overlays/meson-cmake.nix index b7c90df..5be4d40 100644 --- a/overlays/meson-cmake.nix +++ b/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) ]; }; diff --git a/overlays/missing-phase-hooks.nix b/overlays/missing-phase-hooks.nix index 191d557..700b66f 100644 --- a/overlays/missing-phase-hooks.nix +++ b/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) ]; } ) diff --git a/overlays/patch-phase.nix b/overlays/patch-phase.nix index 76086c9..d916a40 100644 --- a/overlays/patch-phase.nix +++ b/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) ]; }; diff --git a/overlays/unclear-gpl.nix b/overlays/unclear-gpl.nix index ec65e25..f2f2856 100644 --- a/overlays/unclear-gpl.nix +++ b/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 diff --git a/overlays/unnecessary-parallel-building.nix b/overlays/unnecessary-parallel-building.nix index ed00e84..293959b 100644 --- a/overlays/unnecessary-parallel-building.nix +++ b/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) ]; };