Skip to content

Commit 2ee4197

Browse files
Ericson2314Ericson2314
committed
Fix #13247
Resolve the derivation before creating a building goal, in a context where we know what output(s) we want. That way we have a chance just to download the outputs we want. Fix #13247 (cherry picked from commit 39f6fd9)
1 parent 6642ffb commit 2ee4197

File tree

3 files changed

+92
-107
lines changed

3 files changed

+92
-107
lines changed

‎src/libstore/build/derivation-building-goal.cc‎

Lines changed: 0 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#include "nix/store/build/derivation-building-goal.hh"
2-
#include "nix/store/build/derivation-resolution-goal.hh"
32
#include "nix/store/build/derivation-env-desugar.hh"
4-
#include "nix/store/build/derivation-trampoline-goal.hh"
53
#ifndef _WIN32 // TODO enable build hook on Windows
64
# include "nix/store/build/hook-instance.hh"
75
# include "nix/store/build/derivation-builder.hh"
@@ -158,107 +156,6 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
158156
/* Determine the full set of input paths. */
159157

160158
{
161-
auto resolutionGoal = worker.makeDerivationResolutionGoal(drvPath, *drv, buildMode);
162-
{
163-
Goals waitees{resolutionGoal};
164-
co_await await(std::move(waitees));
165-
}
166-
if (nrFailed != 0) {
167-
co_return doneFailure({BuildResult::Failure::DependencyFailed, "resolution failed"});
168-
}
169-
170-
if (resolutionGoal->resolvedDrv) {
171-
auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv;
172-
173-
/* Store the resolved derivation, as part of the record of
174-
what we're actually building */
175-
writeDerivation(worker.store, drvResolved);
176-
177-
/* TODO https://github.com/NixOS/nix/issues/13247 we should
178-
let the calling goal do this, so it has a change to pass
179-
just the output(s) it cares about. */
180-
auto resolvedDrvGoal =
181-
worker.makeDerivationTrampolineGoal(pathResolved, OutputsSpec::All{}, drvResolved, buildMode);
182-
{
183-
Goals waitees{resolvedDrvGoal};
184-
co_await await(std::move(waitees));
185-
}
186-
187-
trace("resolved derivation finished");
188-
189-
auto resolvedResult = resolvedDrvGoal->buildResult;
190-
191-
// No `std::visit` for coroutines yet
192-
if (auto * successP = resolvedResult.tryGetSuccess()) {
193-
auto & success = *successP;
194-
SingleDrvOutputs builtOutputs;
195-
196-
auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
197-
auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
198-
199-
StorePathSet outputPaths;
200-
201-
for (auto & outputName : drvResolved.outputNames()) {
202-
auto outputHash = get(outputHashes, outputName);
203-
auto resolvedHash = get(resolvedHashes, outputName);
204-
if ((!outputHash) || (!resolvedHash))
205-
throw Error(
206-
"derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)",
207-
worker.store.printStorePath(drvPath),
208-
outputName);
209-
210-
auto realisation = [&] {
211-
auto take1 = get(success.builtOutputs, outputName);
212-
if (take1)
213-
return *take1;
214-
215-
/* The above `get` should work. But stateful tracking of
216-
outputs in resolvedResult, this can get out of sync with the
217-
store, which is our actual source of truth. For now we just
218-
check the store directly if it fails. */
219-
auto take2 = worker.evalStore.queryRealisation(DrvOutput{*resolvedHash, outputName});
220-
if (take2)
221-
return *take2;
222-
223-
throw Error(
224-
"derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)",
225-
worker.store.printStorePath(pathResolved),
226-
outputName);
227-
}();
228-
229-
if (!drv->type().isImpure()) {
230-
auto newRealisation = realisation;
231-
newRealisation.id = DrvOutput{*outputHash, outputName};
232-
newRealisation.signatures.clear();
233-
if (!drv->type().isFixed()) {
234-
auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store;
235-
newRealisation.dependentRealisations =
236-
drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore);
237-
}
238-
worker.store.signRealisation(newRealisation);
239-
worker.store.registerDrvOutput(newRealisation);
240-
}
241-
outputPaths.insert(realisation.outPath);
242-
builtOutputs.emplace(outputName, realisation);
243-
}
244-
245-
runPostBuildHook(worker.store, *logger, drvPath, outputPaths);
246-
247-
auto status = success.status;
248-
if (status == BuildResult::Success::AlreadyValid)
249-
status = BuildResult::Success::ResolvesToAlreadyValid;
250-
251-
co_return doneSuccess(success.status, std::move(builtOutputs));
252-
} else if (resolvedResult.tryGetFailure()) {
253-
co_return doneFailure({
254-
BuildResult::Failure::DependencyFailed,
255-
"build of resolved derivation '%s' failed",
256-
worker.store.printStorePath(pathResolved),
257-
});
258-
} else
259-
assert(false);
260-
}
261-
262159
/* If we get this far, we know no dynamic drvs inputs */
263160

264161
for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) {

‎src/libstore/build/derivation-goal.cc‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "nix/store/build/derivation-goal.hh"
22
#include "nix/store/build/derivation-building-goal.hh"
3+
#include "nix/store/build/derivation-resolution-goal.hh"
34
#ifndef _WIN32 // TODO enable build hook on Windows
45
# include "nix/store/build/hook-instance.hh"
56
# include "nix/store/build/derivation-builder.hh"
@@ -140,6 +141,96 @@ Goal::Co DerivationGoal::haveDerivation()
140141
worker.store.printStorePath(drvPath));
141142
}
142143

144+
auto resolutionGoal = worker.makeDerivationResolutionGoal(drvPath, *drv, buildMode);
145+
{
146+
Goals waitees{resolutionGoal};
147+
co_await await(std::move(waitees));
148+
}
149+
if (nrFailed != 0) {
150+
co_return doneFailure({BuildResult::Failure::DependencyFailed, "resolution failed"});
151+
}
152+
153+
if (resolutionGoal->resolvedDrv) {
154+
auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv;
155+
156+
/* Store the resolved derivation, as part of the record of
157+
what we're actually building */
158+
writeDerivation(worker.store, drvResolved);
159+
160+
auto resolvedDrvGoal = worker.makeDerivationGoal(pathResolved, drvResolved, wantedOutput, buildMode);
161+
{
162+
Goals waitees{resolvedDrvGoal};
163+
co_await await(std::move(waitees));
164+
}
165+
166+
trace("resolved derivation finished");
167+
168+
auto resolvedResult = resolvedDrvGoal->buildResult;
169+
170+
// No `std::visit` for coroutines yet
171+
if (auto * successP = resolvedResult.tryGetSuccess()) {
172+
auto & success = *successP;
173+
auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
174+
auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
175+
176+
StorePathSet outputPaths;
177+
178+
auto outputHash = get(outputHashes, wantedOutput);
179+
auto resolvedHash = get(resolvedHashes, wantedOutput);
180+
if ((!outputHash) || (!resolvedHash))
181+
throw Error(
182+
"derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)",
183+
worker.store.printStorePath(drvPath),
184+
wantedOutput);
185+
186+
auto realisation = [&] {
187+
auto take1 = get(success.builtOutputs, wantedOutput);
188+
if (take1)
189+
return *take1;
190+
191+
/* The above `get` should work. But stateful tracking of
192+
outputs in resolvedResult, this can get out of sync with the
193+
store, which is our actual source of truth. For now we just
194+
check the store directly if it fails. */
195+
auto take2 = worker.evalStore.queryRealisation(DrvOutput{*resolvedHash, wantedOutput});
196+
if (take2)
197+
return *take2;
198+
199+
throw Error(
200+
"derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)",
201+
worker.store.printStorePath(pathResolved),
202+
wantedOutput);
203+
}();
204+
205+
if (!drv->type().isImpure()) {
206+
auto newRealisation = realisation;
207+
newRealisation.id = DrvOutput{*outputHash, wantedOutput};
208+
newRealisation.signatures.clear();
209+
if (!drv->type().isFixed()) {
210+
auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store;
211+
newRealisation.dependentRealisations =
212+
drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore);
213+
}
214+
worker.store.signRealisation(newRealisation);
215+
worker.store.registerDrvOutput(newRealisation);
216+
}
217+
outputPaths.insert(realisation.outPath);
218+
219+
auto status = success.status;
220+
if (status == BuildResult::Success::AlreadyValid)
221+
status = BuildResult::Success::ResolvesToAlreadyValid;
222+
223+
co_return doneSuccess(status, std::move(realisation));
224+
} else if (resolvedResult.tryGetFailure()) {
225+
co_return doneFailure({
226+
BuildResult::Failure::DependencyFailed,
227+
"build of resolved derivation '%s' failed",
228+
worker.store.printStorePath(pathResolved),
229+
});
230+
} else
231+
assert(false);
232+
}
233+
143234
/* Give up on substitution for the output we want, actually build this derivation */
144235

145236
auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode);

‎tests/functional/ca/issue-13247.sh‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,4 @@ buildViaSubstitute use-a-prime-more-outputs^first
6565
# Should only fetch the output we asked for
6666
[[ -d "$(jq -r <"$TEST_ROOT"/a.json '.[0].outputs.out')" ]]
6767
[[ -f "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.first')" ]]
68-
69-
# Output should *not* be here, this is the bug
70-
[[ -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]]
71-
skipTest "bug is not yet fixed"
68+
[[ ! -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]]

0 commit comments

Comments
 (0)