maintainers/scripts/update.nix: Add support for auto-commiting changes #98304
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not sure why I chose ProcessPoolExecutor in the first place.
Printing the changed file and new version can be used to commit the changes to git.
jtojnar
requested review from
NeQuissimus,
hedning,
romildo,
worldofpeace and
doronbehar
ofborg
bot
added
6.topic: GNOME
jtojnar
force-pushed
the
updateScript-commit3
branch
from
fad9c66
to
ebd6ab9
Compare
|
@jtojnar Awesome, will test 👍 |
|
Just for giggles I wanted to see what would happen if I tried to update a single package that I know has no upstream changes. This produced an interesting behavior: (I should also be wondering how that depended on busybox-1.31.1-x86_64-unknown-linux-musl ???)
I then tried it with the gnome update script It produced the same behavior where I had to |
Oh, I guess this is expected even if you update a single package and https://github.com/NixOS/nixpkgs/pull/98304/files#diff-ef694abe116c19d066d3c003b85eadc9R146. |
|
These worktree related messages are certainly sort of confusing. Would it be possible to hide them? I ran: And currently the command hangs with this diff: diff --git i/pkgs/servers/gotify/vendor-sha.nix w/pkgs/servers/gotify/vendor-sha.nix
index f9e648a957e..e16c76dff88 100644
--- i/pkgs/servers/gotify/vendor-sha.nix
+++ w/pkgs/servers/gotify/vendor-sha.nix
@@ -1 +1 @@
-"1in4gzmrgb6z7p5fnz33f88g5l0vki2xlxlllk5wy9icp4h3h9sd"
+""And these messages by update.py: |
|
Thanks for testing.
$ mkcd /tmp
$ mkdir foo
$ ln -s foo bar
$ cd bar
$ git init
Initialized empty Git repository in /tmp/foo/.git/
$ git rev-parse --show-toplevel
/tmp/foo
$ echo '#!/usr/bin/env bash' > script.sh
$ echo 'echo "$(dirname "$0")"' >> script.sh
$ echo 'echo "$(dirname "${BASH_SOURCE[0]}")"' >> script.sh
$ echo 'echo "$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"' >> script.sh # https://stackoverflow.com/a/246128/160386
$ chmod +x script.sh
$ /tmp/foo/script.sh
/tmp/foo
/tmp/foo
/tmp/foo
$ /tmp/bar/script.sh
/tmp/bar
/tmp/bar
/tmp/bar
$ ./script.sh
.
.
/tmp/bar
$ nix-instantiate --expr 'builtins.toString ./script.sh' --eval
"/tmp/foo/script.sh" |
jtojnar
force-pushed
the
updateScript-commit3
branch
from
7c00ae7
to
8071955
Compare
doronbehar
reviewed
Sep 20, 2020
Comment on lines
+151
to
+152
| supportedFeatures = package.updateScript.supportedFeatures or []; | ||
| attrPath = package.updateScript.attrPath or attrPath; |
There was a problem hiding this comment.
It'd be nice if this were documented.
There was a problem hiding this comment.
I intentionally removed the documentation in c1b0544 so that people do not rely on it yet. I am weighting whether the added complexity is worth it and reserve the right to remove it.
ofborg
bot
removed
the
6.topic: GNOME
Update scripts can now declare features using
passthru.updateScript = {
command = [ ../../update.sh pname ];
supportedFeatures = [ "commit" ];
};
A `commit` feature means that when the update script finishes successfully,
it will print a JSON list like the following:
[
{
"attrPath": "volume_key",
"oldVersion": "0.3.11",
"newVersion": "0.3.12",
"files": [
"/path/to/nixpkgs/pkgs/development/libraries/volume-key/default.nix"
]
}
]
and data from that will be used when update.nix is run with --argstr commit true
to create commits.
We will create a new git worktree for each thread in the pool and run the update
script there. Then we will commit the change and cherry pick it in the main repo,
releasing the worktree for a next change.
Get rid of some globals, split main into smaller functions, rename some variables, add typehints.
This will make it cleaner and also better respect SIGTERM.
…trPath Instead of having the updateScript support returning JSON object, it should be sufficient to specify attrPath in passthru.updateScript. It is much easier to use. The former is now considered experimental.
…utes We can determine all of them when attrPath is present so we might jsut as well do it.
We no longer need it for most use cases so I am making it experimental. I have something in mind where it might be useful in the future (customizing commit messages) but for now, it would only confuse people.
…R_PATH The environment variable will contain the attribute path the script is supposed to update.
`update.nix` extracts `passthru.updateScript` attributes in the main repo and when they are relative paths (e.g. `./update.sh`), Nix will resolve them to absolute paths in the main repo. Update scripts can use $(dirname $0) to get the location of files they should update but that would point to the main repo. We want them to modify the appropriate git worktree instead so we replace the prefix accordingly. `git rev-parse --show-toplevel` will resolve symlinks but, fortunately, Nix will do that as well, so the path will match: NixOS#98304 (comment)
jtojnar
force-pushed
the
updateScript-commit3
branch
from
edda38d
to
71c246c
Compare
- Make some arguments more fitting (the path is actually full, not just relative to prefix…). - Increase the purity of packages* functions (they now take pkgs from argument, not from scope). - Add some documentation comments.
jtojnar
force-pushed
the
updateScript-commit3
branch
from
6247163
to
74a5bb4
Compare
|
I tried #98304 (comment) and everything seems to work well 👍 |
worldofpeace
approved these changes
Oct 2, 2020
10 tasks
jtojnar
added a commit
to jtojnar/nixpkgs
that referenced
this pull request
Dec 16, 2020
Lists items are not directly accessible like attributes in attrsets are. This makes it hard to represent their address in `UPDATE_NIX_ATTR_PATH` environment variable passed to update scripts. Given that I only introduced list support for `gnome3` attribute set and we stopped using them there, let’s remove the list support again. NixOS modules are better place for package collections anyway. This was meant to go in with NixOS#98304 but got accidentally omitted somehow.
alyssais
added a commit
to alyssais/nixpkgs
that referenced
this pull request
Jun 17, 2021
NixOS#59372 was replaced with NixOS#98304, which was merged as 74c5472, so I'm following the instructions in the comment and enabling the updateScript. Seems to work.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This replaces #59372 and fixes #57609.
I decided to abandon the asyncio_pool library since it seemed buggy and just went with plain asyncio from standard library.
I also managed to figure out how to make the updates work without any changes to
passthru.updateScriptnecessary.How to best use this to update GNOME
channels/nixos-unstablebranch.nix-shell maintainers/scripts/update.nix --argstr maintainer jtojnar --argstr commit truenix run github:jtojnar/what-changed between-commits <base-commit>(currently only support projects hosted on GNOME servers)git commit --fixup <package>Tabto create a fixup commit. (Or--squashto be able to add extra content to the old commit message.)gnome-3.38branchgit rebase --interactive --autosquash <base-commit>