Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give queryPartialDerivationOutputMap an evalStore parameter #8724

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libcmd/built-path.hh
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "derived-path.hh"
#include "realisation.hh"

namespace nix {

Expand Down
6 changes: 4 additions & 2 deletions src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1251,11 +1251,13 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
void queryReferrers(const StorePath & path, StorePathSet & referrers) override
{ }

std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path) override
std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(
const StorePath & path,
Store * evalStore = nullptr) override
{
if (!goal.isAllowed(path))
throw InvalidPath("cannot query output map for unknown path '%s' in recursive Nix", printStorePath(path));
return next->queryPartialDerivationOutputMap(path);
return next->queryPartialDerivationOutputMap(path, evalStore);
}

std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override
Expand Down
1 change: 0 additions & 1 deletion src/libstore/derived-path.hh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#include "util.hh"
#include "path.hh"
#include "realisation.hh"
#include "outputs-spec.hh"
#include "comparator.hh"

Expand Down
20 changes: 2 additions & 18 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1022,10 +1022,9 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path)


std::map<std::string, std::optional<StorePath>>
LocalStore::queryPartialDerivationOutputMap(const StorePath & path_)
LocalStore::queryStaticPartialDerivationOutputMap(const StorePath & path)
{
auto path = path_;
auto outputs = retrySQLite<std::map<std::string, std::optional<StorePath>>>([&]() {
return retrySQLite<std::map<std::string, std::optional<StorePath>>>([&]() {
auto state(_state.lock());
std::map<std::string, std::optional<StorePath>> outputs;
uint64_t drvId;
Expand All @@ -1037,21 +1036,6 @@ LocalStore::queryPartialDerivationOutputMap(const StorePath & path_)

return outputs;
});

if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations))
return outputs;

auto drv = readInvalidDerivation(path);
auto drvHashes = staticOutputHashes(*this, drv);
for (auto& [outputName, hash] : drvHashes) {
auto realisation = queryRealisation(DrvOutput{hash, outputName});
if (realisation)
outputs.insert_or_assign(outputName, realisation->outPath);
else
outputs.insert({outputName, std::nullopt});
}

return outputs;
Comment on lines -1040 to -1054
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is local-store-agnostic, and thus moved out of here.

}

std::optional<StorePath> LocalStore::queryPathFromHashPart(const std::string & hashPart)
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/local-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public:

StorePathSet queryValidDerivers(const StorePath & path) override;

std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path) override;
std::map<std::string, std::optional<StorePath>> queryStaticPartialDerivationOutputMap(const StorePath & path) override;

std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override;

Expand Down
53 changes: 22 additions & 31 deletions src/libstore/misc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -310,43 +310,34 @@ std::map<DrvOutput, StorePath> drvOutputReferences(

OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, Store * evalStore_)
{
auto & evalStore = evalStore_ ? *evalStore_ : store;
auto outputsOpt_ = store.queryPartialDerivationOutputMap(bfd.drvPath, evalStore_);

OutputPathMap outputs;
auto drv = evalStore.readDerivation(bfd.drvPath);
auto outputHashes = staticOutputHashes(store, drv);
auto drvOutputs = drv.outputsAndOptPaths(store);
auto outputNames = std::visit(overloaded {
auto outputsOpt = std::visit(overloaded {
[&](const OutputsSpec::All &) {
StringSet names;
for (auto & [outputName, _] : drv.outputs)
names.insert(outputName);
return names;
// Keep all outputs
return std::move(outputsOpt_);
},
[&](const OutputsSpec::Names & names) {
return static_cast<std::set<std::string>>(names);
// Get just those mentioned by name
std::map<std::string, std::optional<StorePath>> outputsOpt;
for (auto & output : names) {
auto * pOutputPathOpt = get(outputsOpt_, output);
if (!pOutputPathOpt)
throw Error(
"the derivation '%s' doesn't have an output named '%s'",
store.printStorePath(bfd.drvPath), output);
outputsOpt.insert_or_assign(output, std::move(*pOutputPathOpt));
}
return outputsOpt;
},
}, bfd.outputs.raw());
for (auto & output : outputNames) {
auto outputHash = get(outputHashes, output);
if (!outputHash)
throw Error(
"the derivation '%s' doesn't have an output named '%s'",
store.printStorePath(bfd.drvPath), output);
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
DrvOutput outputId { *outputHash, output };
auto realisation = store.queryRealisation(outputId);
if (!realisation)
throw MissingRealisation(outputId);
outputs.insert_or_assign(output, realisation->outPath);
} else {
// If ca-derivations isn't enabled, assume that
// the output path is statically known.
auto drvOutput = get(drvOutputs, output);
assert(drvOutput);
assert(drvOutput->second);
outputs.insert_or_assign(output, *drvOutput->second);
}

OutputPathMap outputs;
for (auto & [outputName, outputPathOpt] : outputsOpt) {
if (!outputPathOpt)
throw MissingRealisation(store.printStorePath(bfd.drvPath), outputName);
auto & outputPath = *outputPathOpt;
outputs.insert_or_assign(outputName, outputPath);
}
return outputs;
}
Expand Down
9 changes: 7 additions & 2 deletions src/libstore/realisation.hh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "hash.hh"
#include "path.hh"
#include "derived-path.hh"
#include <nlohmann/json_fwd.hpp>
#include "comparator.hh"
#include "crypto.hh"
Expand Down Expand Up @@ -143,9 +144,13 @@ class MissingRealisation : public Error
{
public:
MissingRealisation(DrvOutput & outputId)
: Error( "cannot operate on an output of the "
: MissingRealisation(outputId.outputName, outputId.strHash())
{}
MissingRealisation(std::string_view drv, std::string outputName)
: Error( "cannot operate on output '%s' of the "
"unbuilt derivation '%s'",
outputId.to_string())
outputName,
drv)
{}
};

Expand Down
33 changes: 21 additions & 12 deletions src/libstore/remote-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -378,27 +378,36 @@ StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path)
}


std::map<std::string, std::optional<StorePath>> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path)
std::map<std::string, std::optional<StorePath>> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_)
{
if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) {
auto conn(getConnection());
conn->to << WorkerProto::Op::QueryDerivationOutputMap << printStorePath(path);
conn.processStderr();
return WorkerProto::Serialise<std::map<std::string, std::optional<StorePath>>>::read(*this, *conn);
if (!evalStore_) {
auto conn(getConnection());
conn->to << WorkerProto::Op::QueryDerivationOutputMap << printStorePath(path);
conn.processStderr();
return WorkerProto::Serialise<std::map<std::string, std::optional<StorePath>>>::read(*this, *conn);
} else {
auto & evalStore = *evalStore_;
auto outputs = evalStore.queryStaticPartialDerivationOutputMap(path);
// union with the first branch overriding the statically-known ones
// when non-`std::nullopt`.
for (auto && [outputName, optPath] : queryPartialDerivationOutputMap(path, nullptr)) {
if (optPath)
outputs.insert_or_assign(std::move(outputName), std::move(optPath));
else
outputs.insert({std::move(outputName), std::nullopt});
}
return outputs;
}
} else {
auto & evalStore = evalStore_ ? *evalStore_ : *this;
// Fallback for old daemon versions.
// For floating-CA derivations (and their co-dependencies) this is an
// under-approximation as it only returns the paths that can be inferred
// from the derivation itself (and not the ones that are known because
// the have been built), but as old stores don't handle floating-CA
// derivations this shouldn't matter
auto derivation = readDerivation(path);
auto outputsWithOptPaths = derivation.outputsAndOptPaths(*this);
std::map<std::string, std::optional<StorePath>> ret;
for (auto & [outputName, outputAndPath] : outputsWithOptPaths) {
ret.emplace(outputName, outputAndPath.second);
}
return ret;
return evalStore.queryStaticPartialDerivationOutputMap(path);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/libstore/remote-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public:

StorePathSet queryDerivationOutputs(const StorePath & path) override;

std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path) override;
std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore = nullptr) override;
std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override;

StorePathSet querySubstitutablePaths(const StorePathSet & paths) override;
Expand Down
34 changes: 31 additions & 3 deletions src/libstore/store-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -496,22 +496,50 @@ bool Store::PathInfoCacheValue::isKnownNow()
return std::chrono::steady_clock::now() < time_point + ttl;
}

std::map<std::string, std::optional<StorePath>> Store::queryPartialDerivationOutputMap(const StorePath & path)
std::map<std::string, std::optional<StorePath>> Store::queryStaticPartialDerivationOutputMap(const StorePath & path)
{
std::map<std::string, std::optional<StorePath>> outputs;
auto drv = readInvalidDerivation(path);
for (auto& [outputName, output] : drv.outputsAndOptPaths(*this)) {
for (auto & [outputName, output] : drv.outputsAndOptPaths(*this)) {
outputs.emplace(outputName, output.second);
}
return outputs;
}

std::map<std::string, std::optional<StorePath>> Store::queryPartialDerivationOutputMap(
const StorePath & path,
Store * evalStore_)
{
auto & evalStore = evalStore_ ? *evalStore_ : *this;

auto outputs = evalStore.queryStaticPartialDerivationOutputMap(path);

if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations))
return outputs;

auto drv = evalStore.readInvalidDerivation(path);
auto drvHashes = staticOutputHashes(*this, drv);
for (auto & [outputName, hash] : drvHashes) {
auto realisation = queryRealisation(DrvOutput{hash, outputName});
if (realisation) {
outputs.insert_or_assign(outputName, realisation->outPath);
} else {
// queryStaticPartialDerivationOutputMap is not guaranteed
// to return std::nullopt for outputs which are not
// statically known.
outputs.insert({outputName, std::nullopt});
}
}

return outputs;
}

OutputPathMap Store::queryDerivationOutputMap(const StorePath & path) {
auto resp = queryPartialDerivationOutputMap(path);
OutputPathMap result;
for (auto & [outName, optOutPath] : resp) {
if (!optOutPath)
throw Error("output '%s' of derivation '%s' has no store path mapped to it", outName, printStorePath(path));
throw MissingRealisation(printStorePath(path), outName);
result.insert_or_assign(outName, *optOutPath);
}
return result;
Expand Down
15 changes: 14 additions & 1 deletion src/libstore/store-api.hh
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,20 @@ public:
* derivation. All outputs are mentioned so ones mising the mapping
* are mapped to `std::nullopt`.
*/
virtual std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(const StorePath & path);
virtual std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(
const StorePath & path,
Store * evalStore = nullptr);

/**
* Like `queryPartialDerivationOutputMap` but only considers
* statically known output paths (i.e. those that can be gotten from
* the derivation itself.
*
* Just a helper function for implementing
* `queryPartialDerivationOutputMap`.
*/
virtual std::map<std::string, std::optional<StorePath>> queryStaticPartialDerivationOutputMap(
const StorePath & path);

/**
* Query the mapping outputName=>outputPath for the given derivation.
Expand Down