diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 4e248f979..78dc314fa 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -75,6 +75,18 @@ void EvalState::realiseContext(const PathSet & context) } } +/* Add and attribute to the given attribute map from the output name to + the output path, or a placeholder. + + Where possible the path is used, but for floating CA derivations we + may not know it. For sake of determinism we always assume we don't + and instead put in a place holder. In either case, however, the + string context will contain the drv path and output name, so + downstream derivations will have the proper dependency, and in + addition, before building, the placeholder will be rewritten to be + the actual path. + + The 'drv' and 'drvPath' outputs must correspond. */ static void mkOutputString(EvalState & state, Value & v, const StorePath & drvPath, const BasicDerivation & drv, std::pair o) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1f77b8ea8..e50b23ed6 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -718,7 +718,7 @@ typedef enum {rpAccept, rpDecline, rpPostpone} HookReply; class SubstitutionGoal; -struct KnownInitialOutputStatus { +struct InitialOutputStatus { StorePath path; /* The output optional indicates whether it's already valid; i.e. exists and is registered. If we're repairing, inner bool indicates whether the @@ -731,9 +731,9 @@ struct KnownInitialOutputStatus { } }; -struct InitialOutputStatus { +struct InitialOutput { bool wanted; - std::optional known; + std::optional known; }; class DerivationGoal : public Goal @@ -770,7 +770,7 @@ private: immediate input paths). */ StorePathSet inputPaths; - std::map initialOutputs; + std::map initialOutputs; /* User selected for running the builder. */ std::unique_ptr buildUser; @@ -1052,11 +1052,14 @@ private: /* Forcibly kill the child process, if any. */ void killChild(); - /* Map a path to another (reproducably) so we can avoid overwriting outputs + /* Create alternative path calculated from but distinct from the + input, so we can avoid overwriting outputs (or other store paths) that already exist. */ StorePath makeFallbackPath(const StorePath & path); - /* Make a path to another based on the output name alone, if one doesn't - want to use a random path for CA builds. */ + /* Make a path to another based on the output name along with the + derivation hash. */ + /* FIXME add option to randomize, so we can audit whether our + rewrites caught everything */ StorePath makeFallbackPath(std::string_view outputName); void repairClosure(); @@ -1268,7 +1271,7 @@ void DerivationGoal::haveDerivation() for (auto & [_, status] : initialOutputs) { if (!status.wanted) continue; if (!status.known) { - warn("Do not know how to query for unknown floating CA drv output yet"); + warn("do not know how to query for unknown floating content-addressed derivation output yet"); /* Nothing to wait for; tail call */ return DerivationGoal::gaveUpOnSubstitution(); } @@ -1497,7 +1500,7 @@ void DerivationGoal::inputsRealised() auto optRealizedInput = outputs.at(j); if (!optRealizedInput) throw Error( - "derivation '%s' requires output '%s' from input derivation '%s', which is supposedly realized already, yet we still don't know what path corresponds to that output.", + "derivation '%s' requires output '%s' from input derivation '%s', which is supposedly realized already, yet we still don't know what path corresponds to that output", worker.store.printStorePath(drvPath), j, worker.store.printStorePath(drvPath)); worker.store.computeFSClosure(*optRealizedInput, inputPaths); } else @@ -2070,7 +2073,7 @@ StorePathSet DerivationGoal::exportReferences(const StorePathSet & storePaths) `computeFSClosure` on the output path, rather than derivation itself. That doesn't seem right to me, so I won't try to implemented this for CA derivations. */ - throw UnimplementedError("export references including CA derivations (themselves) is not yet implemented"); + throw UnimplementedError("exportReferences on CA derivations is not yet implemented"); worker.store.computeFSClosure(*k.second.second, paths); } } @@ -2179,8 +2182,6 @@ void DerivationGoal::startBuilder() with the actual hashes. */ auto scratchPath = !status.known - /* FIXME add option to randomize, so we can audit whether our - * rewrites caught everything */ ? makeFallbackPath(outputName) : !needsHashRewrite() /* Can always use original path in sandbox */ @@ -2213,7 +2214,7 @@ void DerivationGoal::startBuilder() differ. */ if (fixedFinalPath == scratchPath) continue; - /* Ensure scratch scratch path is ours to use */ + /* Ensure scratch path is ours to use. */ deletePath(worker.store.printStorePath(scratchPath)); /* Rewrite and unrewrite paths */ @@ -4139,8 +4140,13 @@ void DerivationGoal::registerOutputs() if (lstat(actualPath.c_str(), &st)) throw SysError("getting attributes of path '%1%'", actualPath); mode |= 0200; - if (chmod(actualPath.c_str(), mode) == -1) - throw SysError("changing mode of '%1%' to %2$o", actualPath, mode); + /* Try to change the perms, but only if the file isn't a + symlink as symlinks permissions are mostly ignored and + calling `chmod` on it will just forward the call to the + target of the link. */ + if (!S_ISLNK(st.st_mode)) + if (chmod(actualPath.c_str(), mode) == -1) + throw SysError("changing mode of '%1%' to %2$o", actualPath, mode); } if (rename( actualPath.c_str(), @@ -4619,19 +4625,19 @@ void DerivationGoal::checkPathValidity() { bool checkHash = buildMode == bmRepair; for (auto & i : queryPartialDerivationOutputMap()) { - InitialOutputStatus status { + InitialOutput info { .wanted = wantOutput(i.first, wantedOutputs), }; if (i.second) { auto outputPath = *i.second; - status.known = { + info.known = { .path = outputPath, .valid = !worker.store.isValidPath(outputPath) ? std::optional {} : !checkHash || worker.pathContentsGood(outputPath), }; } - initialOutputs.insert_or_assign(i.first, status); + initialOutputs.insert_or_assign(i.first, info); } } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index eaffbf452..8aa496143 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -154,6 +154,11 @@ Derivation parseDerivation(const Store & store, std::string && s, std::string_vi // FIXME: remove bool isDerivation(const string & fileName); +/* Calculate the name that will be used for the store path for this + output. + + This is usually -, but is just when + the output name is "out". */ std::string outputPathName(std::string_view drvName, std::string_view outputName); // known CA drv's output hashes, current just for fixed-output derivations @@ -213,8 +218,21 @@ struct Sink; Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, std::string_view name); void writeDerivation(Sink & out, const Store & store, const BasicDerivation & drv); +/* This creates an opaque and almost certainly unique string + deterministically from the output name. + + It is used as a placeholder to allow derivations to refer to their + own outputs without needing to use the hash of a derivation in + itself, making the hash near-impossible to calculate. */ std::string hashPlaceholder(const std::string & outputName); +/* This creates an opaque and almost certainly unique string + deterministically from a derivation path and output name. + + It is used as a placeholder to allow derivations to refer to + content-addressed paths whose content --- and thus the path + themselves --- isn't yet known. This occurs when a derivation has a + dependency which is a CA derivation. */ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath, std::string_view outputName); } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index ea430fa14..d5e6d68ef 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -279,9 +279,8 @@ private: specified by the ‘secret-key-files’ option. */ void signPathInfo(ValidPathInfo & info); - /* Add a mapping from the deriver of the path info (if specified) to its - * out path - */ + /* Register the store path 'output' as the output named 'outputName' of + derivation 'deriver'. */ void linkDeriverToPath(const StorePath & deriver, const string & outputName, const StorePath & output); void linkDeriverToPath(State & state, uint64_t deriver, const string & outputName, const StorePath & output); diff --git a/src/nix/command.cc b/src/nix/command.cc index f697eb84c..37a4bc785 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -150,8 +150,9 @@ void MixProfile::updateProfile(const Buildables & buildables) }, [&](BuildableFromDrv bfd) { for (auto & output : bfd.outputs) { - if (!output.second) - throw Error("output path should be known because we just tried to build it"); + /* Output path should be known because we just tried to + build it. */ + assert(!output.second); result.push_back(*output.second); } }, diff --git a/tests/content-addressed.nix b/tests/content-addressed.nix index 5ebc779c1..3dcf916c3 100644 --- a/tests/content-addressed.nix +++ b/tests/content-addressed.nix @@ -13,17 +13,17 @@ rec { mkdir -p $out echo "Hello World" > $out/hello ''; - __contentAddressed = true; - outputHashMode = "recursive"; - outputHashAlgo = "sha256"; }; rootCA = mkDerivation { name = "dependent"; + outputs = [ "out" "dev" ]; buildCommand = '' echo "building a CA derivation" echo "The seed is ${toString seed}" mkdir -p $out echo ${rootLegacy}/hello > $out/dep + # test symlink at root + ln -s $out $dev ''; __contentAddressed = true; outputHashMode = "recursive";