Get rid of addWantedOutputs
#13240
Open
Get rid of addWantedOutputs
#13240
+409
−335
Conversation
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
github-actions
bot
added
with-tests
Ericson2314
force-pushed
the
dyn-drv-take-3
branch
from
15d4fb0 to
3969fcb
Compare
Ericson2314
changed the title
DerivationCreationAndRealisationGoal, get rid of addWantedOutputsaddWantedOutputs
Due this by introducing `DerivationCreationAndRealisationGoal`. `DerivationGoal` now only tracks a single output, and is back to tracking a plain store path `drvPath`, not a deriving path one. Its `addWantedOutputs` method is gone. These changes will allow subsequent PRs to simplify it greatly. Because the purpose of each goal is back to being immutable, we can also once again make `Goal::buildResult` a public field, and get rid of the `getBuildResult` method. This simplifies things also. `DerivationCreationAndRealisationGoal` is a cheap "trampoline" goal. It takes immutable sets of wanted outputs, and just kicks of `DerivationGoal`s for them. Since now "actual work" is done in these goals, it is not wasteful to have separate ones for separate sets of outputs, even if those outputs (and the derivations they are from) overlap. This separation of concerns will make it possible for future work on issues like NixOS#11928, and to continue the path of having more goal types, but each goal type does fewer things (issue NixOS#12628). This commit in some sense reverts f4f28cd, but that one kept around `addWantedOutputs`. I am quite sure it was having two layers of goals with `addWantedOutputs` that caused the issues --- restarting logic like `addWantedOutputs` has is very tempermental! In this version of the change, we have *zero* layers of `addWantedOutputs` --- no goal type needs it, or otherwise has a mutable objective --- and so I think this change is safe.
Ericson2314
force-pushed
the
dyn-drv-take-3
branch
from
3969fcb to
8a9bde3
Compare
|
🎉 All dependencies have been resolved ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
DerivationGoalnow only tracks a single output, and is back to tracking a plain store pathdrvPath, not a deriving path one. ItsaddWantedOutputsmethod is gone. These changes will allow subsequent PRs to simplify it greatly.Because the purpose of each goal is back to being immutable, we can also once again make
Goal::buildResulta public field, and get rid of thegetBuildResultmethod. This simplifies things also.DerivationCreationAndRealisationGoalis a cheap "trampoline" goal. It takes immutable sets of wanted outputs, and just kicks ofDerivationGoals for them. Since now "actual work" is done in these goals, it is not wasteful to have separate ones for separate sets of outputs, even if those outputs (and the derivations they are from) overlap.This separation of concerns will make it possible for future work on issues like #11928, and to continue the path of having more goal types, but each goal type does fewer things (issue #12628).
Context
This commit in some sense reverts f4f28cd, but that one kept around
addWantedOutputs. I am quite sure it was having two layers of goals withaddWantedOutputsthat caused the issues --- restarting logic likeaddWantedOutputshas is very tempermental! In this version of the change, we have zero layers ofaddWantedOutputs--- no goal type needs it, or otherwise has a mutable objective --- and so I think this change is safe.depends on #13186
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.