From 845fc3f605be6dd1140ab81eefb969da1cc5346b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 15 Dec 2022 22:09:32 +0100 Subject: [PATCH] Merge toDerivations() into toDerivedPaths() toDerivedPaths() now returns DerivedPathWithInfo, which is DerivedPath with some attributes needed by 'nix profile' etc. Preparation for #7417. --- src/libcmd/installables.cc | 175 +++++++++++++++++-------------------- src/libcmd/installables.hh | 47 +++++----- src/nix/app.cc | 5 +- src/nix/build.cc | 10 ++- src/nix/log.cc | 2 +- src/nix/profile.cc | 67 ++++++++------ src/nix/store-copy-log.cc | 8 +- src/nix/why-depends.cc | 2 +- 8 files changed, 154 insertions(+), 162 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 5cdd3e12c..97bad5c45 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -358,7 +358,7 @@ void completeFlakeRef(ref store, std::string_view prefix) } } -DerivedPath Installable::toDerivedPath() +DerivedPathWithInfo Installable::toDerivedPath() { auto buildables = toDerivedPaths(); if (buildables.size() != 1) @@ -422,21 +422,9 @@ struct InstallableStorePath : Installable return req.to_string(*store); } - DerivedPaths toDerivedPaths() override + DerivedPathsWithInfo toDerivedPaths() override { - return { req }; - } - - StorePathSet toDrvPaths(ref store) override - { - return std::visit(overloaded { - [&](const DerivedPath::Built & bfd) -> StorePathSet { - return { bfd.drvPath }; - }, - [&](const DerivedPath::Opaque & bo) -> StorePathSet { - return { getDeriver(store, *this, bo.path) }; - }, - }, req.raw()); + return {{req}}; } std::optional getStorePath() override @@ -452,34 +440,6 @@ struct InstallableStorePath : Installable } }; -DerivedPaths InstallableValue::toDerivedPaths() -{ - DerivedPaths res; - - std::map> drvsToOutputs; - RealisedPath::Set drvsToCopy; - - // Group by derivation, helps with .all in particular - for (auto & drv : toDerivations()) { - for (auto & outputName : drv.outputsToInstall) - drvsToOutputs[drv.drvPath].insert(outputName); - drvsToCopy.insert(drv.drvPath); - } - - for (auto & i : drvsToOutputs) - res.push_back(DerivedPath::Built { i.first, i.second }); - - return res; -} - -StorePathSet InstallableValue::toDrvPaths(ref store) -{ - StorePathSet res; - for (auto & drv : toDerivations()) - res.insert(drv.drvPath); - return res; -} - struct InstallableAttrPath : InstallableValue { SourceExprCommand & cmd; @@ -509,40 +469,52 @@ struct InstallableAttrPath : InstallableValue return {vRes, pos}; } - virtual std::vector toDerivations() override; -}; + DerivedPathsWithInfo toDerivedPaths() override + { + auto v = toValue(*state).first; -std::vector InstallableAttrPath::toDerivations() -{ - auto v = toValue(*state).first; + Bindings & autoArgs = *cmd.getAutoArgs(*state); - Bindings & autoArgs = *cmd.getAutoArgs(*state); + DrvInfos drvInfos; + getDerivations(*state, *v, "", autoArgs, drvInfos, false); - DrvInfos drvInfos; - getDerivations(*state, *v, "", autoArgs, drvInfos, false); + DerivedPathsWithInfo res; - std::vector res; - for (auto & drvInfo : drvInfos) { - auto drvPath = drvInfo.queryDrvPath(); - if (!drvPath) - throw Error("'%s' is not a derivation", what()); + // Backward compatibility hack: group results by drvPath. This + // helps keep .all output together. + std::map byDrvPath; - std::set outputsToInstall; + for (auto & drvInfo : drvInfos) { + auto drvPath = drvInfo.queryDrvPath(); + if (!drvPath) + throw Error("'%s' is not a derivation", what()); - if (auto outputNames = std::get_if(&outputsSpec)) - outputsToInstall = *outputNames; - else - for (auto & output : drvInfo.queryOutputs(false, std::get_if(&outputsSpec))) - outputsToInstall.insert(output.first); + std::set outputsToInstall; - res.push_back(DerivationInfo { - .drvPath = *drvPath, - .outputsToInstall = std::move(outputsToInstall) - }); + if (auto outputNames = std::get_if(&outputsSpec)) + outputsToInstall = *outputNames; + else + for (auto & output : drvInfo.queryOutputs(false, std::get_if(&outputsSpec))) + outputsToInstall.insert(output.first); + + auto i = byDrvPath.find(*drvPath); + if (i == byDrvPath.end()) { + byDrvPath[*drvPath] = res.size(); + res.push_back({ + .path = DerivedPath::Built { + .drvPath = std::move(*drvPath), + .outputs = std::move(outputsToInstall), + } + }); + } else { + for (auto & output : outputsToInstall) + std::get(res[i->second].path).outputs.insert(output); + } + } + + return res; } - - return res; -} +}; std::vector InstallableFlake::getActualAttrPaths() { @@ -630,7 +602,7 @@ InstallableFlake::InstallableFlake( throw UsageError("'--arg' and '--argstr' are incompatible with flakes"); } -std::tuple InstallableFlake::toDerivation() +DerivedPathsWithInfo InstallableFlake::toDerivedPaths() { Activity act(*logger, lvlTalkative, actUnknown, fmt("evaluating derivation '%s'", what())); @@ -674,20 +646,19 @@ std::tuple InstallableF if (auto outputNames = std::get_if(&outputsSpec)) outputsToInstall = *outputNames; - auto drvInfo = DerivationInfo { - .drvPath = std::move(drvPath), - .outputsToInstall = std::move(outputsToInstall), - .priority = priority, - }; - - return {attrPath, getLockedFlake()->flake.lockedRef, std::move(drvInfo)}; -} - -std::vector InstallableFlake::toDerivations() -{ - std::vector res; - res.push_back(std::get<2>(toDerivation())); - return res; + return {{ + .path = DerivedPath::Built { + .drvPath = std::move(drvPath), + .outputs = std::move(outputsToInstall), + }, + .info = { + .priority = priority, + .originalRef = flakeRef, + .resolvedRef = getLockedFlake()->flake.lockedRef, + .attrPath = attrPath, + .outputsSpec = outputsSpec, + } + }}; } std::pair InstallableFlake::toValue(EvalState & state) @@ -895,13 +866,19 @@ std::vector, BuiltPathWithResult>> Instal if (mode == Realise::Nothing) settings.readOnlyMode = true; + struct Aux + { + ExtraInfo info; + std::shared_ptr installable; + }; + std::vector pathsToBuild; - std::map>> backmap; + std::map> backmap; for (auto & i : installables) { for (auto b : i->toDerivedPaths()) { - pathsToBuild.push_back(b); - backmap[b].push_back(i); + pathsToBuild.push_back(b.path); + backmap[b.path].push_back({.info = b.info, .installable = i}); } } @@ -914,7 +891,7 @@ std::vector, BuiltPathWithResult>> Instal printMissing(store, pathsToBuild, lvlError); for (auto & path : pathsToBuild) { - for (auto & installable : backmap[path]) { + for (auto & aux : backmap[path]) { std::visit(overloaded { [&](const DerivedPath::Built & bfd) { OutputPathMap outputs; @@ -946,10 +923,14 @@ std::vector, BuiltPathWithResult>> Instal output, *drvOutput->second); } } - res.push_back({installable, {.path = BuiltPath::Built { bfd.drvPath, outputs }}}); + res.push_back({aux.installable, { + .path = BuiltPath::Built { bfd.drvPath, outputs }, + .info = aux.info}}); }, [&](const DerivedPath::Opaque & bo) { - res.push_back({installable, {.path = BuiltPath::Opaque { bo.path }}}); + res.push_back({aux.installable, { + .path = BuiltPath::Opaque { bo.path }, + .info = aux.info}}); }, }, path.raw()); } @@ -965,16 +946,22 @@ std::vector, BuiltPathWithResult>> Instal if (!buildResult.success()) buildResult.rethrow(); - for (auto & installable : backmap[buildResult.path]) { + for (auto & aux : backmap[buildResult.path]) { std::visit(overloaded { [&](const DerivedPath::Built & bfd) { std::map outputs; for (auto & path : buildResult.builtOutputs) outputs.emplace(path.first.outputName, path.second.outPath); - res.push_back({installable, {.path = BuiltPath::Built { bfd.drvPath, outputs }, .result = buildResult}}); + res.push_back({aux.installable, { + .path = BuiltPath::Built { bfd.drvPath, outputs }, + .info = aux.info, + .result = buildResult}}); }, [&](const DerivedPath::Opaque & bo) { - res.push_back({installable, {.path = BuiltPath::Opaque { bo.path }, .result = buildResult}}); + res.push_back({aux.installable, { + .path = BuiltPath::Opaque { bo.path }, + .info = aux.info, + .result = buildResult}}); }, }, buildResult.path.raw()); } @@ -1059,7 +1046,7 @@ StorePathSet Installable::toDerivations( [&](const DerivedPath::Built & bfd) { drvPaths.insert(bfd.drvPath); }, - }, b.raw()); + }, b.path.raw()); return drvPaths; } diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index 02ea351d3..95ab4a40e 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -52,26 +52,42 @@ enum class OperateOn { Derivation }; +struct ExtraInfo +{ + std::optional priority; + std::optional originalRef; + std::optional resolvedRef; + std::optional attrPath; + // FIXME: merge with DerivedPath's 'outputs' field? + std::optional outputsSpec; +}; + +/* A derived path with any additional info that commands might + need from the derivation. */ +struct DerivedPathWithInfo +{ + DerivedPath path; + ExtraInfo info; +}; + struct BuiltPathWithResult { BuiltPath path; + ExtraInfo info; std::optional result; }; +typedef std::vector DerivedPathsWithInfo; + struct Installable { virtual ~Installable() { } virtual std::string what() const = 0; - virtual DerivedPaths toDerivedPaths() = 0; + virtual DerivedPathsWithInfo toDerivedPaths() = 0; - virtual StorePathSet toDrvPaths(ref store) - { - throw Error("'%s' cannot be converted to a derivation path", what()); - } - - DerivedPath toDerivedPath(); + DerivedPathWithInfo toDerivedPath(); UnresolvedApp toApp(EvalState & state); @@ -146,19 +162,6 @@ struct InstallableValue : Installable ref state; InstallableValue(ref state) : state(state) {} - - struct DerivationInfo - { - StorePath drvPath; - std::set outputsToInstall; - std::optional priority; - }; - - virtual std::vector toDerivations() = 0; - - DerivedPaths toDerivedPaths() override; - - StorePathSet toDrvPaths(ref store) override; }; struct InstallableFlake : InstallableValue @@ -186,9 +189,7 @@ struct InstallableFlake : InstallableValue Value * getFlakeOutputs(EvalState & state, const flake::LockedFlake & lockedFlake); - std::tuple toDerivation(); - - std::vector toDerivations() override; + DerivedPathsWithInfo toDerivedPaths() override; std::pair toValue(EvalState & state) override; diff --git a/src/nix/app.cc b/src/nix/app.cc index 5658f2a52..a8d7e115b 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -19,12 +19,11 @@ struct InstallableDerivedPath : Installable { } - std::string what() const override { return derivedPath.to_string(*store); } - DerivedPaths toDerivedPaths() override + DerivedPathsWithInfo toDerivedPaths() override { - return {derivedPath}; + return {{derivedPath}}; } std::optional getStorePath() override diff --git a/src/nix/build.cc b/src/nix/build.cc index 94b169167..12b22d999 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -94,13 +94,15 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile if (dryRun) { std::vector pathsToBuild; - for (auto & i : installables) { - auto b = i->toDerivedPaths(); - pathsToBuild.insert(pathsToBuild.end(), b.begin(), b.end()); - } + for (auto & i : installables) + for (auto & b : i->toDerivedPaths()) + pathsToBuild.push_back(b.path); + printMissing(store, pathsToBuild, lvlError); + if (json) logger->cout("%s", derivedPathsToJSON(pathsToBuild, store).dump()); + return; } diff --git a/src/nix/log.cc b/src/nix/log.cc index 72d02ef11..a0598ca13 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -49,7 +49,7 @@ struct CmdLog : InstallableCommand [&](const DerivedPath::Built & bfd) { return logSub.getBuildLog(bfd.drvPath); }, - }, b.raw()); + }, b.path.raw()); if (!log) continue; stopProgressBar(); printInfo("got build log for '%s' from '%s'", installable->what(), logSub.getUri()); diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 11910523d..db702db1b 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -32,12 +32,14 @@ struct ProfileElementSource } }; +const int defaultPriority = 5; + struct ProfileElement { StorePathSet storePaths; std::optional source; bool active = true; - int priority = 5; + int priority = defaultPriority; std::string describe() const { @@ -251,13 +253,19 @@ struct ProfileManifest } }; -static std::map +static std::map> builtPathsPerInstallable( const std::vector, BuiltPathWithResult>> & builtPaths) { - std::map res; - for (auto & [installable, builtPath] : builtPaths) - res[installable.get()].push_back(builtPath.path); + std::map> res; + for (auto & [installable, builtPath] : builtPaths) { + auto & r = res[installable.get()]; + /* Note that there could be conflicting info + (e.g. meta.priority fields) if the installable returned + multiple derivations. So pick one arbitrarily. */ + r.first.push_back(builtPath.path); + r.second = builtPath.info; + } return res; } @@ -297,28 +305,25 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile for (auto & installable : installables) { ProfileElement element; + auto & [res, info] = builtPaths[installable.get()]; - - if (auto installable2 = std::dynamic_pointer_cast(installable)) { - // FIXME: make build() return this? - auto [attrPath, resolvedRef, drv] = installable2->toDerivation(); + if (info.originalRef && info.resolvedRef && info.attrPath && info.outputsSpec) { element.source = ProfileElementSource { - installable2->flakeRef, - resolvedRef, - attrPath, - installable2->outputsSpec + .originalRef = *info.originalRef, + .resolvedRef = *info.resolvedRef, + .attrPath = *info.attrPath, + .outputs = *info.outputsSpec, }; - - if(drv.priority) { - element.priority = *drv.priority; - } } - if(priority) { // if --priority was specified we want to override the priority of the installable - element.priority = *priority; - }; + // If --priority was specified we want to override the + // priority of the installable. + element.priority = + priority + ? *priority + : info.priority.value_or(defaultPriority); - element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()]); + element.updateStorePaths(getEvalStore(), store, res); manifest.elements.push_back(std::move(element)); } @@ -476,18 +481,22 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf Strings{}, lockFlags); - auto [attrPath, resolvedRef, drv] = installable->toDerivation(); + auto derivedPaths = installable->toDerivedPaths(); + if (derivedPaths.empty()) continue; + auto & info = derivedPaths[0].info; - if (element.source->resolvedRef == resolvedRef) continue; + assert(info.resolvedRef && info.attrPath); + + if (element.source->resolvedRef == info.resolvedRef) continue; printInfo("upgrading '%s' from flake '%s' to '%s'", - element.source->attrPath, element.source->resolvedRef, resolvedRef); + element.source->attrPath, element.source->resolvedRef, *info.resolvedRef); element.source = ProfileElementSource { - installable->flakeRef, - resolvedRef, - attrPath, - installable->outputsSpec + .originalRef = installable->flakeRef, + .resolvedRef = *info.resolvedRef, + .attrPath = *info.attrPath, + .outputs = installable->outputsSpec, }; installables.push_back(installable); @@ -515,7 +524,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf for (size_t i = 0; i < installables.size(); ++i) { auto & installable = installables.at(i); auto & element = manifest.elements[indices.at(i)]; - element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()]); + element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()].first); } updateProfile(manifest.build(store)); diff --git a/src/nix/store-copy-log.cc b/src/nix/store-copy-log.cc index 2e288f743..d5fab5f2f 100644 --- a/src/nix/store-copy-log.cc +++ b/src/nix/store-copy-log.cc @@ -33,13 +33,7 @@ struct CmdCopyLog : virtual CopyCommand, virtual InstallablesCommand auto dstStore = getDstStore(); auto & dstLogStore = require(*dstStore); - StorePathSet drvPaths; - - for (auto & i : installables) - for (auto & drvPath : i->toDrvPaths(getEvalStore())) - drvPaths.insert(drvPath); - - for (auto & drvPath : drvPaths) { + for (auto & drvPath : Installable::toDerivations(getEvalStore(), installables, true)) { if (auto log = srcLogStore.getBuildLog(drvPath)) dstLogStore.addBuildLog(drvPath, *log); else diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 723017497..661df965e 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -111,7 +111,7 @@ struct CmdWhyDepends : SourceExprCommand } return maybePath->second; }, - }, derivedDependency.raw()); + }, derivedDependency.path.raw()); StorePathSet closure; store->computeFSClosure({packagePath}, closure, false, false);