From 9355ecd54301372b6a919a2205340f904c7a51c6 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 14 Dec 2020 17:24:30 +0100 Subject: [PATCH] Add a new Cmd type working on RealisedPaths Where a `RealisedPath` is a store path with its history, meaning either an opaque path for stuff that has been directly added to the store, or a `Realisation` for stuff that has been built by a derivation This is a low-level refactoring that doesn't bring anything by itself (except a few dozen extra lines of code :/ ), but raising the abstraction level a bit is important on a number of levels: - Commands like `nix build` have to query for the realisations after the build is finished which is fragile (see 27905f12e4a7207450abe37c9ed78e31603b67e1 for example). Having them oprate directly at the realisation level would avoid that - Others like `nix copy` currently operate directly on (built) store paths, but need a bit more information as they will need to register the realisations on the remote side --- src/libcmd/command.cc | 42 ++++++++++------- src/libcmd/command.hh | 23 ++++++++-- src/libcmd/installables.cc | 48 ++++++++++++++++---- src/libstore/realisation.cc | 31 +++++++++++++ src/libstore/realisation.hh | 90 +++++++++++++++++++++++++++++++++---- src/nix/copy.cc | 2 + 6 files changed, 200 insertions(+), 36 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index 614dee788..efdc98d5a 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -54,7 +54,7 @@ void StoreCommand::run() run(getStore()); } -StorePathsCommand::StorePathsCommand(bool recursive) +RealisedPathsCommand::RealisedPathsCommand(bool recursive) : recursive(recursive) { if (recursive) @@ -81,30 +81,40 @@ StorePathsCommand::StorePathsCommand(bool recursive) }); } -void StorePathsCommand::run(ref store) +void RealisedPathsCommand::run(ref store) { - StorePaths storePaths; - + std::vector paths; if (all) { if (installables.size()) throw UsageError("'--all' does not expect arguments"); + // XXX: Only uses opaque paths, ignores all the realisations for (auto & p : store->queryAllValidPaths()) - storePaths.push_back(p); - } - - else { - for (auto & p : toStorePaths(store, realiseMode, operateOn, installables)) - storePaths.push_back(p); - + paths.push_back(p); + } else { + auto pathSet = toRealisedPaths(store, realiseMode, operateOn, installables); if (recursive) { - StorePathSet closure; - store->computeFSClosure(StorePathSet(storePaths.begin(), storePaths.end()), closure, false, false); - storePaths.clear(); - for (auto & p : closure) - storePaths.push_back(p); + auto roots = std::move(pathSet); + pathSet = {}; + RealisedPath::closure(*store, roots, pathSet); } + for (auto & path : pathSet) + paths.push_back(path); } + run(store, std::move(paths)); +} + +StorePathsCommand::StorePathsCommand(bool recursive) + : RealisedPathsCommand(recursive) +{ +} + +void StorePathsCommand::run(ref store, std::vector paths) +{ + StorePaths storePaths; + for (auto & p : paths) + storePaths.push_back(p.path()); + run(store, std::move(storePaths)); } diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index ed6980075..8c0b3a94a 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -141,7 +141,7 @@ private: }; /* A command that operates on zero or more store paths. */ -struct StorePathsCommand : public InstallablesCommand +struct RealisedPathsCommand : public InstallablesCommand { private: @@ -154,17 +154,28 @@ protected: public: - StorePathsCommand(bool recursive = false); + RealisedPathsCommand(bool recursive = false); using StoreCommand::run; - virtual void run(ref store, std::vector storePaths) = 0; + virtual void run(ref store, std::vector paths) = 0; void run(ref store) override; bool useDefaultInstallables() override { return !all; } }; +struct StorePathsCommand : public RealisedPathsCommand +{ + StorePathsCommand(bool recursive = false); + + using RealisedPathsCommand::run; + + virtual void run(ref store, std::vector storePaths) = 0; + + void run(ref store, std::vector paths) override; +}; + /* A command that operates on exactly one store path. */ struct StorePathCommand : public InstallablesCommand { @@ -218,6 +229,12 @@ std::set toDerivations(ref store, std::vector> installables, bool useDeriver = false); +std::set toRealisedPaths( + ref store, + Realise mode, + OperateOn operateOn, + std::vector> installables); + /* Helper function to generate args that invoke $EDITOR on filename:lineno. */ Strings editorFor(const Pos & pos); diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 4e6bf4a9a..98a27ded9 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -704,23 +704,43 @@ Buildables build(ref store, Realise mode, return buildables; } -StorePathSet toStorePaths(ref store, - Realise mode, OperateOn operateOn, +std::set toRealisedPaths( + ref store, + Realise mode, + OperateOn operateOn, std::vector> installables) { - StorePathSet outPaths; - + std::set res; if (operateOn == OperateOn::Output) { for (auto & b : build(store, mode, installables)) std::visit(overloaded { [&](BuildableOpaque bo) { - outPaths.insert(bo.path); + res.insert(bo.path); }, [&](BuildableFromDrv bfd) { + auto drv = store->readDerivation(bfd.drvPath); + auto outputHashes = staticOutputHashes(*store, drv); for (auto & output : bfd.outputs) { - if (!output.second) - throw Error("Cannot operate on output of unbuilt CA drv"); - outPaths.insert(*output.second); + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + if (!outputHashes.count(output.first)) + throw Error( + "The derivation %s doesn't have an output " + "named %s", + store->printStorePath(bfd.drvPath), + output.first); + auto outputId = DrvOutput{outputHashes.at(output.first), output.first}; + auto realisation = store->queryRealisation(outputId); + if (!realisation) + throw Error("Cannot operate on output of unbuilt CA drv %s", outputId.to_string()); + res.insert(RealisedPath{*realisation}); + } + else { + // If ca-derivations isn't enabled, behave as if + // all the paths are opaque to keep the default + // behavior + assert(output.second); + res.insert(*output.second); + } } }, }, b); @@ -731,9 +751,19 @@ StorePathSet toStorePaths(ref store, for (auto & i : installables) for (auto & b : i->toBuildables()) if (auto bfd = std::get_if(&b)) - outPaths.insert(bfd->drvPath); + res.insert(bfd->drvPath); } + return res; +} + +StorePathSet toStorePaths(ref store, + Realise mode, OperateOn operateOn, + std::vector> installables) +{ + StorePathSet outPaths; + for (auto & path : toRealisedPaths(store, mode, operateOn, installables)) + outPaths.insert(path.path()); return outPaths; } diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 47ad90eee..c9b66186f 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -46,4 +46,35 @@ Realisation Realisation::fromJSON( }; } +StorePath RealisedPath::path() const { + return visit([](auto && arg) { return arg.getPath(); }); +} + +void RealisedPath::closure( + Store& store, + const RealisedPath::Set& startPaths, + RealisedPath::Set& ret) +{ + // FIXME: This only builds the store-path closure, not the real realisation + // closure + StorePathSet initialStorePaths, pathsClosure; + for (auto& path : startPaths) + initialStorePaths.insert(path.path()); + store.computeFSClosure(initialStorePaths, pathsClosure); + ret.insert(startPaths.begin(), startPaths.end()); + ret.insert(pathsClosure.begin(), pathsClosure.end()); +} + +void RealisedPath::closure(Store& store, RealisedPath::Set& ret) const +{ + RealisedPath::closure(store, {*this}, ret); +} + +RealisedPath::Set RealisedPath::closure(Store& store) const +{ + RealisedPath::Set ret; + closure(store, ret); + return ret; +} + } // namespace nix diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 4b8ead3c5..1ecddc4d1 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -3,6 +3,34 @@ #include "path.hh" #include + +/* Awfull hacky generation of the comparison operators by doing a lexicographic + * comparison between the choosen fields + * ``` + * GENERATE_CMP(ClassName, my->field1, my->field2, ...) + * ``` + * + * will generate comparison operators semantically equivalent to: + * ``` + * bool operator<(const ClassName& other) { + * return field1 < other.field1 && field2 < other.field2 && ...; + * } + * ``` + */ +#define GENERATE_ONE_CMP(COMPARATOR, MY_TYPE, FIELDS...) \ + bool operator COMPARATOR(const MY_TYPE& other) const { \ + const MY_TYPE* me = this; \ + auto fields1 = std::make_tuple( FIELDS ); \ + me = &other; \ + auto fields2 = std::make_tuple( FIELDS ); \ + return fields1 COMPARATOR fields2; \ + } +#define GENERATE_EQUAL(args...) GENERATE_ONE_CMP(==, args) +#define GENERATE_LEQ(args...) GENERATE_ONE_CMP(<, args) +#define GENERATE_CMP(args...) \ + GENERATE_EQUAL(args) \ + GENERATE_LEQ(args) + namespace nix { struct DrvOutput { @@ -17,13 +45,7 @@ struct DrvOutput { static DrvOutput parse(const std::string &); - bool operator<(const DrvOutput& other) const { return to_pair() < other.to_pair(); } - bool operator==(const DrvOutput& other) const { return to_pair() == other.to_pair(); } - -private: - // Just to make comparison operators easier to write - std::pair to_pair() const - { return std::make_pair(drvHash, outputName); } + GENERATE_CMP(DrvOutput, me->drvHash, me->outputName); }; struct Realisation { @@ -32,8 +54,60 @@ struct Realisation { nlohmann::json toJSON() const; static Realisation fromJSON(const nlohmann::json& json, const std::string& whence); + + StorePath getPath() const { return outPath; } + + GENERATE_CMP(Realisation, me->id, me->outPath); }; -typedef std::map DrvOutputs; +struct OpaquePath { + StorePath path; + + StorePath getPath() const { return path; } + + GENERATE_CMP(OpaquePath, me->path); +}; + + +/** + * A store path with all the history of how it went into the store + */ +struct RealisedPath { + /* + * A path is either the result of the realisation of a derivation or + * an opaque blob that has been directly added to the store + */ + using Raw = std::variant; + Raw raw; + + using Set = std::set; + + RealisedPath(StorePath path) : raw(OpaquePath{path}) {} + RealisedPath(Realisation r) : raw(r) {} + + /** + * Syntactic sugar to run `std::visit` on the raw value: + * path.visit(blah) == std::visit(blah, path.raw) + */ + template + constexpr decltype(auto) visit(Visitor && vis) { + return std::visit(vis, raw); + } + template + constexpr decltype(auto) visit(Visitor && vis) const { + return std::visit(vis, raw); + } + + /** + * Get the raw store path associated to this + */ + StorePath path() const; + + void closure(Store& store, Set& ret) const; + static void closure(Store& store, const Set& startPaths, Set& ret); + Set closure(Store& store) const; + + GENERATE_CMP(RealisedPath, me->raw); +}; } diff --git a/src/nix/copy.cc b/src/nix/copy.cc index f15031a45..c56a1def1 100644 --- a/src/nix/copy.cc +++ b/src/nix/copy.cc @@ -16,6 +16,8 @@ struct CmdCopy : StorePathsCommand SubstituteFlag substitute = NoSubstitute; + using StorePathsCommand::run; + CmdCopy() : StorePathsCommand(true) {