Skip to content
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
5 changes: 3 additions & 2 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1068,14 +1068,15 @@ void EvalState::mkOutputString(
Value & value,
const StorePath & drvPath,
const std::string outputName,
std::optional<StorePath> optOutputPath)
std::optional<StorePath> optOutputPath,
const ExperimentalFeatureSettings & xpSettings)
{
value.mkString(
optOutputPath
? store->printStorePath(*std::move(optOutputPath))
/* Downstream we would substitute this for an actual path once
we build the floating CA derivation */
: DownstreamPlaceholder::unknownCaOutput(drvPath, outputName).render(),
: DownstreamPlaceholder::unknownCaOutput(drvPath, outputName, xpSettings).render(),
NixStringContext {
NixStringContextElem::Built {
.drvPath = drvPath,
Expand Down
5 changes: 4 additions & 1 deletion src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -689,12 +689,15 @@ public:
* be passed if and only if output store object is input-addressed.
* Will be printed to form string if passed, otherwise a placeholder
* will be used (see `DownstreamPlaceholder`).
*
* @param xpSettings Stop-gap to avoid globals during unit tests.
*/
void mkOutputString(
Value & value,
const StorePath & drvPath,
const std::string outputName,
std::optional<StorePath> optOutputPath);
std::optional<StorePath> optOutputPath,
const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);

void concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx);

Expand Down
25 changes: 12 additions & 13 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,21 @@ StringMap EvalState::realiseContext(const NixStringContext & context)
for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { d });
store->buildPaths(buildReqs);

/* Get all the output paths corresponding to the placeholders we had */
for (auto & drv : drvs) {
auto outputs = resolveDerivedPath(*store, drv);
for (auto & [outputName, outputPath] : outputs) {
res.insert_or_assign(
DownstreamPlaceholder::unknownCaOutput(drv.drvPath, outputName).render(),
store->printStorePath(outputPath)
);
}
}

/* Add the output of this derivations to the allowed
paths. */
if (allowedPaths) {
for (auto & [_placeholder, outputPath] : res) {
allowPath(store->toRealPath(outputPath));
/* Add the output of this derivations to the allowed
paths. */
if (allowedPaths) {
allowPath(outputPath);
}
/* Get all the output paths corresponding to the placeholders we had */
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
res.insert_or_assign(
DownstreamPlaceholder::unknownCaOutput(drv.drvPath, outputName).render(),
store->printStorePath(outputPath)
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't there be an "else" here (like throwing an error)? Seems strange that we can do nothing if CA derivations are not enabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so actually. It is unclear but basically:

  1. The only way to get these placeholders is the outPaths of CA derivation outputs.
  2. If we don't have those, then there is nothing to substitute.
  3. If someone tried to hand-write one of these (rather than using outPath, it shouldn't work.

Today, it does work; this is mistakenly a bonus feature that can be used without ca-derivations with careful hand-coding. But I don't think we want that.

}
}

Expand Down
9 changes: 8 additions & 1 deletion src/libexpr/tests/derived-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,15 @@ RC_GTEST_FIXTURE_PROP(
prop_built_path_placeholder_round_trip,
(const StorePath & drvPath, const StorePathName & outputName))
{
/**
* We set these in tests rather than the regular globals so we don't have
* to worry about race conditions if the tests run concurrently.
*/
ExperimentalFeatureSettings mockXpSettings;
mockXpSettings.set("experimental-features", "ca-derivations");

auto * v = state.allocValue();
state.mkOutputString(*v, drvPath, outputName.name, std::nullopt);
state.mkOutputString(*v, drvPath, outputName.name, std::nullopt, mockXpSettings);
auto [d, _] = state.coerceToDerivedPathUnchecked(noPos, *v, "");
DerivedPath::Built b {
.drvPath = drvPath,
Expand Down
8 changes: 5 additions & 3 deletions src/libstore/derivations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -878,9 +878,11 @@ std::optional<BasicDerivation> Derivation::tryResolve(
for (auto & [inputDrv, inputOutputs] : inputDrvs) {
for (auto & outputName : inputOutputs) {
if (auto actualPath = get(inputDrvOutputs, { inputDrv, outputName })) {
inputRewrites.emplace(
DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName).render(),
store.printStorePath(*actualPath));
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
inputRewrites.emplace(
DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName).render(),
store.printStorePath(*actualPath));
}
resolved.inputSrcs.insert(*actualPath);
} else {
warn("output '%s' of input '%s' missing, aborting the resolving",
Expand Down
4 changes: 3 additions & 1 deletion src/libstore/downstream-placeholder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ std::string DownstreamPlaceholder::render() const

DownstreamPlaceholder DownstreamPlaceholder::unknownCaOutput(
const StorePath & drvPath,
std::string_view outputName)
std::string_view outputName,
const ExperimentalFeatureSettings & xpSettings)
{
xpSettings.require(Xp::CaDerivations);
auto drvNameWithExtension = drvPath.name();
auto drvName = drvNameWithExtension.substr(0, drvNameWithExtension.size() - 4);
auto clearText = "nix-upstream-output:" + std::string { drvPath.hashPart() } + ":" + outputPathName(drvName, outputName);
Expand Down
5 changes: 4 additions & 1 deletion src/libstore/downstream-placeholder.hh
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ public:
*
* The derivation itself is known (we have a store path for it), but
* the output doesn't yet have a known store path.
*
* @param xpSettings Stop-gap to avoid globals during unit tests.
*/
static DownstreamPlaceholder unknownCaOutput(
const StorePath & drvPath,
std::string_view outputName);
std::string_view outputName,
const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);

/**
* Create a placehold for the output of an unknown derivation.
Expand Down
16 changes: 12 additions & 4 deletions src/libstore/tests/downstream-placeholder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,24 @@
namespace nix {

TEST(DownstreamPlaceholder, unknownCaOutput) {
/**
* We set these in tests rather than the regular globals so we don't have
* to worry about race conditions if the tests run concurrently.
*/
ExperimentalFeatureSettings mockXpSettings;
mockXpSettings.set("experimental-features", "ca-derivations");

ASSERT_EQ(
DownstreamPlaceholder::unknownCaOutput(
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" },
"out").render(),
"out",
mockXpSettings).render(),
"/0c6rn30q4frawknapgwq386zq358m8r6msvywcvc89n6m5p2dgbz");
}

TEST(DownstreamPlaceholder, unknownDerivation) {
/**
* We set these in tests rather than the regular globals so we don't have
* to worry about race conditions if the tests run concurrently.
* Same reason as above
*/
ExperimentalFeatureSettings mockXpSettings;
mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations");
Expand All @@ -24,7 +31,8 @@ TEST(DownstreamPlaceholder, unknownDerivation) {
DownstreamPlaceholder::unknownDerivation(
DownstreamPlaceholder::unknownCaOutput(
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv.drv" },
"out"),
"out",
mockXpSettings),
"out",
mockXpSettings).render(),
"/0gn6agqxjyyalf0dpihgyf49xq5hqxgw100f0wydnj6yqrhqsb3w");
Expand Down
11 changes: 6 additions & 5 deletions src/nix/app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ StringPairs resolveRewrites(
StringPairs res;
for (auto & dep : dependencies)
if (auto drvDep = std::get_if<BuiltPathBuilt>(&dep.path))
for (auto & [ outputName, outputPath ] : drvDep->outputs)
res.emplace(
DownstreamPlaceholder::unknownCaOutput(drvDep->drvPath, outputName).render(),
store.printStorePath(outputPath)
);
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations))
for (auto & [ outputName, outputPath ] : drvDep->outputs)
res.emplace(
DownstreamPlaceholder::unknownCaOutput(drvDep->drvPath, outputName).render(),
store.printStorePath(outputPath)
);
return res;
}

Expand Down