various: simplify handling of some *Phases in setup hooks#360450
Merged
wolfgangwalther merged 4 commits intoNixOS:stagingfrom Dec 14, 2024
Merged
Conversation
github-actions
bot
added
the
6.topic: python
13 tasks
ofborg
bot
added
10.rebuild-darwin-stdenv
wolfgangwalther
requested review from
a team,
7c6f434c,
Artturin,
dotlambda and
lovek323
jtojnar
reviewed
Dec 5, 2024
jtojnar
reviewed
Dec 5, 2024
dotlambda
reviewed
Dec 7, 2024
wolfgangwalther
force-pushed
the
structured-attrs-pre-fixup-dist-phases-staging
branch
from
e546221 to
9b3f7db
Compare
Contributor
Author
|
I addressed the feedback. Once CI / ofborg has run through, this should be good to go. |
github-actions
bot
removed
10.rebuild-darwin-stdenv
This avoids complicated logic to make sure the preFixupPhase is in the right place relative to gappsWrapperArgsHook. What glib's hook is doing here ultimately belons to the install phase anyway.
…Hook Same reasoning as commit before.
We need to make sure that pytest-forked's setup hook runs before the pytestCheckPhase. Instead of mangling the preDistPhases in a complex way, we can just set our environment variables in postInstallCheckHook, which is run right before all preDistPhases.
Same reasoning as commit before.
wolfgangwalther
force-pushed
the
structured-attrs-pre-fixup-dist-phases-staging
branch
from
9b3f7db to
0cd7dfa
Compare
Contributor
Author
|
Rebased to fix ofborg-eval failure. |
ofborg
bot
added
10.rebuild-darwin-stdenv
philiptaron
approved these changes
Dec 14, 2024
Contributor
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
I built a few of these (glib, gobject-introspection, python3Packages.pytest-forked, python3Packages.pytest-xdist). ✅.
wolfgangwalther
deleted the
structured-attrs-pre-fixup-dist-phases-staging
branch
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
4 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.
This is to unblock #352709 and remove all the special cases of
preFixupPhasesandpreDistPhases.My reasoning can be found in #352709 (review) and following. I did not move
pytestCheckHookto the installCheckPhase - we don't need that and can still usepostInstallHooksin pytest-forked and pytest-xdist.I'm very sure that this works - but I couldn't verify against a broken case. Just removing the
ifetc. didn't show any obvious breakage in the few random packages that I looked at.cc @ShamrockLee @emilazy
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.