Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maintainers/scripts/update.nix: various fixes and clean-ups #87731

Merged
merged 4 commits into from May 15, 2020

Conversation

@jtojnar
Copy link
Contributor

jtojnar commented May 13, 2020

Motivation for this change

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.

How to test this

Apply the following patch:

--- a/pkgs/desktops/gnome-3/default.nix
+++ b/pkgs/desktops/gnome-3/default.nix
@@ -27,6 +27,25 @@ lib.makeScope pkgs.newScope (self: with self; {
 
 #### Core (http://ftp.acc.umu.se/pub/GNOME/core/)
 
+  updateableBadSrc = pkgs.mkYarnPackage {
+    name = "test";
+    src = pkgs.fetchgit {
+      url = "https://example.com";
+      rev = "test";
+      sha256 = "0000000000000000000000000000000000000000000000000000000000000000";
+    };
+  };
+
+  updateableList = [
+    adwaita-icon-theme
+  ];
+
+  updateableAttrs = {
+    inherit adwaita-icon-theme;
+  };
+
+  nonUpdateable = 4;
+
   adwaita-icon-theme = callPackage ./core/adwaita-icon-theme { };
 
   baobab = callPackage ./core/baobab { };

and run:

nix-shell maintainers/scripts/update.nix --argstr path gnome3.updateableBadSrc
nix-shell maintainers/scripts/update.nix --argstr path gnome3.updateableList
nix-shell maintainers/scripts/update.nix --argstr path gnome3.updateableAttrs
nix-shell maintainers/scripts/update.nix --argstr path gnome3.nonUpdateable

None of these should fail or try to download their sources.

jtojnar added 3 commits May 13, 2020
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
This will make it easier to change it if we want to decouple from pkgs.
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.
@jtojnar jtojnar force-pushed the jtojnar:update-no-build-src branch from e4b6093 to 3f3aeb7 May 13, 2020
@jtojnar jtojnar requested a review from edolstra as a code owner May 13, 2020
@jtojnar jtojnar requested a review from garbas May 13, 2020
@jtojnar jtojnar merged commit a762c0e into NixOS:master May 15, 2020
13 checks passed
13 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96f3c62"; rev="96f3c622afee15aff241763d2c1a14c65e2fb113"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96f3c62"; rev="96f3c622afee15aff241763d2c1a14c65e2fb113"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96f3c62"; rev="96f3c622afee15aff241763d2c1a14c65e2fb113"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96f3c62"; rev="96f3c622afee15aff241763d2c1a14c65e2fb113"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96f3c62"; rev="96f3c622afee15aff241763d2c1a14c65e2fb113"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96f3c62"; rev="96f3c622afee15aff241763d2c1a14c65e2fb113"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96f3c62"; rev="96f3c622afee15aff241763d2c1a14c65e2fb113"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@jtojnar jtojnar deleted the jtojnar:update-no-build-src branch May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant
You can’t perform that action at this time.