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

Refactor Raw pattern, part of #7479 #8839

Merged
merged 1 commit into from
Aug 18, 2023
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/libcmd/installable-attr-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
[&](const ExtendedOutputsSpec::Explicit & e) -> OutputsSpec {
return e;
},
}, extendedOutputsSpec.raw());
}, extendedOutputsSpec.raw);

auto [iter, didInsert] = byDrvPath.emplace(*drvPath, newOutputs);

Expand All @@ -96,6 +96,7 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
.outputs = outputs,
},
.info = make_ref<ExtraPathInfoValue>(ExtraPathInfoValue::Value {
.extendedOutputsSpec = outputs,
/* FIXME: reconsider backwards compatibility above
so we can fill in this info. */
}),
Expand All @@ -114,7 +115,7 @@ InstallableAttrPath InstallableAttrPath::parse(
return {
state, cmd, v,
prefix == "." ? "" : std::string { prefix },
extendedOutputsSpec
std::move(extendedOutputsSpec),
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/libcmd/installable-derived-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ InstallableDerivedPath InstallableDerivedPath::parse(
.outputs = outputSpec,
};
},
}, extendedOutputsSpec.raw());
}, extendedOutputsSpec.raw);
return InstallableDerivedPath {
store,
std::move(derivedPath),
Expand Down
2 changes: 1 addition & 1 deletion src/libcmd/installable-flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
[&](const ExtendedOutputsSpec::Explicit & e) -> OutputsSpec {
return e;
},
}, extendedOutputsSpec.raw()),
}, extendedOutputsSpec.raw),
},
.info = make_ref<ExtraPathInfoFlake>(
ExtraPathInfoValue::Value {
Expand Down
6 changes: 3 additions & 3 deletions src/libcmd/installables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ Installables SourceExprCommand::parseInstallables(
result.push_back(
make_ref<InstallableAttrPath>(
InstallableAttrPath::parse(
state, *this, vFile, prefix, extendedOutputsSpec)));
state, *this, vFile, std::move(prefix), std::move(extendedOutputsSpec))));
}

} else {
Expand All @@ -475,7 +475,7 @@ Installables SourceExprCommand::parseInstallables(
if (prefix.find('/') != std::string::npos) {
try {
result.push_back(make_ref<InstallableDerivedPath>(
InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec)));
InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec.raw)));
continue;
} catch (BadStorePath &) {
} catch (...) {
Expand All @@ -491,7 +491,7 @@ Installables SourceExprCommand::parseInstallables(
getEvalState(),
std::move(flakeRef),
fragment,
extendedOutputsSpec,
std::move(extendedOutputsSpec),
getDefaultFlakeAttrPaths(),
getDefaultFlakeAttrPathPrefixes(),
lockFlags));
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/eval-cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ string_t AttrCursor::getStringWithContext()
[&](const NixStringContextElem::Opaque & o) -> const StorePath & {
return o.path;
},
}, c.raw());
}, c.raw);
if (!root->state.store->isValidPath(path)) {
valid = false;
break;
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2364,7 +2364,7 @@ std::pair<SingleDerivedPath, std::string_view> EvalState::coerceToSingleDerivedP
[&](NixStringContextElem::Built && b) -> SingleDerivedPath {
return std::move(b);
},
}, ((NixStringContextElem &&) *context.begin()).raw());
}, ((NixStringContextElem &&) *context.begin()).raw);
return {
std::move(derivedPath),
std::move(s),
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/flake/flakeref.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ std::tuple<FlakeRef, std::string, ExtendedOutputsSpec> parseFlakeRefWithFragment
{
auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(url);
auto [flakeRef, fragment] = parseFlakeRefWithFragment(std::string { prefix }, baseDir, allowMissing, isFlake);
return {std::move(flakeRef), fragment, extendedOutputsSpec};
return {std::move(flakeRef), fragment, std::move(extendedOutputsSpec)};
}

}
14 changes: 7 additions & 7 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ StringMap EvalState::realiseContext(const NixStringContext & context)
res.insert_or_assign(ctxS, ctxS);
ensureValid(d.drvPath);
},
}, c.raw());
}, c.raw);
}

if (drvs.empty()) return {};
Expand Down Expand Up @@ -1265,7 +1265,7 @@ drvName, Bindings * attrs, Value & v)
[&](const NixStringContextElem::Opaque & o) {
drv.inputSrcs.insert(o.path);
},
}, c.raw());
}, c.raw);
}

/* Do we have all required attributes? */
Expand Down Expand Up @@ -1334,13 +1334,13 @@ drvName, Bindings * attrs, Value & v)
if (isImpure)
drv.outputs.insert_or_assign(i,
DerivationOutput::Impure {
.method = method.raw,
.method = method,
.hashType = ht,
});
else
drv.outputs.insert_or_assign(i,
DerivationOutput::CAFloating {
.method = method.raw,
.method = method,
.hashType = ht,
});
}
Expand Down Expand Up @@ -1373,15 +1373,15 @@ drvName, Bindings * attrs, Value & v)
drv.env[i] = state.store->printStorePath(outPath);
drv.outputs.insert_or_assign(
i,
DerivationOutputInputAddressed {
DerivationOutput::InputAddressed {
.path = std::move(outPath),
});
}
break;
;
case DrvHash::Kind::Deferred:
for (auto & i : outputs) {
drv.outputs.insert_or_assign(i, DerivationOutputDeferred {});
drv.outputs.insert_or_assign(i, DerivationOutput::Deferred {});
}
}
}
Expand Down Expand Up @@ -2054,7 +2054,7 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val
StorePathSet refs;

for (auto c : context) {
if (auto p = std::get_if<NixStringContextElem::Opaque>(&c))
if (auto p = std::get_if<NixStringContextElem::Opaque>(&c.raw))
refs.insert(p->path);
else
state.debugThrowLastTrace(EvalError({
Expand Down
6 changes: 3 additions & 3 deletions src/libexpr/primops/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx p

NixStringContext context2;
for (auto && c : context) {
if (auto * ptr = std::get_if<NixStringContextElem::DrvDeep>(&c)) {
if (auto * ptr = std::get_if<NixStringContextElem::DrvDeep>(&c.raw)) {
context2.emplace(NixStringContextElem::Opaque {
.path = ptr->drvPath
});
} else {
/* Can reuse original item */
context2.emplace(std::move(c));
context2.emplace(std::move(c).raw);
}
}

Expand Down Expand Up @@ -114,7 +114,7 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args,
[&](NixStringContextElem::Opaque && o) {
contextInfos[std::move(o.path)].path = true;
},
}, ((NixStringContextElem &&) i).raw());
}, ((NixStringContextElem &&) i).raw);
}

auto attrs = state.buildBindings(contextInfos.size());
Expand Down
8 changes: 4 additions & 4 deletions src/libexpr/tests/value/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ TEST(NixStringContextElemTest, slash_invalid) {
TEST(NixStringContextElemTest, opaque) {
std::string_view opaque = "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x";
auto elem = NixStringContextElem::parse(opaque);
auto * p = std::get_if<NixStringContextElem::Opaque>(&elem);
auto * p = std::get_if<NixStringContextElem::Opaque>(&elem.raw);
ASSERT_TRUE(p);
ASSERT_EQ(p->path, StorePath { opaque });
ASSERT_EQ(elem.to_string(), opaque);
Expand All @@ -60,7 +60,7 @@ TEST(NixStringContextElemTest, opaque) {
TEST(NixStringContextElemTest, drvDeep) {
std::string_view drvDeep = "=g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv";
auto elem = NixStringContextElem::parse(drvDeep);
auto * p = std::get_if<NixStringContextElem::DrvDeep>(&elem);
auto * p = std::get_if<NixStringContextElem::DrvDeep>(&elem.raw);
ASSERT_TRUE(p);
ASSERT_EQ(p->drvPath, StorePath { drvDeep.substr(1) });
ASSERT_EQ(elem.to_string(), drvDeep);
Expand All @@ -73,7 +73,7 @@ TEST(NixStringContextElemTest, drvDeep) {
TEST(NixStringContextElemTest, built_opaque) {
std::string_view built = "!foo!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv";
auto elem = NixStringContextElem::parse(built);
auto * p = std::get_if<NixStringContextElem::Built>(&elem);
auto * p = std::get_if<NixStringContextElem::Built>(&elem.raw);
ASSERT_TRUE(p);
ASSERT_EQ(p->output, "foo");
ASSERT_EQ(*p->drvPath, ((SingleDerivedPath) SingleDerivedPath::Opaque {
Expand All @@ -96,7 +96,7 @@ TEST(NixStringContextElemTest, built_built) {

std::string_view built = "!foo!bar!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv";
auto elem = NixStringContextElem::parse(built, mockXpSettings);
auto * p = std::get_if<NixStringContextElem::Built>(&elem);
auto * p = std::get_if<NixStringContextElem::Built>(&elem.raw);
ASSERT_TRUE(p);
ASSERT_EQ(p->output, "foo");
auto * drvPath = std::get_if<SingleDerivedPath::Built>(&*p->drvPath);
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/value/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ std::string NixStringContextElem::to_string() const
res += '=';
res += d.drvPath.to_string();
},
}, raw());
}, raw);

return res;
}
Expand Down
94 changes: 41 additions & 53 deletions src/libexpr/value/context.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
#include "util.hh"
#include "comparator.hh"
#include "derived-path.hh"

#include <variant>
#include "variant-wrapper.hh"

#include <nlohmann/json_fwd.hpp>

Expand All @@ -26,58 +25,47 @@ public:
}
};

/**
* Plain opaque path to some store object.
*
* Encoded as just the path: ‘<path>’.
*/
typedef SingleDerivedPath::Opaque NixStringContextElem_Opaque;

/**
* Path to a derivation and its entire build closure.
*
* The path doesn't just refer to derivation itself and its closure, but
* also all outputs of all derivations in that closure (including the
* root derivation).
*
* Encoded in the form ‘=<drvPath>’.
*/
struct NixStringContextElem_DrvDeep {
StorePath drvPath;

GENERATE_CMP(NixStringContextElem_DrvDeep, me->drvPath);
};
struct NixStringContextElem {
/**
* Plain opaque path to some store object.
*
* Encoded as just the path: ‘<path>’.
*/
using Opaque = SingleDerivedPath::Opaque;

/**
* Derivation output.
*
* Encoded in the form ‘!<output>!<drvPath>’.
*/
typedef SingleDerivedPath::Built NixStringContextElem_Built;

using _NixStringContextElem_Raw = std::variant<
NixStringContextElem_Opaque,
NixStringContextElem_DrvDeep,
NixStringContextElem_Built
>;

struct NixStringContextElem : _NixStringContextElem_Raw {
using Raw = _NixStringContextElem_Raw;
using Raw::Raw;

using Opaque = NixStringContextElem_Opaque;
using DrvDeep = NixStringContextElem_DrvDeep;
using Built = NixStringContextElem_Built;

inline const Raw & raw() const & {
return static_cast<const Raw &>(*this);
}
inline Raw & raw() & {
return static_cast<Raw &>(*this);
}
inline Raw && raw() && {
return static_cast<Raw &&>(*this);
}
/**
* Path to a derivation and its entire build closure.
*
* The path doesn't just refer to derivation itself and its closure, but
* also all outputs of all derivations in that closure (including the
* root derivation).
*
* Encoded in the form ‘=<drvPath>’.
*/
struct DrvDeep {
StorePath drvPath;

GENERATE_CMP(DrvDeep, me->drvPath);
};

/**
* Derivation output.
*
* Encoded in the form ‘!<output>!<drvPath>’.
*/
using Built = SingleDerivedPath::Built;

using Raw = std::variant<
Opaque,
DrvDeep,
Built
>;

Raw raw;

GENERATE_CMP(NixStringContextElem, me->raw);

MAKE_WRAPPER_CONSTRUCTOR(NixStringContextElem);

/**
* Decode a context string, one of:
Expand Down
7 changes: 4 additions & 3 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ void DerivationGoal::inputsRealised()
[&](const DerivationType::Impure &) {
return true;
}
}, drvType.raw());
}, drvType.raw);

if (resolveDrv && !fullDrv.inputDrvs.empty()) {
experimentalFeatureSettings.require(Xp::CaDerivations);
Expand Down Expand Up @@ -996,10 +996,11 @@ void DerivationGoal::buildDone()
}

else {
assert(derivationType);
st =
dynamic_cast<NotDeterministic*>(&e) ? BuildResult::NotDeterministic :
statusOk(status) ? BuildResult::OutputRejected :
!derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure :
!derivationType->isSandboxed() || diskFull ? BuildResult::TransientFailure :
BuildResult::PermanentFailure;
}

Expand Down Expand Up @@ -1358,7 +1359,7 @@ std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity()
[&](const OutputsSpec::Names & names) {
return static_cast<StringSet>(names);
},
}, wantedOutputs.raw());
}, wantedOutputs.raw);
SingleDrvOutputs validOutputs;

for (auto & i : queryPartialDerivationOutputMap()) {
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/build/derivation-goal.hh
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ struct DerivationGoal : public Goal
/**
* The sort of derivation we are building.
*/
DerivationType derivationType;
std::optional<DerivationType> derivationType;

typedef void (DerivationGoal::*GoalState)();
GoalState state;
Expand Down
Loading