Skip to content

Commit

Permalink
Get rid of impureOutputHash
Browse files Browse the repository at this point in the history
I do not believe there is any problem with computing
`hashDerivationModulo` the normal way with impure derivations.

Conversely, the way this used to work is very suspicious because two
almost-equal derivations that only differ in depending on different
impure derivations could have the same drv hash modulo. That is very
suspicious because there is no reason to think those two different
impure derivations will end up producing the same content-addressed
data!

Co-authored-by: Alain Zscheile <[email protected]>
  • Loading branch information
Ericson2314 and fogti committed Feb 12, 2025
1 parent 23259bd commit 50912d0
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 41 deletions.
59 changes: 31 additions & 28 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,41 +185,44 @@ Goal::Co DerivationGoal::haveDerivation()
if (!drv->type().hasKnownOutputPaths())
experimentalFeatureSettings.require(Xp::CaDerivations);

if (drv->type().isImpure()) {
experimentalFeatureSettings.require(Xp::ImpureDerivations);

for (auto & [outputName, output] : drv->outputs) {
auto randomPath = StorePath::random(outputPathName(drv->name, outputName));
assert(!worker.store.isValidPath(randomPath));
initialOutputs.insert({
outputName,
InitialOutput {
.wanted = true,
.outputHash = impureOutputHash,
.known = InitialOutputStatus {
.path = randomPath,
.status = PathStatus::Absent
}
}
});
}

co_return gaveUpOnSubstitution();
}

for (auto & i : drv->outputsAndOptPaths(worker.store))
if (i.second.second)
worker.store.addTempRoot(*i.second.second);

auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
for (auto & [outputName, outputHash] : outputHashes)
initialOutputs.insert({
outputName,
InitialOutput {
{
bool impure = drv->type().isImpure();

if (impure) experimentalFeatureSettings.require(Xp::ImpureDerivations);

auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
for (auto & [outputName, outputHash] : outputHashes) {
InitialOutput v{
.wanted = true, // Will be refined later
.outputHash = outputHash
};

/* TODO we might want to also allow randomizing the paths
for regular CA derivations, e.g. for sake of checking
determinism. */
if (impure) {
v.known = InitialOutputStatus {
.path = StorePath::random(outputPathName(drv->name, outputName)),
.status = PathStatus::Absent,
};
}
});

initialOutputs.insert({
outputName,
std::move(v),
});
}

if (impure) {
/* We don't yet have any safe way to cache an impure derivation at
this step. */
co_return gaveUpOnSubstitution();
}
}

{
/* Check what outputs paths are not already valid. */
Expand Down
12 changes: 1 addition & 11 deletions src/libstore/derivations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -843,16 +843,6 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut
};
}

if (type.isImpure()) {
std::map<std::string, Hash> outputHashes;
for (const auto & [outputName, _] : drv.outputs)
outputHashes.insert_or_assign(outputName, impureOutputHash);
return DrvHash {
.hashes = outputHashes,
.kind = DrvHash::Kind::Deferred,
};
}

auto kind = std::visit(overloaded {
[](const DerivationType::InputAddressed & ia) {
/* This might be a "pesimistically" deferred output, so we don't
Expand All @@ -865,7 +855,7 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut
: DrvHash::Kind::Deferred;
},
[](const DerivationType::Impure &) -> DrvHash::Kind {
assert(false);
return DrvHash::Kind::Deferred;
}
}, drv.type().raw);

Expand Down
2 changes: 0 additions & 2 deletions src/libstore/derivations.hh
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,4 @@ void writeDerivation(Sink & out, const StoreDirConfig & store, const BasicDeriva
*/
std::string hashPlaceholder(const OutputNameView outputName);

extern const Hash impureOutputHash;

}

0 comments on commit 50912d0

Please sign in to comment.