Merge pull request #10 from NixLayeredStore/delete

Fix and test deleting when lower store references

src/libstore/gc.cc

@@ -741,7 +741,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
                 auto i = referrersCache.find(*path);
                 if (i == referrersCache.end()) {
                     StorePathSet referrers;
-                    queryReferrers(*path, referrers);
+                    queryGCReferrers(*path, referrers);
                     referrersCache.emplace(*path, std::move(referrers));
                     i = referrersCache.find(*path);
                 }

src/libstore/local-overlay-store.cc

@@ -138,6 +138,12 @@ void LocalOverlayStore::queryReferrers(const StorePath & path, StorePathSet & re
 }
 
 
+void LocalOverlayStore::queryGCReferrers(const StorePath & path, StorePathSet & referrers)
+{
+    LocalStore::queryReferrers(path, referrers);
+}
+
+
 StorePathSet LocalOverlayStore::queryValidDerivers(const StorePath & path)
 {
     auto res = LocalStore::queryValidDerivers(path);

src/libstore/local-overlay-store.hh

@@ -124,8 +124,17 @@ private:
 
     void optimiseStore() override;
 
+    /**
+     * For lower-store paths, we used the lower store location. This avoids the
+     * wasteful "copying up" that would otherwise happen.
+     */
     Path toRealPathForHardLink(const StorePath & storePath) override;
 
+    /**
+     * Deletion only effects the upper layer, so we ignore lower-layer referrers.
+     */
+    void queryGCReferrers(const StorePath & path, StorePathSet & referrers) override;
+
     void remountIfNecessary();
 
     std::atomic_bool _remountRequired = false;

src/libstore/local-store.hh

@@ -230,6 +230,18 @@ public:
 
     void collectGarbage(const GCOptions & options, GCResults & results) override;
 
+    /**
+     * Called by `collectGarbage` to trace in reverse.
+     *
+     * Using this rather than `queryReferrers` directly allows us to
+     * fine-tune which referrers we consider for garbage collection;
+     * some store implementations take advantage of this.
+     */
+    virtual void queryGCReferrers(const StorePath & path, StorePathSet & referrers)
+    {
+        return queryReferrers(path, referrers);
+    }
+
     /**
      * Called by `collectGarbage` to recursively delete a path.
      * The default implementation simply calls `deletePath`, but it can be

tests/hermetic.nix

@@ -37,7 +37,7 @@ let
     buildCommand = ''
       echo hi-input3
       read x < ${input2}
-      echo $x BAZ > $out
+      echo ${input2} $x BAZ > $out
     '';
   };
 
@@ -51,6 +51,6 @@ in
       ''
         read x < ${input1}
         read y < ${input3}
-        echo "$x $y" > $out
+        echo ${input1} ${input3} "$x $y" > $out
       '';
   }

tests/overlay-local-store/delete-refs-inner.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env bash
+
+set -eu -o pipefail
+
+source common.sh
+
+# Avoid store dir being inside sandbox build-dir
+unset NIX_STORE_DIR
+unset NIX_STATE_DIR
+
+storeDirs
+
+initLowerStore
+
+mountOverlayfs
+
+export NIX_REMOTE="$storeB"
+stateB="$storeBRoot/nix/var/nix"
+hermetic=$(nix-build ../hermetic.nix --no-out-link --arg busybox "$busybox" --arg seed 2)
+input1=$(nix-build ../hermetic.nix --no-out-link --arg busybox "$busybox" --arg seed 2 -A passthru.input1 -j0)
+input2=$(nix-build ../hermetic.nix --no-out-link --arg busybox "$busybox" --arg seed 2 -A passthru.input2 -j0)
+input3=$(nix-build ../hermetic.nix --no-out-link --arg busybox "$busybox" --arg seed 2 -A passthru.input3 -j0)
+
+# Can't delete because referenced
+expectStderr 1 nix-store --delete $input1 | grepQuiet "Cannot delete path"
+expectStderr 1 nix-store --delete $input2 | grepQuiet "Cannot delete path"
+expectStderr 1 nix-store --delete $input3 | grepQuiet "Cannot delete path"
+
+# These same paths are referenced in the lower layer (by the seed 1
+# build done in `initLowerStore`).
+expectStderr 1 nix-store --store "$storeA" --delete $input2 | grepQuiet "Cannot delete path"
+expectStderr 1 nix-store --store "$storeA" --delete $input3 | grepQuiet "Cannot delete path"
+
+# Can delete
+nix-store --delete $hermetic
+
+# Now unreferenced in upper layer, can delete
+nix-store --delete $input3
+nix-store --delete $input2

tests/overlay-local-store/delete-refs.sh

@@ -0,0 +1,5 @@
+source common.sh
+
+requireEnvironment
+setupConfig
+execUnshare ./delete-refs-inner.sh

tests/overlay-local-store/local.mk

@@ -4,9 +4,10 @@ overlay-local-store-tests := \
   $(d)/build.sh \
   $(d)/bad-uris.sh \
   $(d)/add-lower.sh \
+  $(d)/delete-refs.sh \
+  $(d)/delete-duplicate.sh \
   $(d)/gc.sh \
   $(d)/verify.sh \
-  $(d)/optimise.sh \
+  $(d)/optimise.sh
-  $(d)/delete-duplicate.sh
 
 install-tests-groups += overlay-local-store