From 9121fed4b4d02c286166373fe9805773afb13694 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 16 Aug 2023 12:29:23 -0400 Subject: [PATCH] Fixing #7479 Types converted: - `NixStringContextElem` - `OutputsSpec` - `ExtendedOutputsSpec` - `DerivationOutput` - `DerivationType` Existing ones mostly conforming the pattern cleaned up: - `ContentAddressMethod` - `ContentAddressWithReferences` The `DerivationGoal::derivationType` field had a bogus initialization, now caught, so I made it `std::optional`. I think #8829 can make it non-optional again because it will ensure we always have the derivation when we construct a `DerivationGoal`. See that issue (#7479) for details on the general goal. `git grep 'Raw::Raw'` indicates the two types I didn't yet convert `DerivedPath` and `BuiltPath` (and their `Single` variants) . This is because @roberth and I (can't find issue right now...) plan on reworking them somewhat, so I didn't want to churn them more just yet. Co-authored-by: Eelco Dolstra --- src/libcmd/installable-attr-path.cc | 5 +- src/libcmd/installable-derived-path.cc | 2 +- src/libcmd/installable-flake.cc | 2 +- src/libcmd/installables.cc | 6 +- src/libexpr/eval-cache.cc | 2 +- src/libexpr/eval.cc | 2 +- src/libexpr/flake/flakeref.cc | 2 +- src/libexpr/primops.cc | 14 +- src/libexpr/primops/context.cc | 6 +- src/libexpr/tests/value/context.cc | 8 +- src/libexpr/value/context.cc | 2 +- src/libexpr/value/context.hh | 82 +++--- src/libstore/build/derivation-goal.cc | 7 +- src/libstore/build/derivation-goal.hh | 2 +- src/libstore/build/local-derivation-goal.cc | 18 +- src/libstore/build/worker.cc | 5 +- src/libstore/content-address.cc | 2 +- src/libstore/content-address.hh | 12 +- src/libstore/derivations.cc | 36 +-- src/libstore/derivations.hh | 311 ++++++++++---------- src/libstore/misc.cc | 8 +- src/libstore/outputs-spec.cc | 20 +- src/libstore/outputs-spec.hh | 95 +++--- src/libstore/path-with-outputs.cc | 2 +- src/libutil/variant-wrapper.hh | 30 ++ src/nix/app.cc | 2 +- src/nix/bundle.cc | 2 +- src/nix/develop.cc | 2 +- src/nix/flake.cc | 2 +- 29 files changed, 355 insertions(+), 334 deletions(-) create mode 100644 src/libutil/variant-wrapper.hh diff --git a/src/libcmd/installable-attr-path.cc b/src/libcmd/installable-attr-path.cc index 2f89eee02..06e507872 100644 --- a/src/libcmd/installable-attr-path.cc +++ b/src/libcmd/installable-attr-path.cc @@ -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); @@ -96,6 +96,7 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths() .outputs = outputs, }, .info = make_ref(ExtraPathInfoValue::Value { + .extendedOutputsSpec = outputs, /* FIXME: reconsider backwards compatibility above so we can fill in this info. */ }), @@ -114,7 +115,7 @@ InstallableAttrPath InstallableAttrPath::parse( return { state, cmd, v, prefix == "." ? "" : std::string { prefix }, - extendedOutputsSpec + std::move(extendedOutputsSpec), }; } diff --git a/src/libcmd/installable-derived-path.cc b/src/libcmd/installable-derived-path.cc index b45641e8a..4d1f83a1c 100644 --- a/src/libcmd/installable-derived-path.cc +++ b/src/libcmd/installable-derived-path.cc @@ -55,7 +55,7 @@ InstallableDerivedPath InstallableDerivedPath::parse( .outputs = outputSpec, }; }, - }, extendedOutputsSpec.raw()); + }, extendedOutputsSpec.raw); return InstallableDerivedPath { store, std::move(derivedPath), diff --git a/src/libcmd/installable-flake.cc b/src/libcmd/installable-flake.cc index 1b169c3bd..4074da06d 100644 --- a/src/libcmd/installable-flake.cc +++ b/src/libcmd/installable-flake.cc @@ -141,7 +141,7 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths() [&](const ExtendedOutputsSpec::Explicit & e) -> OutputsSpec { return e; }, - }, extendedOutputsSpec.raw()), + }, extendedOutputsSpec.raw), }, .info = make_ref( ExtraPathInfoValue::Value { diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 4c7d134ec..eb1903084 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -459,7 +459,7 @@ Installables SourceExprCommand::parseInstallables( result.push_back( make_ref( InstallableAttrPath::parse( - state, *this, vFile, prefix, extendedOutputsSpec))); + state, *this, vFile, std::move(prefix), std::move(extendedOutputsSpec)))); } } else { @@ -475,7 +475,7 @@ Installables SourceExprCommand::parseInstallables( if (prefix.find('/') != std::string::npos) { try { result.push_back(make_ref( - InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec))); + InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec.raw))); continue; } catch (BadStorePath &) { } catch (...) { @@ -491,7 +491,7 @@ Installables SourceExprCommand::parseInstallables( getEvalState(), std::move(flakeRef), fragment, - extendedOutputsSpec, + std::move(extendedOutputsSpec), getDefaultFlakeAttrPaths(), getDefaultFlakeAttrPathPrefixes(), lockFlags)); diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 6a60ba87b..2c9aa5532 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -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; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 79e2c4727..7e839dbe7 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2364,7 +2364,7 @@ std::pair 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), diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index d3fa1d557..e1bce90bc 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -246,7 +246,7 @@ std::tuple 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)}; } } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 283f99a48..5067da449 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -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 {}; @@ -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? */ @@ -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, }); } @@ -1373,7 +1373,7 @@ 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), }); } @@ -1381,7 +1381,7 @@ drvName, Bindings * attrs, Value & v) ; case DrvHash::Kind::Deferred: for (auto & i : outputs) { - drv.outputs.insert_or_assign(i, DerivationOutputDeferred {}); + drv.outputs.insert_or_assign(i, DerivationOutput::Deferred {}); } } } @@ -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(&c)) + if (auto p = std::get_if(&c.raw)) refs.insert(p->path); else state.debugThrowLastTrace(EvalError({ diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index bfc731744..e8542503a 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -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(&c)) { + if (auto * ptr = std::get_if(&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); } } @@ -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()); diff --git a/src/libexpr/tests/value/context.cc b/src/libexpr/tests/value/context.cc index c56b50b59..19407d071 100644 --- a/src/libexpr/tests/value/context.cc +++ b/src/libexpr/tests/value/context.cc @@ -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(&elem); + auto * p = std::get_if(&elem.raw); ASSERT_TRUE(p); ASSERT_EQ(p->path, StorePath { opaque }); ASSERT_EQ(elem.to_string(), opaque); @@ -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(&elem); + auto * p = std::get_if(&elem.raw); ASSERT_TRUE(p); ASSERT_EQ(p->drvPath, StorePath { drvDeep.substr(1) }); ASSERT_EQ(elem.to_string(), drvDeep); @@ -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(&elem); + auto * p = std::get_if(&elem.raw); ASSERT_TRUE(p); ASSERT_EQ(p->output, "foo"); ASSERT_EQ(*p->drvPath, ((SingleDerivedPath) SingleDerivedPath::Opaque { @@ -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(&elem); + auto * p = std::get_if(&elem.raw); ASSERT_TRUE(p); ASSERT_EQ(p->output, "foo"); auto * drvPath = std::get_if(&*p->drvPath); diff --git a/src/libexpr/value/context.cc b/src/libexpr/value/context.cc index d8116011e..22361d8fa 100644 --- a/src/libexpr/value/context.cc +++ b/src/libexpr/value/context.cc @@ -99,7 +99,7 @@ std::string NixStringContextElem::to_string() const res += '='; res += d.drvPath.to_string(); }, - }, raw()); + }, raw); return res; } diff --git a/src/libexpr/value/context.hh b/src/libexpr/value/context.hh index a1b71695b..9f1d59317 100644 --- a/src/libexpr/value/context.hh +++ b/src/libexpr/value/context.hh @@ -4,8 +4,7 @@ #include "util.hh" #include "comparator.hh" #include "derived-path.hh" - -#include +#include "variant-wrapper.hh" #include @@ -26,58 +25,47 @@ public: } }; -/** - * Plain opaque path to some store object. - * - * Encoded as just the path: ‘’. - */ -typedef SingleDerivedPath::Opaque NixStringContextElem_Opaque; +struct NixStringContextElem { + /** + * Plain opaque path to some store object. + * + * Encoded as just the path: ‘’. + */ + using Opaque = SingleDerivedPath::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 ‘=’. - */ -struct NixStringContextElem_DrvDeep { - StorePath drvPath; + /** + * 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 ‘=’. + */ + struct DrvDeep { + StorePath drvPath; - GENERATE_CMP(NixStringContextElem_DrvDeep, me->drvPath); -}; + GENERATE_CMP(DrvDeep, me->drvPath); + }; -/** - * Derivation output. - * - * Encoded in the form ‘!!’. - */ -typedef SingleDerivedPath::Built NixStringContextElem_Built; + /** + * Derivation output. + * + * Encoded in the form ‘!!’. + */ + using Built = SingleDerivedPath::Built; -using _NixStringContextElem_Raw = std::variant< - NixStringContextElem_Opaque, - NixStringContextElem_DrvDeep, - NixStringContextElem_Built ->; + using Raw = std::variant< + Opaque, + DrvDeep, + Built + >; -struct NixStringContextElem : _NixStringContextElem_Raw { - using Raw = _NixStringContextElem_Raw; - using Raw::Raw; + Raw raw; - using Opaque = NixStringContextElem_Opaque; - using DrvDeep = NixStringContextElem_DrvDeep; - using Built = NixStringContextElem_Built; + GENERATE_CMP(NixStringContextElem, me->raw); - inline const Raw & raw() const & { - return static_cast(*this); - } - inline Raw & raw() & { - return static_cast(*this); - } - inline Raw && raw() && { - return static_cast(*this); - } + MAKE_WRAPPER_CONSTRUCTOR(NixStringContextElem); /** * Decode a context string, one of: diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 8bdf2f367..84da7f2e1 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -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); @@ -996,10 +996,11 @@ void DerivationGoal::buildDone() } else { + assert(derivationType); st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : + !derivationType->isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } @@ -1358,7 +1359,7 @@ std::pair DerivationGoal::checkPathValidity() [&](const OutputsSpec::Names & names) { return static_cast(names); }, - }, wantedOutputs.raw()); + }, wantedOutputs.raw); SingleDrvOutputs validOutputs; for (auto & i : queryPartialDerivationOutputMap()) { diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index ee8f06f25..9d6fe1c0f 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -184,7 +184,7 @@ struct DerivationGoal : public Goal /** * The sort of derivation we are building. */ - DerivationType derivationType; + std::optional derivationType; typedef void (DerivationGoal::*GoalState)(); GoalState state; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 920097680..78f943d1f 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -178,6 +178,8 @@ void LocalDerivationGoal::tryLocalBuild() return; } + assert(derivationType); + /* Are we doing a chroot build? */ { auto noChroot = parsedDrv->getBoolAttr("__noChroot"); @@ -195,7 +197,7 @@ void LocalDerivationGoal::tryLocalBuild() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = derivationType.isSandboxed() && !noChroot; + useChroot = derivationType->isSandboxed() && !noChroot; } auto & localStore = getLocalStore(); @@ -689,7 +691,7 @@ void LocalDerivationGoal::startBuilder() "nogroup:x:65534:\n", sandboxGid())); /* Create /etc/hosts with localhost entry. */ - if (derivationType.isSandboxed()) + if (derivationType->isSandboxed()) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -893,7 +895,7 @@ void LocalDerivationGoal::startBuilder() us. */ - if (derivationType.isSandboxed()) + if (derivationType->isSandboxed()) privateNetwork = true; userNamespaceSync.create(); @@ -1121,7 +1123,7 @@ void LocalDerivationGoal::initEnv() derivation, tell the builder, so that for instance `fetchurl' can skip checking the output. On older Nixes, this environment variable won't be set, so `fetchurl' will do the check. */ - if (derivationType.isFixed()) env["NIX_OUTPUT_CHECKED"] = "1"; + if (derivationType->isFixed()) env["NIX_OUTPUT_CHECKED"] = "1"; /* *Only* if this is a fixed-output derivation, propagate the values of the environment variables specified in the @@ -1132,7 +1134,7 @@ void LocalDerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (!derivationType.isSandboxed()) { + if (!derivationType->isSandboxed()) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -1797,7 +1799,7 @@ void LocalDerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (!derivationType.isSandboxed()) { + if (!derivationType->isSandboxed()) { // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the @@ -2048,7 +2050,7 @@ void LocalDerivationGoal::runChild() #include "sandbox-defaults.sb" ; - if (!derivationType.isSandboxed()) + if (!derivationType->isSandboxed()) sandboxProfile += #include "sandbox-network.sb" ; @@ -2599,7 +2601,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() }); }, - }, output->raw()); + }, output->raw); /* FIXME: set proper permissions in restorePath() so we don't have to do another traversal. */ diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 6779dbcf3..b58fc5c1c 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -268,7 +268,10 @@ void Worker::run(const Goals & _topGoals) for (auto & i : _topGoals) { topGoals.insert(i); if (auto goal = dynamic_cast(i.get())) { - topPaths.push_back(DerivedPath::Built{makeConstantStorePathRef(goal->drvPath), goal->wantedOutputs}); + topPaths.push_back(DerivedPath::Built { + .drvPath = makeConstantStorePathRef(goal->drvPath), + .outputs = goal->wantedOutputs, + }); } else if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Opaque{goal->storePath}); } diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 080456e18..e290a8d38 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -115,7 +115,7 @@ ContentAddress ContentAddress::parse(std::string_view rawCa) auto [caMethod, hashType] = parseContentAddressMethodPrefix(rest); return ContentAddress { - .method = std::move(caMethod).raw, + .method = std::move(caMethod), .hash = Hash::parseNonSRIUnprefixed(rest, hashType), }; } diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index 01b771e52..c4d619bdc 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -5,6 +5,7 @@ #include "hash.hh" #include "path.hh" #include "comparator.hh" +#include "variant-wrapper.hh" namespace nix { @@ -71,11 +72,7 @@ struct ContentAddressMethod GENERATE_CMP(ContentAddressMethod, me->raw); - /* The moral equivalent of `using Raw::Raw;` */ - ContentAddressMethod(auto &&... arg) - : raw(std::forward(arg)...) - { } - + MAKE_WRAPPER_CONSTRUCTOR(ContentAddressMethod); /** * Parse the prefix tag which indicates how the files @@ -252,10 +249,7 @@ struct ContentAddressWithReferences GENERATE_CMP(ContentAddressWithReferences, me->raw); - /* The moral equivalent of `using Raw::Raw;` */ - ContentAddressWithReferences(auto &&... arg) - : raw(std::forward(arg)...) - { } + MAKE_WRAPPER_CONSTRUCTOR(ContentAddressWithReferences); /** * Create a `ContentAddressWithReferences` from a mere diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index f4e4980c2..0b8bdaf1c 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -32,7 +32,7 @@ std::optional DerivationOutput::path(const Store & store, std::string [](const DerivationOutput::Impure &) -> std::optional { return std::nullopt; }, - }, raw()); + }, raw); } @@ -60,7 +60,7 @@ bool DerivationType::isCA() const [](const Impure &) { return true; }, - }, raw()); + }, raw); } bool DerivationType::isFixed() const @@ -75,7 +75,7 @@ bool DerivationType::isFixed() const [](const Impure &) { return false; }, - }, raw()); + }, raw); } bool DerivationType::hasKnownOutputPaths() const @@ -90,7 +90,7 @@ bool DerivationType::hasKnownOutputPaths() const [](const Impure &) { return false; }, - }, raw()); + }, raw); } @@ -106,7 +106,7 @@ bool DerivationType::isSandboxed() const [](const Impure &) { return false; }, - }, raw()); + }, raw); } @@ -122,7 +122,7 @@ bool DerivationType::isPure() const [](const Impure &) { return false; }, - }, raw()); + }, raw); } @@ -408,13 +408,13 @@ std::string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); }, - [&](const DerivationOutputImpure & doi) { + [&](const DerivationOutput::Impure & doi) { // FIXME s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, doi.method.renderPrefix() + printHashType(doi.hashType)); s += ','; printUnquotedString(s, "impure"); } - }, i.second.raw()); + }, i.second.raw); s += ')'; } @@ -509,7 +509,7 @@ DerivationType BasicDerivation::type() const [&](const DerivationOutput::Impure &) { impureOutputs.insert(i.first); }, - }, i.second.raw()); + }, i.second.raw); } if (inputAddressedOutputs.empty() @@ -626,7 +626,7 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut if (type.isFixed()) { std::map outputHashes; for (const auto & i : drv.outputs) { - auto & dof = std::get(i.second.raw()); + auto & dof = std::get(i.second.raw); auto hash = hashString(htSHA256, "fixed:out:" + dof.ca.printMethodAlgo() + ":" + dof.ca.hash.to_string(Base16, false) + ":" @@ -663,7 +663,7 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut [](const DerivationType::Impure &) -> DrvHash::Kind { assert(false); } - }, drv.type().raw()); + }, drv.type().raw); std::map inputs2; for (auto & [drvPath, inputOutputs0] : drv.inputDrvs) { @@ -720,10 +720,10 @@ StringSet BasicDerivation::outputNames() const DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const { DerivationOutputsAndOptPaths outsAndOptPaths; - for (auto output : outputs) + for (auto & [outputName, output] : outputs) outsAndOptPaths.insert(std::make_pair( - output.first, - std::make_pair(output.second, output.second.path(store, name, output.first)) + outputName, + std::make_pair(output, output.path(store, name, outputName)) ) ); return outsAndOptPaths; @@ -798,7 +798,7 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr << (doi.method.renderPrefix() + printHashType(doi.hashType)) << "impure"; }, - }, i.second.raw()); + }, i.second.raw); } WorkerProto::write(store, WorkerProto::WriteConn { .to = out }, @@ -840,7 +840,7 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { - if (std::holds_alternative(output.raw())) { + if (std::holds_alternative(output.raw)) { auto h = get(hashModulo.hashes, outputName); if (!h) throw Error("derivation '%s' output '%s' has no hash (derivations.cc/rewriteDerivation)", @@ -955,7 +955,7 @@ void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const [&](const DerivationOutput::Impure &) { /* Nothing to check */ }, - }, i.second.raw()); + }, i.second.raw); } } @@ -984,7 +984,7 @@ nlohmann::json DerivationOutput::toJSON( res["hashAlgo"] = doi.method.renderPrefix() + printHashType(doi.hashType); res["impure"] = true; }, - }, raw()); + }, raw); return res; } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index fa79f77fd..a92082089 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -9,6 +9,7 @@ #include "derived-path.hh" #include "sync.hh" #include "comparator.hh" +#include "variant-wrapper.hh" #include #include @@ -20,108 +21,110 @@ class Store; /* Abstract syntax of derivations. */ -/** - * The traditional non-fixed-output derivation type. - */ -struct DerivationOutputInputAddressed -{ - StorePath path; - - GENERATE_CMP(DerivationOutputInputAddressed, me->path); -}; - -/** - * Fixed-output derivations, whose output paths are content - * addressed according to that fixed output. - */ -struct DerivationOutputCAFixed -{ - /** - * Method and hash used for expected hash computation. - * - * References are not allowed by fiat. - */ - ContentAddress ca; - - /** - * Return the \ref StorePath "store path" corresponding to this output - * - * @param drvName The name of the derivation this is an output of, without the `.drv`. - * @param outputName The name of this output. - */ - StorePath path(const Store & store, std::string_view drvName, std::string_view outputName) const; - - GENERATE_CMP(DerivationOutputCAFixed, me->ca); -}; - -/** - * Floating-output derivations, whose output paths are content - * addressed, but not fixed, and so are dynamically calculated from - * whatever the output ends up being. - * */ -struct DerivationOutputCAFloating -{ - /** - * How the file system objects will be serialized for hashing - */ - ContentAddressMethod method; - - /** - * How the serialization will be hashed - */ - HashType hashType; - - GENERATE_CMP(DerivationOutputCAFloating, me->method, me->hashType); -}; - -/** - * Input-addressed output which depends on a (CA) derivation whose hash - * isn't known yet. - */ -struct DerivationOutputDeferred { - GENERATE_CMP(DerivationOutputDeferred); -}; - -/** - * Impure output which is moved to a content-addressed location (like - * CAFloating) but isn't registered as a realization. - */ -struct DerivationOutputImpure -{ - /** - * How the file system objects will be serialized for hashing - */ - ContentAddressMethod method; - - /** - * How the serialization will be hashed - */ - HashType hashType; - - GENERATE_CMP(DerivationOutputImpure, me->method, me->hashType); -}; - -typedef std::variant< - DerivationOutputInputAddressed, - DerivationOutputCAFixed, - DerivationOutputCAFloating, - DerivationOutputDeferred, - DerivationOutputImpure -> _DerivationOutputRaw; - /** * A single output of a BasicDerivation (and Derivation). */ -struct DerivationOutput : _DerivationOutputRaw +struct DerivationOutput { - using Raw = _DerivationOutputRaw; - using Raw::Raw; + /** + * The traditional non-fixed-output derivation type. + */ + struct InputAddressed + { + StorePath path; - using InputAddressed = DerivationOutputInputAddressed; - using CAFixed = DerivationOutputCAFixed; - using CAFloating = DerivationOutputCAFloating; - using Deferred = DerivationOutputDeferred; - using Impure = DerivationOutputImpure; + GENERATE_CMP(InputAddressed, me->path); + }; + + /** + * Fixed-output derivations, whose output paths are content + * addressed according to that fixed output. + */ + struct CAFixed + { + /** + * Method and hash used for expected hash computation. + * + * References are not allowed by fiat. + */ + ContentAddress ca; + + /** + * Return the \ref StorePath "store path" corresponding to this output + * + * @param drvName The name of the derivation this is an output of, without the `.drv`. + * @param outputName The name of this output. + */ + StorePath path(const Store & store, std::string_view drvName, std::string_view outputName) const; + + GENERATE_CMP(CAFixed, me->ca); + }; + + /** + * Floating-output derivations, whose output paths are content + * addressed, but not fixed, and so are dynamically calculated from + * whatever the output ends up being. + * */ + struct CAFloating + { + /** + * How the file system objects will be serialized for hashing + */ + ContentAddressMethod method; + + /** + * How the serialization will be hashed + */ + HashType hashType; + + GENERATE_CMP(CAFloating, me->method, me->hashType); + }; + + /** + * Input-addressed output which depends on a (CA) derivation whose hash + * isn't known yet. + */ + struct Deferred { + GENERATE_CMP(Deferred); + }; + + /** + * Impure output which is moved to a content-addressed location (like + * CAFloating) but isn't registered as a realization. + */ + struct Impure + { + /** + * How the file system objects will be serialized for hashing + */ + ContentAddressMethod method; + + /** + * How the serialization will be hashed + */ + HashType hashType; + + GENERATE_CMP(Impure, me->method, me->hashType); + }; + + typedef std::variant< + InputAddressed, + CAFixed, + CAFloating, + Deferred, + Impure + > Raw; + + Raw raw; + + GENERATE_CMP(DerivationOutput, me->raw); + + MAKE_WRAPPER_CONSTRUCTOR(DerivationOutput); + + /** + * Force choosing a variant + */ + DerivationOutput() = delete; /** * \note when you use this function you should make sure that you're @@ -131,10 +134,6 @@ struct DerivationOutput : _DerivationOutputRaw */ std::optional path(const Store & store, std::string_view drvName, std::string_view outputName) const; - inline const Raw & raw() const { - return static_cast(*this); - } - nlohmann::json toJSON( const Store & store, std::string_view drvName, @@ -167,61 +166,71 @@ typedef std::map DerivationInputs; -/** - * Input-addressed derivation types - */ -struct DerivationType_InputAddressed { +struct DerivationType { /** - * True iff the derivation type can't be determined statically, - * for instance because it (transitively) depends on a content-addressed - * derivation. - */ - bool deferred; -}; - -/** - * Content-addressed derivation types - */ -struct DerivationType_ContentAddressed { - /** - * Whether the derivation should be built safely inside a sandbox. + * Input-addressed derivation types */ - bool sandboxed; + struct InputAddressed { + /** + * True iff the derivation type can't be determined statically, + * for instance because it (transitively) depends on a content-addressed + * derivation. + */ + bool deferred; + + GENERATE_CMP(InputAddressed, me->deferred); + }; + /** - * Whether the derivation's outputs' content-addresses are "fixed" - * or "floating. - * - * - Fixed: content-addresses are written down as part of the - * derivation itself. If the outputs don't end up matching the - * build fails. - * - * - Floating: content-addresses are not written down, we do not - * know them until we perform the build. + * Content-addressed derivation types */ - bool fixed; -}; + struct ContentAddressed { + /** + * Whether the derivation should be built safely inside a sandbox. + */ + bool sandboxed; + /** + * Whether the derivation's outputs' content-addresses are "fixed" + * or "floating". + * + * - Fixed: content-addresses are written down as part of the + * derivation itself. If the outputs don't end up matching the + * build fails. + * + * - Floating: content-addresses are not written down, we do not + * know them until we perform the build. + */ + bool fixed; -/** - * Impure derivation type - * - * This is similar at buil-time to the content addressed, not standboxed, not fixed - * type, but has some restrictions on its usage. - */ -struct DerivationType_Impure { -}; + GENERATE_CMP(ContentAddressed, me->sandboxed, me->fixed); + }; -typedef std::variant< - DerivationType_InputAddressed, - DerivationType_ContentAddressed, - DerivationType_Impure -> _DerivationTypeRaw; + /** + * Impure derivation type + * + * This is similar at buil-time to the content addressed, not standboxed, not fixed + * type, but has some restrictions on its usage. + */ + struct Impure { + GENERATE_CMP(Impure); + }; -struct DerivationType : _DerivationTypeRaw { - using Raw = _DerivationTypeRaw; - using Raw::Raw; - using InputAddressed = DerivationType_InputAddressed; - using ContentAddressed = DerivationType_ContentAddressed; - using Impure = DerivationType_Impure; + typedef std::variant< + InputAddressed, + ContentAddressed, + Impure + > Raw; + + Raw raw; + + GENERATE_CMP(DerivationType, me->raw); + + MAKE_WRAPPER_CONSTRUCTOR(DerivationType); + + /** + * Force choosing a variant + */ + DerivationType() = delete; /** * Do the outputs of the derivation have paths calculated from their @@ -257,10 +266,6 @@ struct DerivationType : _DerivationTypeRaw { * closure, or if fixed output. */ bool hasKnownOutputPaths() const; - - inline const Raw & raw() const { - return static_cast(*this); - } }; struct BasicDerivation diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 185d61c15..631213306 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -88,7 +88,7 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv) auto out = drv.outputs.find("out"); if (out == drv.outputs.end()) return nullptr; - if (auto dof = std::get_if(&out->second)) { + if (auto dof = std::get_if(&out->second.raw)) { return &dof->ca; } return nullptr; @@ -370,7 +370,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, } return outputsOpt; }, - }, bfd.outputs.raw()); + }, bfd.outputs.raw); OutputPathMap outputs; for (auto & [outputName, outputPathOpt] : outputsOpt) { @@ -418,7 +418,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd) [&](const OutputsSpec::Names & names) { return static_cast(names); }, - }, bfd.outputs.raw()); + }, bfd.outputs.raw); for (auto iter = outputMap.begin(); iter != outputMap.end();) { auto & outputName = iter->first; if (bfd.outputs.contains(outputName)) { @@ -431,7 +431,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd) if (!outputsLeft.empty()) throw Error("derivation '%s' does not have an outputs %s", store.printStorePath(drvPath), - concatStringsSep(", ", quoteStrings(std::get(bfd.outputs)))); + concatStringsSep(", ", quoteStrings(std::get(bfd.outputs.raw)))); return outputMap; } diff --git a/src/libstore/outputs-spec.cc b/src/libstore/outputs-spec.cc index e26c38138..d943bc111 100644 --- a/src/libstore/outputs-spec.cc +++ b/src/libstore/outputs-spec.cc @@ -17,7 +17,7 @@ bool OutputsSpec::contains(const std::string & outputName) const [&](const OutputsSpec::Names & outputNames) { return outputNames.count(outputName) > 0; }, - }, raw()); + }, raw); } static std::string outputSpecRegexStr = @@ -49,7 +49,7 @@ OutputsSpec OutputsSpec::parse(std::string_view s) std::optional spec = parseOpt(s); if (!spec) throw Error("invalid outputs specifier '%s'", s); - return *spec; + return std::move(*spec); } @@ -85,7 +85,7 @@ std::string OutputsSpec::to_string() const [&](const OutputsSpec::Names & outputNames) -> std::string { return concatStringsSep(",", outputNames); }, - }, raw()); + }, raw); } @@ -98,7 +98,7 @@ std::string ExtendedOutputsSpec::to_string() const [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> std::string { return "^" + outputSpec.to_string(); }, - }, raw()); + }, raw); } @@ -118,9 +118,9 @@ OutputsSpec OutputsSpec::union_(const OutputsSpec & that) const ret.insert(thoseNames.begin(), thoseNames.end()); return ret; }, - }, that.raw()); + }, that.raw); }, - }, raw()); + }, raw); } @@ -142,9 +142,9 @@ bool OutputsSpec::isSubsetOf(const OutputsSpec & that) const ret = false; return ret; }, - }, raw()); + }, raw); }, - }, that.raw()); + }, that.raw); } } @@ -169,7 +169,7 @@ void adl_serializer::to_json(json & json, OutputsSpec t) { [&](const OutputsSpec::Names & names) { json = names; }, - }, t.raw()); + }, t.raw); } @@ -189,7 +189,7 @@ void adl_serializer::to_json(json & json, ExtendedOutputsSp [&](const ExtendedOutputsSpec::Explicit & e) { adl_serializer::to_json(json, e); }, - }, t.raw()); + }, t.raw); } } diff --git a/src/libstore/outputs-spec.hh b/src/libstore/outputs-spec.hh index 5a726fe90..ae19f1040 100644 --- a/src/libstore/outputs-spec.hh +++ b/src/libstore/outputs-spec.hh @@ -6,62 +6,57 @@ #include #include +#include "comparator.hh" #include "json-impls.hh" +#include "comparator.hh" +#include "variant-wrapper.hh" namespace nix { -/** - * A non-empty set of outputs, specified by name - */ -struct OutputNames : std::set { - using std::set::set; +struct OutputsSpec { + /** + * A non-empty set of outputs, specified by name + */ + struct Names : std::set { + using std::set::set; - /* These need to be "inherited manually" */ + /* These need to be "inherited manually" */ - OutputNames(const std::set & s) - : std::set(s) - { assert(!empty()); } + Names(const std::set & s) + : std::set(s) + { assert(!empty()); } + + /** + * Needs to be "inherited manually" + */ + Names(std::set && s) + : std::set(s) + { assert(!empty()); } + + /* This set should always be non-empty, so we delete this + constructor in order make creating empty ones by mistake harder. + */ + Names() = delete; + }; /** - * Needs to be "inherited manually" + * The set of all outputs, without needing to name them explicitly */ - OutputNames(std::set && s) - : std::set(s) - { assert(!empty()); } + struct All : std::monostate { }; - /* This set should always be non-empty, so we delete this - constructor in order make creating empty ones by mistake harder. - */ - OutputNames() = delete; -}; + typedef std::variant Raw; -/** - * The set of all outputs, without needing to name them explicitly - */ -struct AllOutputs : std::monostate { }; + Raw raw; -typedef std::variant _OutputsSpecRaw; + GENERATE_CMP(OutputsSpec, me->raw); -struct OutputsSpec : _OutputsSpecRaw { - using Raw = _OutputsSpecRaw; - using Raw::Raw; + MAKE_WRAPPER_CONSTRUCTOR(OutputsSpec); /** * Force choosing a variant */ OutputsSpec() = delete; - using Names = OutputNames; - using All = AllOutputs; - - inline const Raw & raw() const { - return static_cast(*this); - } - - inline Raw & raw() { - return static_cast(*this); - } - bool contains(const std::string & output) const; /** @@ -84,20 +79,22 @@ struct OutputsSpec : _OutputsSpecRaw { std::string to_string() const; }; -struct DefaultOutputs : std::monostate { }; - -typedef std::variant _ExtendedOutputsSpecRaw; - -struct ExtendedOutputsSpec : _ExtendedOutputsSpecRaw { - using Raw = _ExtendedOutputsSpecRaw; - using Raw::Raw; - - using Default = DefaultOutputs; +struct ExtendedOutputsSpec { + struct Default : std::monostate { }; using Explicit = OutputsSpec; - inline const Raw & raw() const { - return static_cast(*this); - } + typedef std::variant Raw; + + Raw raw; + + GENERATE_CMP(ExtendedOutputsSpec, me->raw); + + MAKE_WRAPPER_CONSTRUCTOR(ExtendedOutputsSpec); + + /** + * Force choosing a variant + */ + ExtendedOutputsSpec() = delete; /** * Parse a string of the form 'prefix^output1,...outputN' or diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index 81f360f3a..af6837370 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -63,7 +63,7 @@ StorePathWithOutputs::ParseResult StorePathWithOutputs::tryFromDerivedPath(const [&](const OutputsSpec::Names & outputs) { return static_cast(outputs); }, - }, bfd.outputs.raw()), + }, bfd.outputs.raw), }; }, [&](const SingleDerivedPath::Built &) -> StorePathWithOutputs::ParseResult { diff --git a/src/libutil/variant-wrapper.hh b/src/libutil/variant-wrapper.hh new file mode 100644 index 000000000..cedcb999c --- /dev/null +++ b/src/libutil/variant-wrapper.hh @@ -0,0 +1,30 @@ +#pragma once +///@file + +// not used, but will be used by callers +#include + +/** + * Force the default versions of all constructors (copy, move, copy + * assignment). + */ +#define FORCE_DEFAULT_CONSTRUCTORS(CLASS_NAME) \ + CLASS_NAME(const CLASS_NAME &) = default; \ + CLASS_NAME(CLASS_NAME &) = default; \ + CLASS_NAME(CLASS_NAME &&) = default; \ + \ + CLASS_NAME & operator =(const CLASS_NAME &) = default; \ + CLASS_NAME & operator =(CLASS_NAME &) = default; + +/** + * Make a wrapper constructor. All args are forwarded to the + * construction of the "raw" field. (Which we assume is the only one.) + * + * The moral equivalent of `using Raw::Raw;` + */ +#define MAKE_WRAPPER_CONSTRUCTOR(CLASS_NAME) \ + FORCE_DEFAULT_CONSTRUCTORS(CLASS_NAME) \ + \ + CLASS_NAME(auto &&... arg) \ + : raw(std::forward(arg)...) \ + { } diff --git a/src/nix/app.cc b/src/nix/app.cc index 16a921194..67c5ef344 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -81,7 +81,7 @@ UnresolvedApp InstallableValue::toApp(EvalState & state) .path = o.path, }; }, - }, c.raw())); + }, c.raw)); } return UnresolvedApp{App { diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 5a80f0308..fbc83b08e 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -80,7 +80,7 @@ struct CmdBundle : InstallableValueCommand auto [bundlerFlakeRef, bundlerName, extendedOutputsSpec] = parseFlakeRefWithFragmentAndExtendedOutputsSpec(bundler, absPath(".")); const flake::LockFlags lockFlags{ .writeLockFile = false }; InstallableFlake bundler{this, - evalState, std::move(bundlerFlakeRef), bundlerName, extendedOutputsSpec, + evalState, std::move(bundlerFlakeRef), bundlerName, std::move(extendedOutputsSpec), {"bundlers." + settings.thisSystem.get() + ".default", "defaultBundler." + settings.thisSystem.get() }, diff --git a/src/nix/develop.cc b/src/nix/develop.cc index c033804e4..c01ef1a2a 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -547,7 +547,7 @@ struct CmdDevelop : Common, MixEnvironment state, std::move(nixpkgs), "bashInteractive", - DefaultOutputs(), + ExtendedOutputsSpec::Default(), Strings{}, Strings{"legacyPackages." + settings.thisSystem.get() + "."}, nixpkgsLockFlags); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 83b74c8ca..87dd4da1b 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -778,7 +778,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand auto [templateFlakeRef, templateName] = parseFlakeRefWithFragment(templateUrl, absPath(".")); auto installable = InstallableFlake(nullptr, - evalState, std::move(templateFlakeRef), templateName, DefaultOutputs(), + evalState, std::move(templateFlakeRef), templateName, ExtendedOutputsSpec::Default(), defaultTemplateAttrPaths, defaultTemplateAttrPathsPrefixes, lockFlags);