maintainers/scripts/update: Allow updating in (reverse) topological order#386490
Merged
jtojnar merged 10 commits intoNixOS:masterfrom Mar 11, 2025
Merged
maintainers/scripts/update: Allow updating in (reverse) topological order#386490jtojnar merged 10 commits intoNixOS:masterfrom
jtojnar merged 10 commits intoNixOS:masterfrom
Conversation
`list`, `dict` and `tuple` can accept generic arguments since Python 3.9: https://docs.python.org/3.9/whatsnew/3.9.html#type-hinting-generics-in-standard-collections `T | None` can be used instead of `Optional` since 3.10: https://docs.python.org/3.10/whatsnew/3.10.html#pep-604-new-type-union-operator
This was revealed when we added return type to `check_subprocess_output`.
I believe this is only relevant if we were to `join` the queue itself but it is a good practice anyway. https://docs.python.org/3/library/asyncio-queue.html#asyncio.Queue.task_done
github-actions
bot
added
10.rebuild-darwin: 0
jtojnar
force-pushed
the
update-topo
branch
2 times, most recently
from
8a58f2e to
0e53ba5
Compare
github-actions
bot
added
6.topic: GNOME
…ot one This can happen e.g. due to an error during `merge_changes` since we do not do `stderr=PIPE` for git commands.
…straint We only allow a single value for `attrPath` across all sequenced update scripts. But previously `null` (representing `passthru.updateScript.attrPath` not being defined) was counted as one value. This would prevent us from explicitly specifying `attrPath` in `gnome.updateScript` in the next commit. Let’s ignore update scripts without specified `attrPath` attribute for the purpose of this check.
Without this, `update.nix` will discover `libxml2` as `emscriptenPackages.libxml2`. But since that instantiates a different, and much less common derivation, it will be erroneously considered a leaf package in topological order of packages.
Just minor refactorings: - Extract `updater_tasks` into a variable. - Make `main` itself async.
…rder
Previously, when updating multiple packages, we just updated them in arbitrary order. However, when some of those packages depended on each other, it could happen that some of the intermediary commits would not build because of version constraints on dependencies.
If we want each commit in the history to build when feasible, we need to consider four different scenarios:
1. Updated dependant is compatible with both the old and the new version of the dependency. Order of commits does not matter. But updating dependents first (i.e. reverse topological order) is useful since it allows building each package on the commit that updates it with minimal rebuilds.
2. Updated dependant raises the minimal dependency version. Dependency needs to be updated first (i.e. topological order).
3. Old dependant sets the maximal dependency version. Dependant needs to be updated first (i.e. reverse topological order).
4. Updated dependant depends on exact version of dependency and they are expected to be updated in lockstep. The earlier commit will be broken no matter the order.
This change allows selecting the order of updates to facilitate the first three scenarios. Since most package sets only have loose version constraints, the reverse topological order will generally be the most convenient. In major package set updates like bumping GNOME release, there will be exceptions (e.g. libadwaita typically requires GTK 4 from the same release) but those were probably in broken order before as well.
The downside of this feature is that it is quite slow – it requires instantiating each package and then querying Nix store for requisites.
It may also fail to detect dependency if there are multiple variants of the package and dependant uses a different one than the canonical one.
Testing with:
env GNOME_UPDATE_STABILITY=unstable NIX_PATH=nixpkgs=$HOME/Projects/nixpkgs nix-shell maintainers/scripts/update.nix --arg predicate '(path: pkg: path == ["gnome-shell"] || path == ["mutter"] || path == ["glib"] || path == ["gtk3"] || path == ["pango"] || path == ["gnome-text-editor"])' --argstr order reverse-topological --argstr commit true --argstr max-workers 4
Merged
Member
Author
|
Examples of where reverse topological order failed in #386514 due to bumped min version in the dependent:
Not much we can do about that without analysing sources or build outputs. Examples where the dependency was incorrectly detected:
We could collect alternative attr names when uniqueing them but so far it has not been so common as to bother me into implementing that. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Previously, when updating multiple packages, we just updated them in arbitrary order. However, when some of those packages depended on each other, it could happen that some of the intermediary commits would not build because of version constraints on dependencies.
If we want each commit in the history to build when feasible, we need to consider four different scenarios:
This change allows selecting the order of updates to facilitate the first three scenarios. Since most package sets only have loose version constraints, the reverse topological order will generally be the most convenient. In major package set updates like bumping GNOME release, there will be exceptions (e.g. libadwaita typically requires GTK 4 from the same release) but those were probably in broken order before as well.
The downside of this feature is that it is quite slow – it requires instantiating each package and then querying Nix store for requisites.
It may also fail to detect dependency if there are multiple variants of the package and dependant uses a different one than the canonical one.
Testing with:
Also include some cleanups of the Python script and some fixes of update scripts that were required for updating GNOME to work.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.