From 8f50956ee059d882be5ed53ad9704ebcaa41856d Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 13 May 2020 08:35:35 +0200 Subject: [PATCH 1/4] maintainers/scripts/update.nix: Import lib into scope This will make it easier to change it if we want to decouple from pkgs. --- maintainers/scripts/update.nix | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/maintainers/scripts/update.nix b/maintainers/scripts/update.nix index 7c45e148e82..23ea11b66e1 100755 --- a/maintainers/scripts/update.nix +++ b/maintainers/scripts/update.nix @@ -9,6 +9,10 @@ # TODO: add assert statements let + pkgs = import ./../../default.nix (if include-overlays then { } else { overlays = []; }); + + inherit (pkgs) lib; + /* Remove duplicate elements from the list based on some extracted value. O(n^2) complexity. */ nubOn = f: list: @@ -16,21 +20,19 @@ let [] else let - x = pkgs.lib.head list; - xs = pkgs.lib.filter (p: f x != f p) (pkgs.lib.drop 1 list); + x = lib.head list; + xs = lib.filter (p: f x != f p) (lib.drop 1 list); in [x] ++ nubOn f xs; - pkgs = import ./../../default.nix (if include-overlays then { } else { overlays = []; }); - packagesWith = cond: return: set: nubOn (pkg: pkg.updateScript) - (pkgs.lib.flatten - (pkgs.lib.mapAttrsToList + (lib.flatten + (lib.mapAttrsToList (name: pkg: let result = builtins.tryEval ( - if pkgs.lib.isDerivation pkg && cond name pkg + if lib.isDerivation pkg && cond name pkg then [(return name pkg)] else if pkg.recurseForDerivations or false || pkg.recurseForRelease or false then packagesWith cond return pkg @@ -47,10 +49,10 @@ let packagesWithUpdateScriptAndMaintainer = maintainer': let maintainer = - if ! builtins.hasAttr maintainer' pkgs.lib.maintainers then + if ! builtins.hasAttr maintainer' lib.maintainers then builtins.throw "Maintainer with name `${maintainer'} does not exist in `maintainers/maintainer-list.nix`." else - builtins.getAttr maintainer' pkgs.lib.maintainers; + builtins.getAttr maintainer' lib.maintainers; in packagesWith (name: pkg: builtins.hasAttr "updateScript" pkg && (if builtins.hasAttr "maintainers" pkg.meta @@ -66,7 +68,7 @@ let packagesWithUpdateScript = path: let - attrSet = pkgs.lib.attrByPath (pkgs.lib.splitString "." path) null pkgs; + attrSet = lib.attrByPath (lib.splitString "." path) null pkgs; in if attrSet == null then builtins.throw "Attribute path `${path}` does not exists." @@ -77,7 +79,7 @@ let packageByName = name: let - package = pkgs.lib.attrByPath (pkgs.lib.splitString "." name) null pkgs; + package = lib.attrByPath (lib.splitString "." name) null pkgs; in if package == null then builtins.throw "Package with an attribute name `${name}` does not exists." @@ -125,15 +127,15 @@ let packageData = package: { name = package.name; - pname = pkgs.lib.getName package; - updateScript = map builtins.toString (pkgs.lib.toList package.updateScript); + pname = lib.getName package; + updateScript = map builtins.toString (lib.toList package.updateScript); }; packagesJson = pkgs.writeText "packages.json" (builtins.toJSON (map packageData packages)); optionalArgs = - pkgs.lib.optional (max-workers != null) "--max-workers=${max-workers}" - ++ pkgs.lib.optional (keep-going == "true") "--keep-going"; + lib.optional (max-workers != null) "--max-workers=${max-workers}" + ++ lib.optional (keep-going == "true") "--keep-going"; args = [ packagesJson ] ++ optionalArgs; From fab2ee8c10c2d39310c8e3cdddde9fd118727063 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 13 May 2020 08:30:39 +0200 Subject: [PATCH 2/4] maintainers/scripts/update.nix: derivation is the final station It does not make sense to look for derivations within derivations, not even when `recurseForDerivations` is true. Nix does not do that either: https://github.com/NixOS/nix/blob/ebc024df2287085d48ed6194aa756fd70c07f76c/src/libexpr/get-drvs.cc#L346-L355 --- maintainers/scripts/update.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/maintainers/scripts/update.nix b/maintainers/scripts/update.nix index 23ea11b66e1..492000a1037 100755 --- a/maintainers/scripts/update.nix +++ b/maintainers/scripts/update.nix @@ -32,8 +32,8 @@ let (name: pkg: let result = builtins.tryEval ( - if lib.isDerivation pkg && cond name pkg - then [(return name pkg)] + if lib.isDerivation pkg + then lib.optional (cond name pkg) (return name pkg) else if pkg.recurseForDerivations or false || pkg.recurseForRelease or false then packagesWith cond return pkg else [] From 3f3aeb7c85fb5b7e6b9e950aed3a36a0e65e8bde Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 13 May 2020 10:29:01 +0200 Subject: [PATCH 3/4] maintainers/scripts/update.nix: refactor package collector The `packagesWith` function expected an attrSet but `packagesWithUpdateScript` could be passing it a derivation or a list when the attribute path supplied by user through the `--argstr path` argument pointed to one. It only worked because derivations are also attrSets and contain their outputs as attributes, and did not work for lists at all. Additionally, the improper handling would cause the `src` attribute to be built in some rare cases (`mkYarnPackage` seems to trigger this). Rewriting the `packagesWith` function to be inductive with a derivation as a base case and attrSets and lists as inductive steps is much cleaner and also fixes the unnecessary build. --- maintainers/scripts/update.nix | 57 ++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/maintainers/scripts/update.nix b/maintainers/scripts/update.nix index 492000a1037..66ea18ddf62 100755 --- a/maintainers/scripts/update.nix +++ b/maintainers/scripts/update.nix @@ -25,26 +25,29 @@ let in [x] ++ nubOn f xs; - packagesWith = cond: return: set: - nubOn (pkg: pkg.updateScript) - (lib.flatten - (lib.mapAttrsToList - (name: pkg: - let - result = builtins.tryEval ( - if lib.isDerivation pkg - then lib.optional (cond name pkg) (return name pkg) - else if pkg.recurseForDerivations or false || pkg.recurseForRelease or false - then packagesWith cond return pkg - else [] - ); - in - if result.success then result.value - else [] - ) - set - ) - ); + packagesWithPath = relativePath: cond: return: pathContent: + let + result = builtins.tryEval pathContent; + + dedupResults = lst: nubOn (pkg: pkg.updateScript) (lib.concatLists lst); + in + if result.success then + let + pathContent = result.value; + in + if lib.isDerivation pathContent then + lib.optional (cond relativePath pathContent) (return relativePath pathContent) + else if lib.isAttrs pathContent then + # If user explicitly points to an attrSet or it is marked for recursion, we recur. + if relativePath == [] || pathContent.recurseForDerivations or false || pathContent.recurseForRelease or false then + dedupResults (lib.mapAttrsToList (name: elem: packagesWithPath (relativePath ++ [name]) cond return elem) pathContent) + else [] + else if lib.isList pathContent then + dedupResults (lib.imap0 (i: elem: packagesWithPath (relativePath ++ [i]) cond return elem) pathContent) + else [] + else []; + + packagesWith = packagesWithPath []; packagesWithUpdateScriptAndMaintainer = maintainer': let @@ -54,7 +57,7 @@ let else builtins.getAttr maintainer' lib.maintainers; in - packagesWith (name: pkg: builtins.hasAttr "updateScript" pkg && + packagesWith (relativePath: pkg: builtins.hasAttr "updateScript" pkg && (if builtins.hasAttr "maintainers" pkg.meta then (if builtins.isList pkg.meta.maintainers then builtins.elem maintainer pkg.meta.maintainers @@ -63,19 +66,19 @@ let else false ) ) - (name: pkg: pkg) + (relativePath: pkg: pkg) pkgs; packagesWithUpdateScript = path: let - attrSet = lib.attrByPath (lib.splitString "." path) null pkgs; + pathContent = lib.attrByPath (lib.splitString "." path) null pkgs; in - if attrSet == null then + if pathContent == null then builtins.throw "Attribute path `${path}` does not exists." else - packagesWith (name: pkg: builtins.hasAttr "updateScript" pkg) - (name: pkg: pkg) - attrSet; + packagesWith (relativePath: pkg: builtins.hasAttr "updateScript" pkg) + (relativePath: pkg: pkg) + pathContent; packageByName = name: let From 96f3c622afee15aff241763d2c1a14c65e2fb113 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 13 May 2020 14:16:47 +0200 Subject: [PATCH 4/4] github/CODEOWNERS: Add myself to updaters --- .github/CODEOWNERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 7c39ce48a3f..211779a470e 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -55,6 +55,13 @@ # NixOS integration test driver /nixos/lib/test-driver @tfc +# Updaters +## update.nix +/maintainers/scripts/update.nix @jtojnar +/maintainers/scripts/update.py @jtojnar +## common-updater-scripts +/pkgs/common-updater/scripts/update-source-version @jtojnar + # Python-related code and docs /maintainers/scripts/update-python-libraries @FRidh /pkgs/top-level/python-packages.nix @FRidh @jonringer