From 152b1d6bf9c89b4db9848475e3000821e159d479 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 24 Feb 2016 17:44:12 +0100 Subject: deletePath(): Succeed if path doesn't exist Also makes it robust against concurrent deletions. --- src/libstore/build.cc | 22 +++++++++------------- src/libstore/gc.cc | 2 +- src/libstore/local-store.cc | 6 +++--- src/libutil/util.cc | 12 ++++++++---- src/libutil/util.hh | 4 ++-- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 249ab2335b..547981e5bb 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1310,7 +1310,6 @@ void DerivationGoal::tryToBuild() for (auto & i : drv->outputs) { Path path = i.second.path; if (worker.store.isValidPath(path)) continue; - if (!pathExists(path)) continue; debug(format("removing invalid path ‘%1%’") % path); deletePath(path); } @@ -1390,8 +1389,7 @@ void replaceValidPath(const Path & storePath, const Path tmpPath) rename(storePath.c_str(), oldPath.c_str()); if (rename(tmpPath.c_str(), storePath.c_str()) == -1) throw SysError(format("moving ‘%1%’ to ‘%2%’") % tmpPath % storePath); - if (pathExists(oldPath)) - deletePath(oldPath); + deletePath(oldPath); } @@ -1490,7 +1488,7 @@ void DerivationGoal::buildDone() /* Delete unused redirected outputs (when doing hash rewriting). */ for (auto & i : redirectedOutputs) - if (pathExists(i.second)) deletePath(i.second); + deletePath(i.second); /* Delete the chroot (if we were using one). */ autoDelChroot.reset(); /* this runs the destructor */ @@ -1939,7 +1937,7 @@ void DerivationGoal::startBuilder() to ensure that we can create hard-links to non-directory inputs in the fake Nix store in the chroot (see below). */ chrootRootDir = drvPath + ".chroot"; - if (pathExists(chrootRootDir)) deletePath(chrootRootDir); + deletePath(chrootRootDir); /* Clean up the chroot directory automatically. */ autoDelChroot = std::make_shared(chrootRootDir); @@ -2514,7 +2512,7 @@ void DerivationGoal::runChild() debug(sandboxProfile); Path sandboxFile = drvPath + ".sb"; - if (pathExists(sandboxFile)) deletePath(sandboxFile); + deletePath(sandboxFile); autoDelSandbox.reset(sandboxFile, false); writeFile(sandboxFile, sandboxProfile); @@ -2706,8 +2704,7 @@ void DerivationGoal::registerOutputs() return; if (actualPath != dest) { PathLocks outputLocks({dest}); - if (pathExists(dest)) - deletePath(dest); + deletePath(dest); if (rename(actualPath.c_str(), dest.c_str()) == -1) throw SysError(format("moving ‘%1%’ to ‘%2%’") % actualPath % dest); } @@ -2738,7 +2735,7 @@ void DerivationGoal::registerOutputs() if (hash.first != info.narHash) { if (settings.keepFailed) { Path dst = path + checkSuffix; - if (pathExists(dst)) deletePath(dst); + deletePath(dst); if (rename(actualPath.c_str(), dst.c_str())) throw SysError(format("renaming ‘%1%’ to ‘%2%’") % actualPath % dst); throw Error(format("derivation ‘%1%’ may not be deterministic: output ‘%2%’ differs from ‘%3%’") @@ -2830,7 +2827,7 @@ void DerivationGoal::registerOutputs() if (settings.keepFailed) { for (auto & i : drv->outputs) { Path prev = i.second.path + checkSuffix; - if (pathExists(prev)) deletePath(prev); + deletePath(prev); if (curRound < nrRounds) { Path dst = i.second.path + checkSuffix; if (rename(i.second.path.c_str(), dst.c_str())) @@ -2998,7 +2995,7 @@ Path DerivationGoal::addHashRewrite(const Path & path) string h1 = string(path, settings.nixStore.size() + 1, 32); string h2 = string(printHash32(hashString(htSHA256, "rewrite:" + drvPath + ":" + path)), 0, 32); Path p = settings.nixStore + "/" + h2 + string(path, settings.nixStore.size() + 33); - if (pathExists(p)) deletePath(p); + deletePath(p); assert(path.size() == p.size()); rewritesToTmp[h1] = h2; rewritesFromTmp[h2] = h1; @@ -3259,8 +3256,7 @@ void SubstitutionGoal::tryToRun() destPath = repair ? storePath + ".tmp" : storePath; /* Remove the (stale) output path if it exists. */ - if (pathExists(destPath)) - deletePath(destPath); + deletePath(destPath); worker.store.setSubstituterEnv(); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 1bc8d162f8..e082f67143 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -608,7 +608,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) state.shouldDelete = options.action == GCOptions::gcDeleteDead || options.action == GCOptions::gcDeleteSpecific; - if (state.shouldDelete && pathExists(reservedPath)) + if (state.shouldDelete) deletePath(reservedPath); /* Acquire the global GC root. This prevents diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index da4d932df6..33f2569128 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1365,7 +1365,7 @@ Path LocalStore::addToStoreFromDump(const string & dump, const string & name, if (repair || !isValidPath(dstPath)) { - if (pathExists(dstPath)) deletePath(dstPath); + deletePath(dstPath); if (recursive) { StringSource source(dump); @@ -1434,7 +1434,7 @@ Path LocalStore::addTextToStore(const string & name, const string & s, if (repair || !isValidPath(dstPath)) { - if (pathExists(dstPath)) deletePath(dstPath); + deletePath(dstPath); writeFile(dstPath, s); @@ -1659,7 +1659,7 @@ Path LocalStore::importPath(bool requireSignature, Source & source) if (!isValidPath(dstPath)) { - if (pathExists(dstPath)) deletePath(dstPath); + deletePath(dstPath); if (rename(unpacked.c_str(), dstPath.c_str()) == -1) throw SysError(format("cannot move ‘%1%’ to ‘%2%’") diff --git a/src/libutil/util.cc b/src/libutil/util.cc index def0525abc..3becbbabc2 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -320,9 +320,11 @@ static void _deletePath(const Path & path, unsigned long long & bytesFreed) { checkInterrupt(); - printMsg(lvlVomit, format("%1%") % path); - - struct stat st = lstat(path); + struct stat st; + if (lstat(path.c_str(), &st) == -1) { + if (errno == ENOENT) return; + throw SysError(format("getting status of ‘%1%’") % path); + } if (!S_ISDIR(st.st_mode) && st.st_nlink == 1) bytesFreed += st.st_blocks * 512; @@ -338,8 +340,10 @@ static void _deletePath(const Path & path, unsigned long long & bytesFreed) _deletePath(path + "/" + i.name, bytesFreed); } - if (remove(path.c_str()) == -1) + if (remove(path.c_str()) == -1) { + if (errno == ENOENT) return; throw SysError(format("cannot unlink ‘%1%’") % path); + } } diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 9eebb67fdf..3606f6ec9e 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -92,8 +92,8 @@ string readLine(int fd); void writeLine(int fd, string s); /* Delete a path; i.e., in the case of a directory, it is deleted - recursively. Don't use this at home, kids. The second variant - returns the number of bytes and blocks freed. */ + recursively. It's not an error if the path does not exist. The + second variant returns the number of bytes and blocks freed. */ void deletePath(const Path & path); void deletePath(const Path & path, unsigned long long & bytesFreed); -- cgit 1.4.1