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

Add --rev to update-source-version, add a generic unstable updater #101083

Merged
merged 3 commits into from Nov 21, 2020

Conversation

@fgaz
Copy link
Member

@fgaz fgaz commented Oct 19, 2020

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 to update-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
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Copy link
Contributor

@jtojnar jtojnar left a comment

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).

pkgs/common-updater/unstable-updater.nix Outdated Show resolved Hide resolved
pkgs/common-updater/unstable-updater.nix Outdated Show resolved Hide resolved
# update the nix expression
${common-updater-scripts}/bin/update-source-version "$attr_path" \
"unstable-$commit_date" \
Copy link
Contributor

@jtojnar jtojnar Oct 19, 2020

Relevant: #100833

Copy link
Member Author

@fgaz fgaz Oct 20, 2020

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")

Copy link
Member Author

@fgaz fgaz Oct 20, 2020

I think I'll leave it as is for now

pkgs/common-updater/unstable-updater.nix Outdated Show resolved Hide resolved
@@ -94,6 +94,8 @@ in

genericUpdater = callPackage ../common-updater/generic-updater.nix { };

unstableUpdater = callPackage ../common-updater/unstable-updater.nix { };
Copy link
Contributor

@jtojnar jtojnar Oct 19, 2020

We probably should make the name signal that it is git specific.

Copy link
Member Author

@fgaz fgaz Oct 20, 2020

I based the name on genericUpdater. Should I rename that one too or is it too late?

Copy link
Contributor

@jtojnar jtojnar Oct 20, 2020

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.

Copy link
Member Author

@fgaz fgaz Oct 20, 2020

Done

Copy link
Member Author

@fgaz fgaz Oct 20, 2020

I named it "unstableGitUpdater", not sure if it's clear enough

@fgaz
Copy link
Member Author

@fgaz fgaz commented Oct 20, 2020

@jtojnar

we might want to implement it using genericUpdater

I considered that, but

  • it would require at least a versionSorter, a versionFilter, maybe a makeVersion parameters
  • and/or a way to trigger the --rev behavior
    • which is pretty much specific to unstable versions
  • unstableUpdater can make shallow clones, potentially using much less resources (edit: nevermind, we probably get a full clone with the later commands anyway)
  • unstableUpdater is quite simple on its own

@fgaz fgaz force-pushed the unstable-updater branch from 7c80c3b to 6f3c871 Oct 20, 2020
@fgaz
Copy link
Member Author

@fgaz fgaz commented Nov 20, 2020

Ping

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Nov 20, 2020

Could you autosquash the fixup commits and fix the evaluation?

fgaz added 3 commits Nov 21, 2020
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.
@fgaz fgaz force-pushed the unstable-updater branch from d0e0b51 to 97e5fc3 Nov 21, 2020
@jtojnar jtojnar merged commit a0efbc1 into NixOS:master Nov 21, 2020
19 of 20 checks passed
@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Nov 21, 2020

Looks good, thanks.

@fgaz fgaz deleted the unstable-updater branch Nov 21, 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

2 participants