puck/lix
0
0
Fork 0
forked from lix-project/lix
Code Pull requests Activity

Merge pull request #3875 from obsidiansystems/new-interface-for-path-pathOpt

Browse source 
Offer a safer interface for path and pathOpt
This commit is contained in:
Eelco Dolstra 2020-08-14 17:19:19 +02:00 committed by GitHub
parent 7714d9a943 1d2e80ddd6
commit 13e49be660
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 112 additions and 72 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
perl/lib/Nix/Store.xs
View file

@ -303,10 +303,10 @@ SV * derivationFromPath(char * drvPath)
hash = newHV();
HV * outputs = newHV();
for (auto & i : drv.outputs)
for (auto & i : drv.outputsAndPaths(*store()))
hv_store(
outputs, i.first.c_str(), i.first.size(),
newSVpv(store()->printStorePath(i.second.path(*store(), drv.name)).c_str(), 0),
newSVpv(store()->printStorePath(i.second.second).c_str(), 0),
0);
hv_stores(hash, "outputs", newRV((SV *) outputs));

8
src/libexpr/primops.cc
View file

@ -113,9 +113,9 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args
state.mkList(*outputsVal, drv.outputs.size());
unsigned int outputs_index = 0;
for (const auto & o : drv.outputs) {
for (const auto & o : drv.outputsAndPaths(*state.store)) {
v2 = state.allocAttr(w, state.symbols.create(o.first));
mkString(*v2, state.store->printStorePath(o.second.path(*state.store, drv.name)), {"!" + o.first + "!" + path});
mkString(*v2, state.store->printStorePath(o.second.second), {"!" + o.first + "!" + path});
outputsVal->listElems()[outputs_index] = state.allocValue();
mkString(*(outputsVal->listElems()[outputs_index++]), o.first);
}
@ -852,9 +852,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
state.mkAttrs(v, 1 + drv.outputs.size());
mkString(*state.allocAttr(v, state.sDrvPath), drvPathS, {"=" + drvPathS});
for (auto & i : drv.outputs) {
for (auto & i : drv.outputsAndPaths(*state.store)) {
mkString(*state.allocAttr(v, state.symbols.create(i.first)),
state.store->printStorePath(i.second.path(*state.store, drv.name)), {"!" + i.first + "!" + drvPathS});
state.store->printStorePath(i.second.second), {"!" + i.first + "!" + drvPathS});
}
v.attrs->sort();
}

84
src/libstore/build.cc
View file

@ -1181,8 +1181,8 @@ void DerivationGoal::haveDerivation()
retrySubstitution = false;
for (auto & i : drv->outputs)
worker.store.addTempRoot(i.second.path(worker.store, drv->name));
for (auto & i : drv->outputsAndPaths(worker.store))
worker.store.addTempRoot(i.second.second);
/* Check what outputs paths are not already valid. */
auto invalidOutputs = checkPathValidity(false, buildMode == bmRepair);
@ -1288,14 +1288,14 @@ void DerivationGoal::repairClosure()
/* Get the output closure. */
StorePathSet outputClosure;
for (auto & i : drv->outputs) {
for (auto & i : drv->outputsAndPaths(worker.store)) {
if (!wantOutput(i.first, wantedOutputs)) continue;
worker.store.computeFSClosure(i.second.path(worker.store, drv->name), outputClosure);
worker.store.computeFSClosure(i.second.second, outputClosure);
}
/* Filter out our own outputs (which we have already checked). */
for (auto & i : drv->outputs)
outputClosure.erase(i.second.path(worker.store, drv->name));
for (auto & i : drv->outputsAndPaths(worker.store))
outputClosure.erase(i.second.second);
/* Get all dependencies of this derivation so that we know which
derivation is responsible for which path in the output
@ -1306,8 +1306,8 @@ void DerivationGoal::repairClosure()
for (auto & i : inputClosure)
if (i.isDerivation()) {
Derivation drv = worker.store.derivationFromPath(i);
for (auto & j : drv.outputs)
outputsToDrv.insert_or_assign(j.second.path(worker.store, drv.name), i);
for (auto & j : drv.outputsAndPaths(worker.store))
outputsToDrv.insert_or_assign(j.second.second, i);
}
/* Check each path (slow!). */
@ -1466,10 +1466,10 @@ void DerivationGoal::tryToBuild()
/* If any of the outputs already exist but are not valid, delete
them. */
for (auto & i : drv->outputs) {
if (worker.store.isValidPath(i.second.path(worker.store, drv->name))) continue;
debug("removing invalid path '%s'", worker.store.printStorePath(i.second.path(worker.store, drv->name)));
deletePath(worker.store.Store::toRealPath(i.second.path(worker.store, drv->name)));
for (auto & i : drv->outputsAndPaths(worker.store)) {
if (worker.store.isValidPath(i.second.second)) continue;
debug("removing invalid path '%s'", worker.store.printStorePath(i.second.second));
deletePath(worker.store.Store::toRealPath(i.second.second));
}
/* Don't do a remote build if the derivation has the attribute
@ -1919,8 +1919,8 @@ StorePathSet DerivationGoal::exportReferences(const StorePathSet & storePaths)
for (auto & j : paths2) {
if (j.isDerivation()) {
Derivation drv = worker.store.derivationFromPath(j);
for (auto & k : drv.outputs)
worker.store.computeFSClosure(k.second.path(worker.store, drv.name), paths);
for (auto & k : drv.outputsAndPaths(worker.store))
worker.store.computeFSClosure(k.second.second, paths);
}
}
@ -2014,8 +2014,8 @@ void DerivationGoal::startBuilder()
chownToBuilder(tmpDir);
/* Substitute output placeholders with the actual output paths. */
for (auto & output : drv->outputs)
inputRewrites[hashPlaceholder(output.first)] = worker.store.printStorePath(output.second.path(worker.store, drv->name));
for (auto & output : drv->outputsAndPaths(worker.store))
inputRewrites[hashPlaceholder(output.first)] = worker.store.printStorePath(output.second.second);
/* Construct the environment passed to the builder. */
initEnv();
@ -2199,8 +2199,8 @@ void DerivationGoal::startBuilder()
rebuilding a path that is in settings.dirsInChroot
(typically the dependencies of /bin/sh). Throw them
out. */
for (auto & i : drv->outputs)
dirsInChroot.erase(worker.store.printStorePath(i.second.path(worker.store, drv->name)));
for (auto & i : drv->outputsAndPaths(worker.store))
dirsInChroot.erase(worker.store.printStorePath(i.second.second));
#elif __APPLE__
/* We don't really have any parent prep work to do (yet?)
@ -2612,8 +2612,8 @@ void DerivationGoal::writeStructuredAttrs()
/* Add an "outputs" object containing the output paths. */
nlohmann::json outputs;
for (auto & i : drv->outputs)
outputs[i.first] = rewriteStrings(worker.store.printStorePath(i.second.path(worker.store, drv->name)), inputRewrites);
for (auto & i : drv->outputsAndPaths(worker.store))
outputs[i.first] = rewriteStrings(worker.store.printStorePath(i.second.second), inputRewrites);
json["outputs"] = outputs;
/* Handle exportReferencesGraph. */
@ -2815,9 +2815,9 @@ struct RestrictedStore : public LocalFSStore
if (!goal.isAllowed(path.path))
throw InvalidPath("cannot build unknown path '%s' in recursive Nix", printStorePath(path.path));
auto drv = derivationFromPath(path.path);
for (auto & output : drv.outputs)
for (auto & output : drv.outputsAndPaths(*this))
if (wantOutput(output.first, path.outputs))
newPaths.insert(output.second.path(*this, drv.name));
newPaths.insert(output.second.second);
} else if (!goal.isAllowed(path.path))
throw InvalidPath("cannot build unknown path '%s' in recursive Nix", printStorePath(path.path));
}
@ -3617,8 +3617,8 @@ void DerivationGoal::registerOutputs()
to do anything here. */
if (hook) {
bool allValid = true;
for (auto & i : drv->outputs)
if (!worker.store.isValidPath(i.second.path(worker.store, drv->name))) allValid = false;
for (auto & i : drv->outputsAndPaths(worker.store))
if (!worker.store.isValidPath(i.second.second)) allValid = false;
if (allValid) return;
}
@ -3639,23 +3639,23 @@ void DerivationGoal::registerOutputs()
Nix calls. */
StorePathSet referenceablePaths;
for (auto & p : inputPaths) referenceablePaths.insert(p);
for (auto & i : drv->outputs) referenceablePaths.insert(i.second.path(worker.store, drv->name));
for (auto & i : drv->outputsAndPaths(worker.store)) referenceablePaths.insert(i.second.second);
for (auto & p : addedPaths) referenceablePaths.insert(p);
/* Check whether the output paths were created, and grep each
output path to determine what other paths it references. Also make all
output paths read-only. */
for (auto & i : drv->outputs) {
auto path = worker.store.printStorePath(i.second.path(worker.store, drv->name));
if (!missingPaths.count(i.second.path(worker.store, drv->name))) continue;
for (auto & i : drv->outputsAndPaths(worker.store)) {
auto path = worker.store.printStorePath(i.second.second);
if (!missingPaths.count(i.second.second)) continue;
Path actualPath = path;
if (needsHashRewrite()) {
auto r = redirectedOutputs.find(i.second.path(worker.store, drv->name));
auto r = redirectedOutputs.find(i.second.second);
if (r != redirectedOutputs.end()) {
auto redirected = worker.store.Store::toRealPath(r->second);
if (buildMode == bmRepair
&& redirectedBadOutputs.count(i.second.path(worker.store, drv->name))
&& redirectedBadOutputs.count(i.second.second)
&& pathExists(redirected))
replaceValidPath(path, redirected);
if (buildMode == bmCheck)
@ -3722,7 +3722,7 @@ void DerivationGoal::registerOutputs()
hash). */
std::optional<ContentAddress> ca;
if (! std::holds_alternative<DerivationOutputInputAddressed>(i.second.output)) {
if (! std::holds_alternative<DerivationOutputInputAddressed>(i.second.first.output)) {
DerivationOutputCAFloating outputHash;
std::visit(overloaded {
[&](DerivationOutputInputAddressed doi) {
@ -3737,7 +3737,7 @@ void DerivationGoal::registerOutputs()
[&](DerivationOutputCAFloating dof) {
outputHash = dof;
},
}, i.second.output);
}, i.second.first.output);
if (outputHash.method == FileIngestionMethod::Flat) {
/* The output path should be a regular file without execute permission. */
@ -3754,12 +3754,12 @@ void DerivationGoal::registerOutputs()
? hashPath(outputHash.hashType, actualPath).first
: hashFile(outputHash.hashType, actualPath);
auto dest = worker.store.makeFixedOutputPath(outputHash.method, h2, i.second.path(worker.store, drv->name).name());
auto dest = worker.store.makeFixedOutputPath(outputHash.method, h2, i.second.second.name());
// true if either floating CA, or incorrect fixed hash.
bool needsMove = true;
if (auto p = std::get_if<DerivationOutputCAFixed>(& i.second.output)) {
if (auto p = std::get_if<DerivationOutputCAFixed>(& i.second.first.output)) {
Hash & h = p->hash.hash;
if (h != h2) {
@ -3924,8 +3924,8 @@ void DerivationGoal::registerOutputs()
/* If this is the first round of several, then move the output out of the way. */
if (nrRounds > 1 && curRound == 1 && curRound < nrRounds && keepPreviousRound) {
for (auto & i : drv->outputs) {
auto path = worker.store.printStorePath(i.second.path(worker.store, drv->name));
for (auto & i : drv->outputsAndPaths(worker.store)) {
auto path = worker.store.printStorePath(i.second.second);
Path prev = path + checkSuffix;
deletePath(prev);
Path dst = path + checkSuffix;
@ -3942,8 +3942,8 @@ void DerivationGoal::registerOutputs()
/* Remove the .check directories if we're done. FIXME: keep them
if the result was not determistic? */
if (curRound == nrRounds) {
for (auto & i : drv->outputs) {
Path prev = worker.store.printStorePath(i.second.path(worker.store, drv->name)) + checkSuffix;
for (auto & i : drv->outputsAndPaths(worker.store)) {
Path prev = worker.store.printStorePath(i.second.second) + checkSuffix;
deletePath(prev);
}
}
@ -4241,12 +4241,12 @@ void DerivationGoal::flushLine()
StorePathSet DerivationGoal::checkPathValidity(bool returnValid, bool checkHash)
{
StorePathSet result;
for (auto & i : drv->outputs) {
for (auto & i : drv->outputsAndPaths(worker.store)) {
if (!wantOutput(i.first, wantedOutputs)) continue;
bool good =
worker.store.isValidPath(i.second.path(worker.store, drv->name)) &&
(!checkHash || worker.pathContentsGood(i.second.path(worker.store, drv->name)));
if (good == returnValid) result.insert(i.second.path(worker.store, drv->name));
worker.store.isValidPath(i.second.second) &&
(!checkHash || worker.pathContentsGood(i.second.second));
if (good == returnValid) result.insert(i.second.second);
}
return result;
}

37
src/libstore/derivations.cc
View file

@ -480,12 +480,12 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m
throw Error("Regular input-addressed derivations are not yet allowed to depend on CA derivations");
case DerivationType::CAFixed: {
std::map<std::string, Hash> outputHashes;
for (const auto & i : drv.outputs) {
auto & dof = std::get<DerivationOutputCAFixed>(i.second.output);
for (const auto & i : drv.outputsAndPaths(store)) {
auto & dof = std::get<DerivationOutputCAFixed>(i.second.first.output);
auto hash = hashString(htSHA256, "fixed:out:"
+ dof.hash.printMethodAlgo() + ":"
+ dof.hash.hash.to_string(Base16, false) + ":"
+ store.printStorePath(i.second.path(store, drv.name)));
+ store.printStorePath(i.second.second));
outputHashes.insert_or_assign(i.first, std::move(hash));
}
return outputHashes;
@ -539,8 +539,8 @@ bool wantOutput(const string & output, const std::set<string> & wanted)
StorePathSet BasicDerivation::outputPaths(const Store & store) const
{
StorePathSet paths;
for (auto & i : outputs)
paths.insert(i.second.path(store, name));
for (auto & i : outputsAndPaths(store))
paths.insert(i.second.second);
return paths;
}
@ -561,6 +561,27 @@ StringSet BasicDerivation::outputNames() const
return names;
}
DerivationOutputsAndPaths BasicDerivation::outputsAndPaths(const Store & store) const {
DerivationOutputsAndPaths outsAndPaths;
for (auto output : outputs)
outsAndPaths.insert(std::make_pair(
output.first,
std::make_pair(output.second, output.second.path(store, name))
)
);
return outsAndPaths;
}
DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const {
DerivationOutputsAndOptPaths outsAndOptPaths;
for (auto output : outputs)
outsAndOptPaths.insert(std::make_pair(
output.first,
std::make_pair(output.second, output.second.pathOpt(store, output.first))
)
);
return outsAndOptPaths;
}
std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) {
auto nameWithSuffix = drvPath.name();
@ -601,9 +622,9 @@ Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv,
void writeDerivation(Sink & out, const Store & store, const BasicDerivation & drv)
{
out << drv.outputs.size();
for (auto & i : drv.outputs) {
for (auto & i : drv.outputsAndPaths(store)) {
out << i.first
<< store.printStorePath(i.second.path(store, drv.name));
<< store.printStorePath(i.second.second);
std::visit(overloaded {
[&](DerivationOutputInputAddressed doi) {
out << "" << "";
@ -616,7 +637,7 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr
out << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType))
<< "";
},
}, i.second.output);
}, i.second.first.output);
}
writeStorePaths(store, out, drv.inputSrcs);
out << drv.platform << drv.builder << drv.args;

19
src/libstore/derivations.hh
View file

@ -47,6 +47,9 @@ struct DerivationOutput
DerivationOutputCAFloating
> output;
std::optional<HashType> hashAlgoOpt(const Store & store) const;
/* Note, when you use this function you should make sure that you're passing
the right derivation name. When in doubt, you should use the safer
interface provided by BasicDerivation::outputsAndPaths */
std::optional<StorePath> pathOpt(const Store & store, std::string_view drvName) const;
/* DEPRECATED: Remove after CA drvs are fully implemented */
StorePath path(const Store & store, std::string_view drvName) const {
@ -58,6 +61,15 @@ struct DerivationOutput
typedef std::map<string, DerivationOutput> DerivationOutputs;
/* These are analogues to the previous DerivationOutputs data type, but they
also contains, for each output, the (optional) store path in which it would
be written. To calculate values of these types, see the corresponding
functions in BasicDerivation */
typedef std::map<string, std::pair<DerivationOutput, StorePath>>
DerivationOutputsAndPaths;
typedef std::map<string, std::pair<DerivationOutput, std::optional<StorePath>>>
DerivationOutputsAndOptPaths;
/* For inputs that are sub-derivations, we specify exactly which
output IDs we are interested in. */
typedef std::map<StorePath, StringSet> DerivationInputs;
@ -107,6 +119,13 @@ struct BasicDerivation
/* Return the output names of a derivation. */
StringSet outputNames() const;
/* Calculates the maps that contains all the DerivationOutputs, but
augmented with knowledge of the Store paths they would be written into.
The first one of these functions will be removed when the CA work is
completed */
DerivationOutputsAndPaths outputsAndPaths(const Store & store) const;
DerivationOutputsAndOptPaths outputsAndOptPaths(const Store & store) const;
static std::string_view nameFromPath(const StorePath & storePath);
};

4
src/libstore/local-store.cc
View file

@ -618,11 +618,11 @@ uint64_t LocalStore::addValidPath(State & state,
registration above is undone. */
if (checkOutputs) checkDerivationOutputs(info.path, drv);
for (auto & i : drv.outputs) {
for (auto & i : drv.outputsAndPaths(*this)) {
state.stmtAddDerivationOutput.use()
(id)
(i.first)
(printStorePath(i.second.path(*this, drv.name)))
(printStorePath(i.second.second))
.exec();
}
}

6
src/libstore/misc.cc
View file

@ -207,10 +207,10 @@ void Store::queryMissing(const std::vector<StorePathWithOutputs> & targets,
ParsedDerivation parsedDrv(StorePath(path.path), *drv);
PathSet invalid;
for (auto & j : drv->outputs)
for (auto & j : drv->outputsAndPaths(*this))
if (wantOutput(j.first, path.outputs)
&& !isValidPath(j.second.path(*this, drv->name)))
invalid.insert(printStorePath(j.second.path(*this, drv->name)));
&& !isValidPath(j.second.second))
invalid.insert(printStorePath(j.second.second));
if (invalid.empty()) return;
if (settings.useSubstitutes && parsedDrv.substitutesAllowed()) {

8
src/nix-store/nix-store.cc
View file

@ -218,8 +218,8 @@ static StorePathSet maybeUseOutputs(const StorePath & storePath, bool useOutput,
if (useOutput && storePath.isDerivation()) {
auto drv = store->derivationFromPath(storePath);
StorePathSet outputs;
for (auto & i : drv.outputs)
outputs.insert(i.second.path(*store, drv.name));
for (auto & i : drv.outputsAndPaths(*store))
outputs.insert(i.second.second);
return outputs;
}
else return {storePath};
@ -312,8 +312,8 @@ static void opQuery(Strings opFlags, Strings opArgs)
auto i2 = store->followLinksToStorePath(i);
if (forceRealise) realisePath({i2});
Derivation drv = store->derivationFromPath(i2);
for (auto & j : drv.outputs)
cout << fmt("%1%\n", store->printStorePath(j.second.path(*store, drv.name)));
for (auto & j : drv.outputsAndPaths(*store))
cout << fmt("%1%\n", store->printStorePath(j.second.second));
}
break;
}

4
src/nix/installables.cc
View file

@ -304,8 +304,8 @@ struct InstallableStorePath : Installable
if (storePath.isDerivation()) {
std::map<std::string, StorePath> outputs;
auto drv = store->readDerivation(storePath);
for (auto & [name, output] : drv.outputs)
outputs.emplace(name, output.path(*store, drv.name));
for (auto & i : drv.outputsAndPaths(*store))
outputs.emplace(i.first, i.second.second);
return {
BuildableFromDrv {
.drvPath = storePath,

4
src/nix/repl.cc
View file

@ -490,8 +490,8 @@ bool NixRepl::processLine(string line)
if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPath}) == 0) {
auto drv = readDerivation(*state->store, drvPath, Derivation::nameFromPath(state->store->parseStorePath(drvPath)));
std::cout << std::endl << "this derivation produced the following outputs:" << std::endl;
for (auto & i : drv.outputs)
std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.path(*state->store, drv.name)));
for (auto & i : drv.outputsAndPaths(*state->store))
std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.second));
}
} else if (command == ":i") {
runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPath});

6
src/nix/show-derivation.cc
View file

@ -67,9 +67,9 @@ struct CmdShowDerivation : InstallablesCommand
{
auto outputsObj(drvObj.object("outputs"));
for (auto & output : drv.outputs) {
for (auto & output : drv.outputsAndPaths(*store)) {
auto outputObj(outputsObj.object(output.first));
outputObj.attr("path", store->printStorePath(output.second.path(*store, drv.name)));
outputObj.attr("path", store->printStorePath(output.second.second));
std::visit(overloaded {
[&](DerivationOutputInputAddressed doi) {
@ -81,7 +81,7 @@ struct CmdShowDerivation : InstallablesCommand
[&](DerivationOutputCAFloating dof) {
outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType));
},
}, output.second.output);
}, output.second.first.output);
}
}