-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Add --rev to update-source-version, add a generic unstable updater #101083
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the u-s-v change.
The update script itself looks fine. But we might want to implement it using genericUpdater
, cc @romildo. Though it might need changes before it is usable (e.g. make it more flexible, do not create fileForGitCommands
now that update.nix
supports auto-comitting).
|
||
# update the nix expression | ||
${common-updater-scripts}/bin/update-source-version "$attr_path" \ | ||
"unstable-$commit_date" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relevant: #100833
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether to leave it as is until #100833 is in, or to make it configurable by taking a makeVersionString
function as an argument (with default date → "unstable-$date"
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll leave it as is for now
pkgs/top-level/all-packages.nix
Outdated
@@ -94,6 +94,8 @@ in | |||
|
|||
genericUpdater = callPackage ../common-updater/generic-updater.nix { }; | |||
|
|||
unstableUpdater = callPackage ../common-updater/unstable-updater.nix { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably should make the name signal that it is git specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I based the name on genericUpdater. Should I rename that one too or is it too late?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
genericUpdater
is quite fresh an undocumented so I do not see an issue with renaming it. We can leave it for a separate PR, though, if we decide to not use it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I named it "unstableGitUpdater", not sure if it's clear enough
I considered that, but
|
7c80c3b
to
6f3c871
Compare
Ping |
Could you autosquash the fixup commits and fix the evaluation? |
Adds a --rev=<revision> parameter to the script that makes it possible to explicitly specify a new revision. Useful to update unstable packages, where the version and revision may be independent.
d0e0b51
to
97e5fc3
Compare
Looks good, thanks. |
Motivation for this change
Unstable packages are tedious to keep up to date. You have to manually check the repository for new commits, update the version to HEAD's commit date, and update the revision to HEAD's sha.
This patch adds a
--rev
parameter toupdate-source-version
(necessary since in unstable packages version and rev are independent), and a generic unstable updater to handle the case described above.Finally, in the last commit, the updater is applied to a package,
qbe
, that benefits from it, and on which you can test the updater.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)