RFC: More common updater script implementations #21766

dezgeg · 2017-01-09T12:16:35Z

As a first step of adding some common updater script implementations, this PR adds a script to automate patching the version and sha256 fields in .nix files. It currently uses the hacky way of replacing the existing hash with an invalid one and parsing the proper hash from the error message. When NixOS/nix#1172 is fixed, nix-build --hash can be used.

Now, others have proposed (and used) different and arguably cleaner approaches than patching the .nix files, like generating separate JSON files for sources (e.g. #21734). But from a pragmatic point of view, we already have thousands of packages using the traditional "version/sha in .nix file" style, so it likely makes sense to make the tooling work with those instead of trying to convert all of them.

cc @garbas for the tp_smapi stuff and @taku0 for the Firefox/Thunderbird stuff. Also @edolstra for the general approach & locations of the scripts.

For easier testing, here are the same commits + the relevant packages downgraded: https://github.com/dezgeg/nixpkgs/commits/updater-downgraded

mention-bot · 2017-01-09T12:16:37Z

@dezgeg, thanks for your PR! By analyzing the history of the files in this pull request, we identified @taku0, @garbas and @FRidh to be potential reviewers.

taku0 · 2017-01-09T13:13:04Z

I prefer generating separate JSON file. I should have done so for Firefox/Thunderbird. After all, we need rewrite each packages to adopt updateScript, which may not a simple task. We may need to write a web scraper to know the latest version. Compared to it, rewriting default.nix to using separate JSON file is a simple task.

taku0 · 2017-01-09T13:16:40Z

Of course, this update script is far better than my ad hoc update script using ed.

dezgeg · 2017-01-09T13:25:44Z

I think in a large number of cases rewriting the packages is not necessary and things like figuring out the latest version can be automated. E.g. if a package is currently using fetchgit to fetch a particular tag, it's very likely that an automated upgrade could just update to a newer tag in the repo. Similarly, for packages using tarballs all that might be needed is to specify meta.downloadPage and see if there are links to newer tarballs following the same version pattern.

(This is the similar logic what http://monitor.nixos.org/ used to do when it was up and I found it very useful).

edolstra · 2017-01-09T14:01:11Z

pkgs/applications/networking/mailreaders/thunderbird/default.nix

-    name = "thunderbird";
-    sourceSectionRegex = ".";
-    basePath = "pkgs/applications/networking/mailreaders/thunderbird";
+  passthru.updateScript = callPackage ./../../browsers/firefox/update.nix {


This is an abuse of callPackage. update.nix is not a package...

Should we have the same thing under a different name? It's quite painful to duplicate the list of upgrade script dependencies four times.

edolstra · 2017-01-09T14:30:53Z

maintainers/scripts/common-updater/update-source-version

+#! nix-shell --pure -p bash nix coreutils gawk gnused -i bash
+set -e
+
+# FIXME: It's dumb that this is needed.


Why is this needed?

Otherwise this happens (on NixOS):

error: Nix database directory ‘/nix/var/nix/db’ is not writable: Permission denied ./maintainers/scripts/common-updater/update-source-version: error: Couldn't evaluate 'thunderbird.meta.position' to locate the .nix file!

garbas

i'm not a fan of using nix-shell inside nix-shell. i would rather have this common-scripts become packages in nixpkgs (under some namespace) and which we run them using maintainers/scripts/update.nix

edolstra · 2017-01-09T14:36:20Z

This looks basically good to me. Having a 90-line shell script full of grep/sed magic is not ideal but that could be improved in the future. (I'm thinking that ideally, we would have nix edit-attr command that would use the evaluator's knowledge of position info in the AST. That could also be useful for editing NixOS configurations programmatically...)

edolstra · 2017-01-09T14:42:03Z

@garbas I agree.

@dezgeg Regarding callPackage, I think the real issue is abstracting away boilerplate code in update scripts. We really don't want to pass in coreutils etc. in every update script. See my comment here: https://github.com/NixOS/nixpkgs/pull/21405/files#r95163181. Such a mkUpdateScript function would presumably also have update-source-version in its default PATH.

dezgeg · 2017-01-13T12:39:02Z

Okay, thank you for the feedback. I've now moved this to pkgs/common-updater/scripts.nix, with the intention of placing the mkUpdateScript function Eelco proposed into pkgs/common-updater/default.nix later on. I have some ideas (and unfinished code) on the overall structure and things like common implementation for updating a fetchFromGitHub package to the latest tag in the repository that I will try to polish up soon-ish.

Adds a script to help automatically upgrading packages: this one can patch name/version attributes like: version = "50.1.0"; name = "bc-1.06"; ... to the given version, and updates the sha256 hash to match. Usage is: update-source-version <attr> <new-version> [<new-source-hash>] where: - attr is the attribute path of the package - new-version is the version string to be patched in - new-source-hash is the optional sha256/etc. hash of the source. If not given, the script will automatically calculate it. This is added to a subdirectory where other useful scripts can be added in the future, like figuring out the newest version from a git repo or GitHub releases etc.

This way we have the benefit of the usual Nixpkgs style, and gain a slight reduction in amount of code in the updater. Also use callPackage to reduce duplication of the dependencies of the update script and use makeBinPath to make things neater.

Replace the custom patching code with the common script. Also use callPackage and makeBinPath

dezgeg · 2017-01-26T00:34:22Z

Any issues with the updated version?

dezgeg · 2017-02-19T14:51:47Z

I have pushed this as it also fixes the updateScript for tp_smapi that got broken by c20cc6d.

dezgeg mentioned this pull request Jan 9, 2017

add nix-prefetch-source #21734

Merged

edolstra reviewed Jan 9, 2017

View reviewed changes

garbas self-requested a review January 9, 2017 14:35

garbas requested changes Jan 9, 2017

View reviewed changes

garbas self-assigned this Jan 12, 2017

dezgeg force-pushed the updater branch from 73c807e to 6df326c Compare January 13, 2017 11:55

dezgeg added 3 commits January 13, 2017 15:46

firefox, thunderbird: Use common-updater/update-source-version script

c8c7bf1

Replace the custom patching code with the common script. Also use callPackage and makeBinPath

dezgeg force-pushed the updater branch from 6df326c to c8c7bf1 Compare January 13, 2017 13:46

dezgeg closed this Feb 19, 2017

nbp mentioned this pull request Feb 26, 2017

RFC to describe the RFC process NixOS/rfcs#1

Merged

4 tasks

jtojnar added the 6.topic: updaters Tooling for (semi-)automated updating of packages label Jun 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: More common updater script implementations #21766

RFC: More common updater script implementations #21766

dezgeg commented Jan 9, 2017 •

edited

mention-bot commented Jan 9, 2017

taku0 commented Jan 9, 2017

taku0 commented Jan 9, 2017

dezgeg commented Jan 9, 2017

edolstra Jan 9, 2017

dezgeg Jan 9, 2017

edolstra Jan 9, 2017

dezgeg Jan 9, 2017

garbas left a comment

edolstra commented Jan 9, 2017 •

edited

edolstra commented Jan 9, 2017

dezgeg commented Jan 13, 2017

dezgeg commented Jan 26, 2017

dezgeg commented Feb 19, 2017

RFC: More common updater script implementations #21766

RFC: More common updater script implementations #21766

Conversation

dezgeg commented Jan 9, 2017 • edited

mention-bot commented Jan 9, 2017

taku0 commented Jan 9, 2017

taku0 commented Jan 9, 2017

dezgeg commented Jan 9, 2017

edolstra Jan 9, 2017

Choose a reason for hiding this comment

dezgeg Jan 9, 2017

Choose a reason for hiding this comment

edolstra Jan 9, 2017

Choose a reason for hiding this comment

dezgeg Jan 9, 2017

Choose a reason for hiding this comment

garbas left a comment

Choose a reason for hiding this comment

edolstra commented Jan 9, 2017 • edited

edolstra commented Jan 9, 2017

dezgeg commented Jan 13, 2017

dezgeg commented Jan 26, 2017

dezgeg commented Feb 19, 2017

dezgeg commented Jan 9, 2017 •

edited

edolstra commented Jan 9, 2017 •

edited