diff options
| -rw-r--r-- | src/libmain/progress-bar.cc | 2 | ||||
| -rw-r--r-- | src/libstore/build.cc | 88 | ||||
| -rw-r--r-- | src/libstore/derivations.cc | 57 | ||||
| -rw-r--r-- | src/libstore/derivations.hh | 29 | ||||
| -rw-r--r-- | src/libstore/local-store.cc | 50 | ||||
| -rw-r--r-- | tests/content-addressed.nix | 22 | ||||
| -rw-r--r-- | tests/content-addressed.sh | 25 | ||||
| -rw-r--r-- | tests/tarball.sh | 2 |
8 files changed, 235 insertions, 40 deletions
diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index be3c06a38..07b45b3b5 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -256,7 +256,7 @@ public: } else if (type == resBuildLogLine || type == resPostBuildLogLine) { - auto lastLine = trim(getS(fields, 0)); + auto lastLine = chomp(getS(fields, 0)); if (!lastLine.empty()) { auto i = state->its.find(act); assert(i != state->its.end()); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 83fbe89a0..0499273a4 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1008,6 +1008,8 @@ private: void tryLocalBuild(); void buildDone(); + void resolvedFinished(); + /* Is the build hook willing to perform the build? */ HookReply tryBuildHook(); @@ -1483,8 +1485,40 @@ void DerivationGoal::inputsRealised() /* Determine the full set of input paths. */ /* First, the input derivations. */ - if (useDerivation) - for (auto & [depDrvPath, wantedDepOutputs] : dynamic_cast<Derivation *>(drv.get())->inputDrvs) { + if (useDerivation) { + auto & fullDrv = *dynamic_cast<Derivation *>(drv.get()); + + if (!fullDrv.inputDrvs.empty() && fullDrv.type() == DerivationType::CAFloating) { + /* We are be able to resolve this derivation based on the + now-known results of dependencies. If so, we become a stub goal + aliasing that resolved derivation goal */ + std::optional attempt = fullDrv.tryResolve(worker.store); + assert(attempt); + Derivation drvResolved { *std::move(attempt) }; + + auto pathResolved = writeDerivation(worker.store, drvResolved); + /* Add to memotable to speed up downstream goal's queries with the + original derivation. */ + drvPathResolutions.lock()->insert_or_assign(drvPath, pathResolved); + + auto msg = fmt("Resolved derivation: '%s' -> '%s'", + worker.store.printStorePath(drvPath), + worker.store.printStorePath(pathResolved)); + act = std::make_unique<Activity>(*logger, lvlInfo, actBuildWaiting, msg, + Logger::Fields { + worker.store.printStorePath(drvPath), + worker.store.printStorePath(pathResolved), + }); + + auto resolvedGoal = worker.makeDerivationGoal( + pathResolved, wantedOutputs, buildMode); + addWaitee(resolvedGoal); + + state = &DerivationGoal::resolvedFinished; + return; + } + + for (auto & [depDrvPath, wantedDepOutputs] : fullDrv.inputDrvs) { /* Add the relevant output closures of the input derivation `i' as input paths. Only add the closures of output paths that are specified as inputs. */ @@ -1504,6 +1538,7 @@ void DerivationGoal::inputsRealised() worker.store.printStorePath(drvPath), j, worker.store.printStorePath(drvPath)); } } + } /* Second, the input sources. */ worker.store.computeFSClosure(drv->inputSrcs, inputPaths); @@ -1631,6 +1666,13 @@ void DerivationGoal::tryToBuild() actLock.reset(); + state = &DerivationGoal::tryLocalBuild; + worker.wakeUp(shared_from_this()); +} + +void DerivationGoal::tryLocalBuild() { + bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store); + /* Make sure that we are allowed to start a build. If this derivation prefers to be done locally, do it even if maxBuildJobs is 0. */ @@ -1641,12 +1683,6 @@ void DerivationGoal::tryToBuild() return; } - state = &DerivationGoal::tryLocalBuild; - worker.wakeUp(shared_from_this()); -} - -void DerivationGoal::tryLocalBuild() { - /* If `build-users-group' is not empty, then we have to build as one of the members of that group. */ if (settings.buildUsersGroup != "" && getuid() == 0) { @@ -1961,6 +1997,9 @@ void DerivationGoal::buildDone() done(BuildResult::Built); } +void DerivationGoal::resolvedFinished() { + done(BuildResult::Built); +} HookReply DerivationGoal::tryBuildHook() { @@ -4256,11 +4295,13 @@ void DerivationGoal::registerOutputs() /* Register each output path as valid, and register the sets of paths referenced by each of them. If there are cycles in the outputs, this will fail. */ - ValidPathInfos infos2; - for (auto & [outputName, newInfo] : infos) { - infos2.push_back(newInfo); + { + ValidPathInfos infos2; + for (auto & [outputName, newInfo] : infos) { + infos2.push_back(newInfo); + } + worker.store.registerValidPaths(infos2); } - worker.store.registerValidPaths(infos2); /* In case of a fixed-output derivation hash mismatch, throw an exception now that we have registered the output as valid. */ @@ -4272,12 +4313,21 @@ void DerivationGoal::registerOutputs() means it's safe to link the derivation to the output hash. We must do that for floating CA derivations, which otherwise couldn't be cached, but it's fine to do in all cases. */ - for (auto & [outputName, newInfo] : infos) { - /* FIXME: we will want to track this mapping in the DB whether or - not we have a drv file. */ - if (useDerivation) - worker.store.linkDeriverToPath(drvPath, outputName, newInfo.path); + bool isCaFloating = drv->type() == DerivationType::CAFloating; + + auto drvPathResolved = drvPath; + if (!useDerivation && isCaFloating) { + /* Once a floating CA derivations reaches this point, it + must already be resolved, so we don't bother trying to + downcast drv to get would would just be an empty + inputDrvs field. */ + Derivation drv2 { *drv }; + drvPathResolved = writeDerivation(worker.store, drv2); } + + if (useDerivation || isCaFloating) + for (auto & [outputName, newInfo] : infos) + worker.store.linkDeriverToPath(drvPathResolved, outputName, newInfo.path); } @@ -4567,7 +4617,7 @@ void DerivationGoal::flushLine() std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDerivationOutputMap() { - if (drv->type() != DerivationType::CAFloating) { + if (!useDerivation || drv->type() != DerivationType::CAFloating) { std::map<std::string, std::optional<StorePath>> res; for (auto & [name, output] : drv->outputs) res.insert_or_assign(name, output.path(worker.store, drv->name, name)); @@ -4579,7 +4629,7 @@ std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDeri OutputPathMap DerivationGoal::queryDerivationOutputMap() { - if (drv->type() != DerivationType::CAFloating) { + if (!useDerivation || drv->type() != DerivationType::CAFloating) { OutputPathMap res; for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) res.insert_or_assign(name, *output.second); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 9d8ce5e36..2612f1ff7 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -69,7 +69,7 @@ bool BasicDerivation::isBuiltin() const StorePath writeDerivation(Store & store, - const Derivation & drv, RepairFlag repair) + const Derivation & drv, RepairFlag repair, bool readOnly) { auto references = drv.inputSrcs; for (auto & i : drv.inputDrvs) @@ -79,7 +79,7 @@ StorePath writeDerivation(Store & store, held during a garbage collection). */ auto suffix = std::string(drv.name) + drvExtension; auto contents = drv.unparse(store, false); - return settings.readOnlyMode + return readOnly || settings.readOnlyMode ? store.computeStorePathForText(suffix, contents, references) : store.addTextToStore(suffix, contents, references, repair); } @@ -644,4 +644,57 @@ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath return "/" + hashString(htSHA256, clearText).to_string(Base32, false); } + +// N.B. Outputs are left unchanged +static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) { + + debug("Rewriting the derivation"); + + for (auto &rewrite: rewrites) { + debug("rewriting %s as %s", rewrite.first, rewrite.second); + } + + drv.builder = rewriteStrings(drv.builder, rewrites); + for (auto & arg: drv.args) { + arg = rewriteStrings(arg, rewrites); + } + + StringPairs newEnv; + for (auto & envVar: drv.env) { + auto envName = rewriteStrings(envVar.first, rewrites); + auto envValue = rewriteStrings(envVar.second, rewrites); + newEnv.emplace(envName, envValue); + } + drv.env = newEnv; +} + + +Sync<DrvPathResolutions> drvPathResolutions; + +std::optional<BasicDerivation> Derivation::tryResolve(Store & store) { + BasicDerivation resolved { *this }; + + // Input paths that we'll want to rewrite in the derivation + StringMap inputRewrites; + + for (auto & input : inputDrvs) { + auto inputDrvOutputs = store.queryPartialDerivationOutputMap(input.first); + StringSet newOutputNames; + for (auto & outputName : input.second) { + auto actualPathOpt = inputDrvOutputs.at(outputName); + if (!actualPathOpt) + return std::nullopt; + auto actualPath = *actualPathOpt; + inputRewrites.emplace( + downstreamPlaceholder(store, input.first, outputName), + store.printStorePath(actualPath)); + resolved.inputSrcs.insert(std::move(actualPath)); + } + } + + rewriteDerivation(store, resolved, inputRewrites); + + return resolved; +} + } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 0b5652685..d48266774 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -4,6 +4,7 @@ #include "types.hh" #include "hash.hh" #include "content-address.hh" +#include "sync.hh" #include <map> #include <variant> @@ -100,7 +101,7 @@ struct BasicDerivation StringPairs env; std::string name; - BasicDerivation() { } + BasicDerivation() = default; virtual ~BasicDerivation() { }; bool isBuiltin() const; @@ -127,7 +128,17 @@ struct Derivation : BasicDerivation std::string unparse(const Store & store, bool maskOutputs, std::map<std::string, StringSet> * actualInputs = nullptr) const; - Derivation() { } + /* Return the underlying basic derivation but with these changes: + + 1. Input drvs are emptied, but the outputs of them that were used are + added directly to input sources. + + 2. Input placeholders are replaced with realized input store paths. */ + std::optional<BasicDerivation> tryResolve(Store & store); + + Derivation() = default; + Derivation(const BasicDerivation & bd) : BasicDerivation(bd) { } + Derivation(BasicDerivation && bd) : BasicDerivation(std::move(bd)) { } }; @@ -137,7 +148,9 @@ enum RepairFlag : bool { NoRepair = false, Repair = true }; /* Write a derivation to the Nix store, and return its path. */ StorePath writeDerivation(Store & store, - const Derivation & drv, RepairFlag repair = NoRepair); + const Derivation & drv, + RepairFlag repair = NoRepair, + bool readOnly = false); /* Read a derivation from a file. */ Derivation parseDerivation(const Store & store, std::string && s, std::string_view name); @@ -191,6 +204,16 @@ typedef std::map<StorePath, DrvHashModulo> DrvHashes; extern DrvHashes drvHashes; // FIXME: global, not thread-safe +/* Memoisation of `readDerivation(..).resove()`. */ +typedef std::map< + StorePath, + std::optional<StorePath> +> DrvPathResolutions; + +// FIXME: global, though at least thread-safe. +// FIXME: arguably overlaps with hashDerivationModulo memo table. +extern Sync<DrvPathResolutions> drvPathResolutions; + bool wantOutput(const string & output, const std::set<string> & wanted); struct Source; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ee997ef3a..d29236a9c 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -721,7 +721,7 @@ uint64_t LocalStore::queryValidPathId(State & state, const StorePath & path) { auto use(state.stmtQueryPathInfo.use()(printStorePath(path))); if (!use.next()) - throw Error("path '%s' is not valid", printStorePath(path)); + throw InvalidPath("path '%s' is not valid", printStorePath(path)); return use.getInt(0); } @@ -796,18 +796,58 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path) } -std::map<std::string, std::optional<StorePath>> LocalStore::queryPartialDerivationOutputMap(const StorePath & path) +std::map<std::string, std::optional<StorePath>> LocalStore::queryPartialDerivationOutputMap(const StorePath & path_) { + auto path = path_; std::map<std::string, std::optional<StorePath>> outputs; - BasicDerivation drv = readDerivation(path); + Derivation drv = readDerivation(path); for (auto & [outName, _] : drv.outputs) { outputs.insert_or_assign(outName, std::nullopt); } + bool haveCached = false; + { + auto resolutions = drvPathResolutions.lock(); + auto resolvedPathOptIter = resolutions->find(path); + if (resolvedPathOptIter != resolutions->end()) { + auto & [_, resolvedPathOpt] = *resolvedPathOptIter; + if (resolvedPathOpt) + path = *resolvedPathOpt; + haveCached = true; + } + } + /* can't just use else-if instead of `!haveCached` because we need to unlock + `drvPathResolutions` before it is locked in `Derivation::resolve`. */ + if (!haveCached && drv.type() == DerivationType::CAFloating) { + /* Try resolve drv and use that path instead. */ + auto attempt = drv.tryResolve(*this); + if (!attempt) + /* If we cannot resolve the derivation, we cannot have any path + assigned so we return the map of all std::nullopts. */ + return outputs; + /* Just compute store path */ + auto pathResolved = writeDerivation(*this, *std::move(attempt), NoRepair, true); + /* Store in memo table. */ + /* FIXME: memo logic should not be local-store specific, should have + wrapper-method instead. */ + drvPathResolutions.lock()->insert_or_assign(path, pathResolved); + path = std::move(pathResolved); + } return retrySQLite<std::map<std::string, std::optional<StorePath>>>([&]() { auto state(_state.lock()); - auto useQueryDerivationOutputs(state->stmtQueryDerivationOutputs.use() - (queryValidPathId(*state, path))); + uint64_t drvId; + try { + drvId = queryValidPathId(*state, path); + } catch (InvalidPath &) { + /* FIXME? if the derivation doesn't exist, we cannot have a mapping + for it. */ + return outputs; + } + + auto useQueryDerivationOutputs { + state->stmtQueryDerivationOutputs.use() + (drvId) + }; while (useQueryDerivationOutputs.next()) outputs.insert_or_assign( diff --git a/tests/content-addressed.nix b/tests/content-addressed.nix index 5e9bad0ac..3dcf916c3 100644 --- a/tests/content-addressed.nix +++ b/tests/content-addressed.nix @@ -29,4 +29,26 @@ rec { outputHashMode = "recursive"; outputHashAlgo = "sha256"; }; + dependentCA = mkDerivation { + name = "dependent"; + buildCommand = '' + echo "building a dependent derivation" + mkdir -p $out + echo ${rootCA}/hello > $out/dep + ''; + __contentAddressed = true; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; + }; + transitivelyDependentCA = mkDerivation { + name = "transitively-dependent"; + buildCommand = '' + echo "building transitively-dependent" + cat ${dependentCA}/dep + echo ${dependentCA} > $out + ''; + __contentAddressed = true; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; + }; } diff --git a/tests/content-addressed.sh b/tests/content-addressed.sh index 0ae2852d2..61ec03fe3 100644 --- a/tests/content-addressed.sh +++ b/tests/content-addressed.sh @@ -2,19 +2,26 @@ source common.sh -clearStore -clearCache - -export REMOTE_STORE=file://$cacheDir - drv=$(nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 1) nix --experimental-features 'nix-command ca-derivations' show-derivation --derivation "$drv" --arg seed 1 -commonArgs=("--experimental-features" "ca-derivations" "./content-addressed.nix" "-A" "rootCA" "--no-out-link") -out1=$(nix-build "${commonArgs[@]}" ./content-addressed.nix --arg seed 1) -out2=$(nix-build "${commonArgs[@]}" ./content-addressed.nix --arg seed 2) +testDerivation () { + local derivationPath=$1 + local commonArgs=("--experimental-features" "ca-derivations" "./content-addressed.nix" "-A" "$derivationPath" "--no-out-link") + local out1 out2 + out1=$(nix-build "${commonArgs[@]}" --arg seed 1) + out2=$(nix-build "${commonArgs[@]}" --arg seed 2 "${secondSeedArgs[@]}") + test "$out1" == "$out2" +} -test $out1 == $out2 +testDerivation rootCA +# The seed only changes the root derivation, and not it's output, so the +# dependent derivations should only need to be built once. +secondSeedArgs=(-j0) +# Don't directly build depenentCA, that way we'll make sure we dodn't rely on +# dependent derivations always being already built. +#testDerivation dependentCA +testDerivation transitivelyDependentCA nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 5 nix-collect-garbage --experimental-features ca-derivations --option keep-derivations true diff --git a/tests/tarball.sh b/tests/tarball.sh index 88a1a07a0..fe65a22e4 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -17,7 +17,7 @@ test_tarball() { local compressor="$2" tarball=$TEST_ROOT/tarball.tar$ext - (cd $TEST_ROOT && tar c tarball) | $compressor > $tarball + (cd $TEST_ROOT && tar cf - tarball) | $compressor > $tarball nix-env -f file://$tarball -qa --out-path | grep -q dependencies |