Remove a possible existing store path when building CA derivations

In case a previous interrupted build left a garbage path laying around,
remove it before trying to move the path to its final location.

Fix #4858

configure.ac

@@ -65,7 +65,7 @@ AC_SYS_LARGEFILE
 AC_STRUCT_DIRENT_D_TYPE
 if test "$sys_name" = sunos; then
     # Solaris requires -lsocket -lnsl for network functions
-    LIBS="-lsocket -lnsl $LIBS"
+    LDFLAGS="-lsocket -lnsl $LDFLAGS"
 fi
 
 
@@ -150,7 +150,7 @@ int main() {
 }]])], GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=no, GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=yes)
 AC_MSG_RESULT($GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC)
 if test "x$GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC" = xyes; then
-    LIBS="-latomic $LIBS"
+    LDFLAGS="-latomic $LDFLAGS"
 fi
 
 PKG_PROG_PKG_CONFIG

src/libstore/build/local-derivation-goal.cc

@@ -292,7 +292,7 @@ bool LocalDerivationGoal::cleanupDecideWhetherDiskFull()
         auto & localStore = getLocalStore();
         uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable
         struct statvfs st;
-        if (statvfs(localStore.realStoreDir.c_str(), &st) == 0 &&
+        if (statvfs(localStore.realStoreDir.get().c_str(), &st) == 0 &&
             (uint64_t) st.f_bavail * st.f_bsize < required)
             diskFull = true;
         if (statvfs(tmpDir.c_str(), &st) == 0 &&
@@ -417,7 +417,7 @@ void LocalDerivationGoal::startBuilder()
     }
 
     auto & localStore = getLocalStore();
-    if (localStore.storeDir != localStore.realStoreDir) {
+    if (localStore.storeDir != localStore.realStoreDir.get()) {
         #if __linux__
             useChroot = true;
         #else
@@ -2487,6 +2487,7 @@ void LocalDerivationGoal::registerOutputs()
                 assert(newInfo.ca);
             } else {
                 auto destPath = worker.store.toRealPath(finalDestPath);
+                deletePath(destPath);
                 movePath(actualPath, destPath);
                 actualPath = destPath;
             }

src/libstore/gc.cc

@@ -775,7 +775,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
 
         try {
 
-            AutoCloseDir dir(opendir(realStoreDir.c_str()));
+            AutoCloseDir dir(opendir(realStoreDir.get().c_str()));
             if (!dir) throw SysError("opening directory '%1%'", realStoreDir);
 
             /* Read the store and immediately delete all paths that
@@ -856,7 +856,7 @@ void LocalStore::autoGC(bool sync)
             return std::stoll(readFile(*fakeFreeSpaceFile));
 
         struct statvfs st;
-        if (statvfs(realStoreDir.c_str(), &st))
+        if (statvfs(realStoreDir.get().c_str(), &st))
             throw SysError("getting filesystem info about '%s'", realStoreDir);
 
         return (uint64_t) st.f_bavail * st.f_frsize;

src/libstore/local-fs-store.hh

@@ -18,6 +18,9 @@ struct LocalFSStoreConfig : virtual StoreConfig
     const PathSetting logDir{(StoreConfig*) this, false,
         rootDir != "" ? rootDir + "/nix/var/log/nix" : settings.nixLogDir,
         "log", "directory where Nix will store state"};
+    const PathSetting realStoreDir{(StoreConfig*) this, false,
+        rootDir != "" ? rootDir + "/nix/store" : storeDir, "real",
+        "physical path to the Nix store"};
 };
 
 class LocalFSStore : public virtual LocalFSStoreConfig, public virtual Store
@@ -34,7 +37,7 @@ public:
     /* Register a permanent GC root. */
     Path addPermRoot(const StorePath & storePath, const Path & gcRoot);
 
-    virtual Path getRealStoreDir() { return storeDir; }
+    virtual Path getRealStoreDir() { return realStoreDir; }
 
     Path toRealPath(const Path & storePath) override
     {

src/libstore/local-store.cc

@@ -106,9 +106,6 @@ LocalStore::LocalStore(const Params & params)
     , LocalStoreConfig(params)
     , Store(params)
     , LocalFSStore(params)
-    , realStoreDir_{this, false, rootDir != "" ? rootDir + "/nix/store" : storeDir, "real",
-        "physical path to the Nix store"}
-    , realStoreDir(realStoreDir_)
     , dbDir(stateDir + "/db")
     , linksDir(realStoreDir + "/.links")
     , reservedPath(dbDir + "/reserved")
@@ -153,13 +150,13 @@ LocalStore::LocalStore(const Params & params)
             printError("warning: the group '%1%' specified in 'build-users-group' does not exist", settings.buildUsersGroup);
         else {
             struct stat st;
-            if (stat(realStoreDir.c_str(), &st))
+            if (stat(realStoreDir.get().c_str(), &st))
                 throw SysError("getting attributes of path '%1%'", realStoreDir);
 
             if (st.st_uid != 0 || st.st_gid != gr->gr_gid || (st.st_mode & ~S_IFMT) != perm) {
-                if (chown(realStoreDir.c_str(), 0, gr->gr_gid) == -1)
+                if (chown(realStoreDir.get().c_str(), 0, gr->gr_gid) == -1)
                     throw SysError("changing ownership of path '%1%'", realStoreDir);
-                if (chmod(realStoreDir.c_str(), perm) == -1)
+                if (chmod(realStoreDir.get().c_str(), perm) == -1)
                     throw SysError("changing permissions on path '%1%'", realStoreDir);
             }
         }
@@ -437,14 +434,14 @@ void LocalStore::makeStoreWritable()
     if (getuid() != 0) return;
     /* Check if /nix/store is on a read-only mount. */
     struct statvfs stat;
-    if (statvfs(realStoreDir.c_str(), &stat) != 0)
+    if (statvfs(realStoreDir.get().c_str(), &stat) != 0)
         throw SysError("getting info about the Nix store mount point");
 
     if (stat.f_flag & ST_RDONLY) {
         if (unshare(CLONE_NEWNS) == -1)
             throw SysError("setting up a private mount namespace");
 
-        if (mount(0, realStoreDir.c_str(), "none", MS_REMOUNT | MS_BIND, 0) == -1)
+        if (mount(0, realStoreDir.get().c_str(), "none", MS_REMOUNT | MS_BIND, 0) == -1)
             throw SysError("remounting %1% writable", realStoreDir);
     }
 #endif

src/libstore/local-store.hh

@@ -83,9 +83,6 @@ private:
 
 public:
 
-    PathSetting realStoreDir_;
-
-    const Path realStoreDir;
     const Path dbDir;
     const Path linksDir;
     const Path reservedPath;
@@ -279,8 +276,6 @@ private:
     void signPathInfo(ValidPathInfo & info);
     void signRealisation(Realisation &);
 
-    Path getRealStoreDir() override { return realStoreDir; }
-
     void createUser(const std::string & userName, uid_t userId) override;
 
     // XXX: Make a generic `Store` method

src/libstore/optimise-store.cc

@@ -198,7 +198,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
     /* Make the containing directory writable, but only if it's not
        the store itself (we don't want or need to mess with its
        permissions). */
-    bool mustToggle = dirOf(path) != realStoreDir;
+    bool mustToggle = dirOf(path) != realStoreDir.get();
     if (mustToggle) makeWritable(dirOf(path));
 
     /* When we're done, make the directory read-only again and reset

src/nix/develop.cc

@@ -185,6 +185,7 @@ struct Common : InstallableCommand, MixProfile
         "NIX_BUILD_TOP",
         "NIX_ENFORCE_PURITY",
         "NIX_LOG_FD",
+        "NIX_REMOTE",
         "PPID",
         "PWD",
         "SHELLOPTS",

src/nix/run.cc

@@ -43,8 +43,8 @@ struct RunCommon : virtual Command
            helper program (chrootHelper() below) to do the work. */
         auto store2 = store.dynamic_pointer_cast<LocalStore>();
 
-        if (store2 && store->storeDir != store2->realStoreDir) {
-            Strings helperArgs = { chrootHelperName, store->storeDir, store2->realStoreDir, program };
+        if (store2 && store->storeDir != store2->getRealStoreDir()) {
+            Strings helperArgs = { chrootHelperName, store->storeDir, store2->getRealStoreDir(), program };
             for (auto & arg : args) helperArgs.push_back(arg);
 
             execv(readLink("/proc/self/exe").c_str(), stringsToCharPtrs(helperArgs).data());

tests/ca/build-with-garbage-path.sh

@@ -0,0 +1,20 @@
+#!/usr/bin/env bash
+
+# Regression test for https://github.com/NixOS/nix/issues/4858
+
+source common.sh
+sed -i 's/experimental-features .*/& ca-derivations ca-references/' "$NIX_CONF_DIR"/nix.conf
+
+# Get the output path of `rootCA`, and put some garbage instead
+outPath="$(nix-build ./content-addressed.nix -A rootCA --no-out-link)"
+nix-store --delete "$outPath"
+touch "$outPath"
+
+# The build should correctly remove the garbage and put the expected path instead
+nix-build ./content-addressed.nix -A rootCA --no-out-link
+
+# Rebuild it. This shouldn’t overwrite the existing path
+oldInode=$(stat -c '%i' "$outPath")
+nix-build ./content-addressed.nix -A rootCA --no-out-link --arg seed 2
+newInode=$(stat -c '%i' "$outPath")
+[[ "$oldInode" == "$newInode" ]]

tests/local.mk

@@ -45,6 +45,7 @@ nix_tests = \
   build.sh \
   compute-levels.sh \
   ca/build.sh \
+  ca/build-with-garbage-path.sh \
   ca/substitute.sh \
   ca/signatures.sh \
   ca/nix-run.sh \