From 6636202356b94ca4128462493770e7fedf997b0e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 1 Mar 2022 18:31:36 +0000 Subject: [PATCH] Factor out a `GcStore` interface Starts progress on #5729. The idea is that we should not have these default methods throwing "unimplemented". This is a small step in that direction. I kept `addTempRoot` because it is a no-op, rather than failure. Also, as a practical matter, it is called all over the place, while doing other tasks, so the downcasting would be annoying. Maybe in the future I could move the "real" `addTempRoot` to `GcStore`, and the existing usecases use a `tryAddTempRoot` wrapper to downcast or do nothing, but I wasn't sure whether that was a good idea so with a bias to less churn I didn't do it yet. --- src/libmain/shared.cc | 1 + src/libstore/build/local-derivation-goal.cc | 3 +- src/libstore/daemon.cc | 12 ++- src/libstore/gc-store.cc | 13 +++ src/libstore/gc-store.hh | 84 +++++++++++++++++++ src/libstore/local-fs-store.hh | 3 +- src/libstore/local-store.hh | 3 +- src/libstore/path-with-outputs.cc | 6 +- src/libstore/remote-store.cc | 1 + src/libstore/remote-store.hh | 3 +- src/libstore/store-api.hh | 73 ---------------- src/libutil/tests/logging.cc | 2 +- .../nix-collect-garbage.cc | 4 +- src/nix-store/nix-store.cc | 18 ++-- src/nix/store-delete.cc | 5 +- src/nix/store-gc.cc | 5 +- 16 files changed, 143 insertions(+), 93 deletions(-) create mode 100644 src/libstore/gc-store.cc create mode 100644 src/libstore/gc-store.hh diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index a0b0f4cb3..562d1b414 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -1,6 +1,7 @@ #include "globals.hh" #include "shared.hh" #include "store-api.hh" +#include "gc-store.hh" #include "util.hh" #include "loggers.hh" diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 7e69d4145..581d63d0e 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1,4 +1,5 @@ #include "local-derivation-goal.hh" +#include "gc-store.hh" #include "hook-instance.hh" #include "worker.hh" #include "builtins.hh" @@ -1127,7 +1128,7 @@ struct RestrictedStoreConfig : virtual LocalFSStoreConfig /* A wrapper around LocalStore that only allows building/querying of paths that are in the input closures of the build or were added via recursive Nix calls. */ -struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore +struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore, public virtual GcStore { ref next; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 9d4f6b4a4..89d9487da 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -3,6 +3,7 @@ #include "worker-protocol.hh" #include "build-result.hh" #include "store-api.hh" +#include "gc-store.hh" #include "path-with-outputs.hh" #include "finally.hh" #include "archive.hh" @@ -623,9 +624,12 @@ static void performOp(TunnelLogger * logger, ref store, case wopAddIndirectRoot: { Path path = absPath(readString(from)); + logger->startWork(); - store->addIndirectRoot(path); + auto & gcStore = requireGcStore(*store); + gcStore.addIndirectRoot(path); logger->stopWork(); + to << 1; break; } @@ -640,7 +644,8 @@ static void performOp(TunnelLogger * logger, ref store, case wopFindRoots: { logger->startWork(); - Roots roots = store->findRoots(!trusted); + auto & gcStore = requireGcStore(*store); + Roots roots = gcStore.findRoots(!trusted); logger->stopWork(); size_t size = 0; @@ -671,7 +676,8 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); if (options.ignoreLiveness) throw Error("you are not allowed to ignore liveness"); - store->collectGarbage(options, results); + auto & gcStore = requireGcStore(*store); + gcStore.collectGarbage(options, results); logger->stopWork(); to << results.paths << results.bytesFreed << 0 /* obsolete */; diff --git a/src/libstore/gc-store.cc b/src/libstore/gc-store.cc new file mode 100644 index 000000000..3dbdec53b --- /dev/null +++ b/src/libstore/gc-store.cc @@ -0,0 +1,13 @@ +#include "gc-store.hh" + +namespace nix { + +GcStore & requireGcStore(Store & store) +{ + auto * gcStore = dynamic_cast(&store); + if (!gcStore) + throw UsageError("Garbage collection not supported by this store"); + return *gcStore; +} + +} diff --git a/src/libstore/gc-store.hh b/src/libstore/gc-store.hh new file mode 100644 index 000000000..829f70dc4 --- /dev/null +++ b/src/libstore/gc-store.hh @@ -0,0 +1,84 @@ +#pragma once + +#include "store-api.hh" + + +namespace nix { + + +typedef std::unordered_map> Roots; + + +struct GCOptions +{ + /* Garbage collector operation: + + - `gcReturnLive': return the set of paths reachable from + (i.e. in the closure of) the roots. + + - `gcReturnDead': return the set of paths not reachable from + the roots. + + - `gcDeleteDead': actually delete the latter set. + + - `gcDeleteSpecific': delete the paths listed in + `pathsToDelete', insofar as they are not reachable. + */ + typedef enum { + gcReturnLive, + gcReturnDead, + gcDeleteDead, + gcDeleteSpecific, + } GCAction; + + GCAction action{gcDeleteDead}; + + /* If `ignoreLiveness' is set, then reachability from the roots is + ignored (dangerous!). However, the paths must still be + unreferenced *within* the store (i.e., there can be no other + store paths that depend on them). */ + bool ignoreLiveness{false}; + + /* For `gcDeleteSpecific', the paths to delete. */ + StorePathSet pathsToDelete; + + /* Stop after at least `maxFreed' bytes have been freed. */ + uint64_t maxFreed{std::numeric_limits::max()}; +}; + + +struct GCResults +{ + /* Depending on the action, the GC roots, or the paths that would + be or have been deleted. */ + PathSet paths; + + /* For `gcReturnDead', `gcDeleteDead' and `gcDeleteSpecific', the + number of bytes that would be or was freed. */ + uint64_t bytesFreed = 0; +}; + + +struct GcStore : public virtual Store +{ + /* Add an indirect root, which is merely a symlink to `path' from + /nix/var/nix/gcroots/auto/. `path' is supposed + to be a symlink to a store path. The garbage collector will + automatically remove the indirect root when it finds that + `path' has disappeared. */ + virtual void addIndirectRoot(const Path & path) = 0; + + /* Find the roots of the garbage collector. Each root is a pair + (link, storepath) where `link' is the path of the symlink + outside of the Nix store that point to `storePath'. If + 'censor' is true, privacy-sensitive information about roots + found in /proc is censored. */ + virtual Roots findRoots(bool censor) = 0; + + /* Perform a garbage collection. */ + virtual void collectGarbage(const GCOptions & options, GCResults & results) = 0; +}; + +GcStore & requireGcStore(Store & store); + +} diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh index d34f0cb62..fbd49dc2c 100644 --- a/src/libstore/local-fs-store.hh +++ b/src/libstore/local-fs-store.hh @@ -1,6 +1,7 @@ #pragma once #include "store-api.hh" +#include "gc-store.hh" namespace nix { @@ -23,7 +24,7 @@ struct LocalFSStoreConfig : virtual StoreConfig "physical path to the Nix store"}; }; -class LocalFSStore : public virtual LocalFSStoreConfig, public virtual Store +class LocalFSStore : public virtual LocalFSStoreConfig, public virtual Store, virtual GcStore { public: diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 1a278c9a8..70d225be3 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -5,6 +5,7 @@ #include "pathlocks.hh" #include "store-api.hh" #include "local-fs-store.hh" +#include "gc-store.hh" #include "sync.hh" #include "util.hh" @@ -43,7 +44,7 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig }; -class LocalStore : public virtual LocalStoreConfig, public virtual LocalFSStore +class LocalStore : public virtual LocalStoreConfig, public virtual LocalFSStore, public virtual GcStore { private: diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index 97aa01b57..078c117bd 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -22,9 +22,9 @@ DerivedPath StorePathWithOutputs::toDerivedPath() const std::vector toDerivedPaths(const std::vector ss) { - std::vector reqs; - for (auto & s : ss) reqs.push_back(s.toDerivedPath()); - return reqs; + std::vector reqs; + for (auto & s : ss) reqs.push_back(s.toDerivedPath()); + return reqs; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index cbcb75c00..a25398ef2 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -1,6 +1,7 @@ #include "serialise.hh" #include "util.hh" #include "path-with-outputs.hh" +#include "gc-store.hh" #include "remote-fs-accessor.hh" #include "build-result.hh" #include "remote-store.hh" diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index b94216d31..2628206b1 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -4,6 +4,7 @@ #include #include "store-api.hh" +#include "gc-store.hh" namespace nix { @@ -29,7 +30,7 @@ struct RemoteStoreConfig : virtual StoreConfig /* FIXME: RemoteStore is a misnomer - should be something like DaemonStore. */ -class RemoteStore : public virtual RemoteStoreConfig, public virtual Store +class RemoteStore : public virtual RemoteStoreConfig, public virtual Store, public virtual GcStore { public: diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 8c57596d0..7bd21519c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -76,59 +76,6 @@ enum AllowInvalidFlag : bool { DisallowInvalid = false, AllowInvalid = true }; const uint32_t exportMagic = 0x4558494e; -typedef std::unordered_map> Roots; - - -struct GCOptions -{ - /* Garbage collector operation: - - - `gcReturnLive': return the set of paths reachable from - (i.e. in the closure of) the roots. - - - `gcReturnDead': return the set of paths not reachable from - the roots. - - - `gcDeleteDead': actually delete the latter set. - - - `gcDeleteSpecific': delete the paths listed in - `pathsToDelete', insofar as they are not reachable. - */ - typedef enum { - gcReturnLive, - gcReturnDead, - gcDeleteDead, - gcDeleteSpecific, - } GCAction; - - GCAction action{gcDeleteDead}; - - /* If `ignoreLiveness' is set, then reachability from the roots is - ignored (dangerous!). However, the paths must still be - unreferenced *within* the store (i.e., there can be no other - store paths that depend on them). */ - bool ignoreLiveness{false}; - - /* For `gcDeleteSpecific', the paths to delete. */ - StorePathSet pathsToDelete; - - /* Stop after at least `maxFreed' bytes have been freed. */ - uint64_t maxFreed{std::numeric_limits::max()}; -}; - - -struct GCResults -{ - /* Depending on the action, the GC roots, or the paths that would - be or have been deleted. */ - PathSet paths; - - /* For `gcReturnDead', `gcDeleteDead' and `gcDeleteSpecific', the - number of bytes that would be or was freed. */ - uint64_t bytesFreed = 0; -}; - - enum BuildMode { bmNormal, bmRepair, bmCheck }; struct BuildResult; @@ -531,26 +478,6 @@ public: virtual void addTempRoot(const StorePath & path) { debug("not creating temporary root, store doesn't support GC"); } - /* Add an indirect root, which is merely a symlink to `path' from - /nix/var/nix/gcroots/auto/. `path' is supposed - to be a symlink to a store path. The garbage collector will - automatically remove the indirect root when it finds that - `path' has disappeared. */ - virtual void addIndirectRoot(const Path & path) - { unsupported("addIndirectRoot"); } - - /* Find the roots of the garbage collector. Each root is a pair - (link, storepath) where `link' is the path of the symlink - outside of the Nix store that point to `storePath'. If - 'censor' is true, privacy-sensitive information about roots - found in /proc is censored. */ - virtual Roots findRoots(bool censor) - { unsupported("findRoots"); } - - /* Perform a garbage collection. */ - virtual void collectGarbage(const GCOptions & options, GCResults & results) - { unsupported("collectGarbage"); } - /* Return a string representing information about the path that can be loaded into the database using `nix-store --load-db' or `nix-store --register-validity'. */ diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index cef3bd481..2ffdc2e9b 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -359,7 +359,7 @@ namespace nix { // constructing without access violation. ErrPos ep(invalid); - + // assignment without access violation. ep = invalid; diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc index 48c030b18..4b28ea6a4 100644 --- a/src/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix-collect-garbage/nix-collect-garbage.cc @@ -1,4 +1,5 @@ #include "store-api.hh" +#include "gc-store.hh" #include "profiles.hh" #include "shared.hh" #include "globals.hh" @@ -80,10 +81,11 @@ static int main_nix_collect_garbage(int argc, char * * argv) // Run the actual garbage collector. if (!dryRun) { auto store = openStore(); + auto & gcStore = requireGcStore(*store); options.action = GCOptions::gcDeleteDead; GCResults results; PrintFreed freed(true, results); - store->collectGarbage(options, results); + gcStore.collectGarbage(options, results); } return 0; diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 1ebc177f5..8ebaf9387 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -3,6 +3,7 @@ #include "dotgraph.hh" #include "globals.hh" #include "build-result.hh" +#include "gc-store.hh" #include "local-store.hh" #include "monitor-fd.hh" #include "serve-protocol.hh" @@ -428,11 +429,12 @@ static void opQuery(Strings opFlags, Strings opArgs) store->computeFSClosure( args, referrers, true, settings.gcKeepOutputs, settings.gcKeepDerivations); - Roots roots = store->findRoots(false); + auto & gcStore = requireGcStore(*store); + Roots roots = gcStore.findRoots(false); for (auto & [target, links] : roots) if (referrers.find(target) != referrers.end()) for (auto & link : links) - cout << fmt("%1% -> %2%\n", link, store->printStorePath(target)); + cout << fmt("%1% -> %2%\n", link, gcStore.printStorePath(target)); break; } @@ -588,20 +590,22 @@ static void opGC(Strings opFlags, Strings opArgs) if (!opArgs.empty()) throw UsageError("no arguments expected"); + auto & gcStore = requireGcStore(*store); + if (printRoots) { - Roots roots = store->findRoots(false); + Roots roots = gcStore.findRoots(false); std::set> roots2; // Transpose and sort the roots. for (auto & [target, links] : roots) for (auto & link : links) roots2.emplace(link, target); for (auto & [link, target] : roots2) - std::cout << link << " -> " << store->printStorePath(target) << "\n"; + std::cout << link << " -> " << gcStore.printStorePath(target) << "\n"; } else { PrintFreed freed(options.action == GCOptions::gcDeleteDead, results); - store->collectGarbage(options, results); + gcStore.collectGarbage(options, results); if (options.action != GCOptions::gcDeleteDead) for (auto & i : results.paths) @@ -625,9 +629,11 @@ static void opDelete(Strings opFlags, Strings opArgs) for (auto & i : opArgs) options.pathsToDelete.insert(store->followLinksToStorePath(i)); + auto & gcStore = requireGcStore(*store); + GCResults results; PrintFreed freed(true, results); - store->collectGarbage(options, results); + gcStore.collectGarbage(options, results); } diff --git a/src/nix/store-delete.cc b/src/nix/store-delete.cc index e4a3cb554..aa7a8b12f 100644 --- a/src/nix/store-delete.cc +++ b/src/nix/store-delete.cc @@ -2,6 +2,7 @@ #include "common-args.hh" #include "shared.hh" #include "store-api.hh" +#include "gc-store.hh" using namespace nix; @@ -32,12 +33,14 @@ struct CmdStoreDelete : StorePathsCommand void run(ref store, std::vector && storePaths) override { + auto & gcStore = requireGcStore(*store); + for (auto & path : storePaths) options.pathsToDelete.insert(path); GCResults results; PrintFreed freed(true, results); - store->collectGarbage(options, results); + gcStore.collectGarbage(options, results); } }; diff --git a/src/nix/store-gc.cc b/src/nix/store-gc.cc index a2d74066e..21718dc0c 100644 --- a/src/nix/store-gc.cc +++ b/src/nix/store-gc.cc @@ -2,6 +2,7 @@ #include "common-args.hh" #include "shared.hh" #include "store-api.hh" +#include "gc-store.hh" using namespace nix; @@ -33,10 +34,12 @@ struct CmdStoreGC : StoreCommand, MixDryRun void run(ref store) override { + auto & gcStore = requireGcStore(*store); + options.action = dryRun ? GCOptions::gcReturnDead : GCOptions::gcDeleteDead; GCResults results; PrintFreed freed(options.action == GCOptions::gcDeleteDead, results); - store->collectGarbage(options, results); + gcStore.collectGarbage(options, results); } };