From 658a16b9f6fe583d969c81100ee7823c4fe7de75 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 13 Apr 2023 10:38:35 -0400 Subject: [PATCH] Support `repairPath` on most stores. More progress on issue #5729 The method trivially generalizes to be store-implementation-agnostic, in fact. However, we force it to continue to be unimplemented with `RemoteStore` and `LegacySSHStore` because the implementation we'd get via the generalization is probably not the one users expect. This keeps our hands untied to do it right going forward. For more about the tension between the scheduler logic being store-type-agnostic and remote stores doing their own scheduling, see issues #5025 and #5056. --- src/libstore/build/entry-points.cc | 2 +- src/libstore/legacy-ssh-store.cc | 11 +++++++++++ src/libstore/local-store.hh | 2 -- src/libstore/remote-store.hh | 11 +++++++++++ src/libstore/store-api.hh | 3 +-- 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 2925fe3ca4dd..efa1445faf33 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -119,7 +119,7 @@ void Store::ensurePath(const StorePath & path) } -void LocalStore::repairPath(const StorePath & path) +void Store::repairPath(const StorePath & path) { Worker worker(*this, *this); GoalPtr goal = worker.makePathSubstitutionGoal(path, Repair); diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index eb471d8fcba2..829d560623f2 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -342,6 +342,17 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor void ensurePath(const StorePath & path) override { unsupported("ensurePath"); } + /** + * The default instance would schedule the work on the client side, but + * for consistency with `buildPaths` and `buildDerivation` it should happen + * on the remote side. + * + * We make this fail for now so we can add implement this properly later + * without it being a breaking change. + */ + void repairPath(const StorePath & path) override + { unsupported("repairPath"); } + void computeFSClosure(const StorePathSet & paths, StorePathSet & out, bool flipDirection = false, bool includeOutputs = false, bool includeDerivers = false) override diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 55add18dd9f8..70debad3897d 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -240,8 +240,6 @@ public: void vacuumDB(); - void repairPath(const StorePath & path) override; - void addSignatures(const StorePath & storePath, const StringSet & sigs) override; /** diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 1c45f543e77c..0da039837e37 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -136,6 +136,17 @@ public: bool verifyStore(bool checkContents, RepairFlag repair) override; + /** + * The default instance would schedule the work on the client side, but + * for consistency with `buildPaths` and `buildDerivation` it should happen + * on the remote side. + * + * We make this fail for now so we can add implement this properly later + * without it being a breaking change. + */ + void repairPath(const StorePath & path) override + { unsupported("repairPath"); } + void addSignatures(const StorePath & storePath, const StringSet & sigs) override; void queryMissing(const std::vector & targets, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 3cb48bff5b48..67f2649a1937 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -685,8 +685,7 @@ public: * Repair the contents of the given path by redownloading it using * a substituter (if available). */ - virtual void repairPath(const StorePath & path) - { unsupported("repairPath"); } + virtual void repairPath(const StorePath & path); /** * Add signatures to the specified store path. The signatures are