xen: finish moving to by-name; refactor#406638
Merged
philiptaron merged 5 commits intoNixOS:masterfrom Jul 6, 2025
Merged
Conversation
SigmaSquadron
added
the
2.status: wait for branch‐off
github-actions
bot
added
the
6.topic: xen-project
SigmaSquadron
requested review from
a team,
CertainLach,
bengsparks,
digitalrane and
hehongbo
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
ad5eb0a to
67a5f8d
Compare
github-actions
bot
added
10.rebuild-darwin: 0
hehongbo
approved these changes
May 13, 2025
hehongbo
added
the
12.approvals: 1
drupol
reviewed
May 13, 2025
Contributor
drupol
left a comment
drupol
left a comment
There was a problem hiding this comment.
Added a minor opinonated feedback to switch from optional to the more explicit optionals (singular to plural form) to reduce the cognitive load for newcomers.
IMO, it also makes sense to me to use it when dealing with lists (plural form), like buildInputs or knownVulnerabilities.
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
67a5f8d to
51db1fb
Compare
CertainLach
reviewed
May 13, 2025
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
51db1fb to
8a20062
Compare
CertainLach
approved these changes
May 13, 2025
philiptaron
approved these changes
May 13, 2025
Contributor
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
It's a pretty steep deprecation, but I trust the Xen project to judge whether that's too fast.
Contributor
Author
|
I should clarify that as a breaking change, this will need to wait for the branch-off. |
wegank
added
12.approvals: 3+
13 tasks
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
8a20062 to
9c56049
Compare
SigmaSquadron
removed
the
2.status: wait for branch‐off
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
9c56049 to
83cf4af
Compare
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
83cf4af to
0a95f78
Compare
nixpkgs-ci
bot
added
the
11.by: package-maintainer
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
2 times, most recently
from
85e0b92 to
fcf0f22
Compare
philiptaron
approved these changes
Jul 3, 2025
Contributor
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
Lots of agreement with the TODOs in the derivation.
Deprecation schedule now looks quite sane.
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
fcf0f22 to
9dc73c0
Compare
Contributor
Author
|
behold the unholy patch (doing this was quite nice. I managed to find several dependencies that weren't in the PATH wrapper) |
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
2 times, most recently
from
59f5d55 to
09636ee
Compare
This causes the removal of buildXenPackage, as the recommended way to build a custom Xen is to override the package. Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
The old `buildXenPackage` builder was a curried function that had to be completed with a second call with attributes like `pname` and `version`. As it is no longer necessary to call a generic builder to build custom variants of Xen, the package must comply with the `by-name` checks, which include the requirement that all `package.nix` files in this directory hierarchy resolve to a derivation. (and not a function) Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Shuffles the input lists to ensure Xen works with `strictDeps` enabled. There are more derivations in `nativeBuildInputs` than strictly necessary for a successful build, but Xen calls for the OCaml and dev86 binaries during certain situations when building certain components. Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Changes some (long overdue) TODO comments and uses `lib.getExe` instead of hardcoding /bin paths. Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
2 times, most recently
from
308cd14 to
fe2444a
Compare
philiptaron
requested changes
Jul 5, 2025
Contributor
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
My quick rg math says that dropping the three IMO erroneous substitutions of echo, kill, and test (which are already dependency injected through patching the bash shebang at the top) drops 203 mutations in the patch file.
This looks like the right direction to me, though, megapatch though it may be.
pkgs/by-name/xe/xen/0002-scripts-external-executable-calls.patch
Outdated
Show resolved
Hide resolved
This replaces the brittle PATH wrapper and substituteInPlace calls that ensured the systemd services and the Xen scripts had all the necessary dependencies. The new patch can replace the calls to external executables much more effectively. Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
SigmaSquadron
force-pushed
the
push-upkkwyrutmwn
branch
from
fe2444a to
5c3b43a
Compare
philiptaron
approved these changes
Jul 6, 2025
| "WGET=${getExe' coreutils "false"}" | ||
| "EFI_VENDOR=${finalAttrs.vendor}" | ||
| "INSTALL_EFI_STRIP=1" | ||
| "LD=${getExe' binutils-unwrapped-all-targets "ld"}" |
Contributor
There was a problem hiding this comment.
I suspect we'll need to replace this with something like the following
preBuild = ''
appendToVar makeFlags "LD=$(LD)"
'';
due to building with cross-compilation.
Contributor
|
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
6 participants
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.
Follow-up to the second Xen refactor:
This finally moves the builder to
by-name, as we can now easily override Xen for a custom build thanks tofinalAttrs.Also does some minor refactoring, like enabling
strictDepsand usinggetExe.Things done
nix-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.