From 0008b0006db59ea8fccfe041cf8d87f05abb427d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Dec 2008 17:03:18 +0000 Subject: [PATCH] * Simplify deleting .lock files in /nix/store: just don't delete them if they belong a path that's currently being built. This gets rid of some Cygwin-specific code. --- src/libstore/gc.cc | 48 ++++++++++++++------------------------ src/nix-store/nix-store.cc | 2 +- tests/Makefile.am | 2 +- tests/gc-concurrent.sh | 20 ++++++++++++++-- 4 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 3621b88d9..32cfbd022 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -476,23 +476,6 @@ void LocalStore::gcPath(const GCOptions & options, GCResults & results, if (!pathExists(path)) return; -#ifndef __CYGWIN__ - AutoCloseFD fdLock; - - /* Only delete a lock file if we can acquire a write lock on it. - That means that it's either stale, or the process that created - it hasn't locked it yet. In the latter case the other process - will detect that we deleted the lock, and retry (see - pathlocks.cc). */ - if (path.size() >= 5 && string(path, path.size() - 5) == ".lock") { - fdLock = openLockFile(path, false); - if (fdLock != -1 && !lockFile(fdLock, ltWrite, false)) { - debug(format("skipping active lock `%1%'") % path); - return; - } - } -#endif - /* Okay, it's safe to delete. */ unsigned long long bytesFreed, blocksFreed; deleteFromStore(path, bytesFreed, blocksFreed); @@ -513,12 +496,6 @@ void LocalStore::gcPath(const GCOptions & options, GCResults & results, throw GCLimitReached(); } } - -#ifndef __CYGWIN__ - if (fdLock != -1) - /* Write token to stale (deleted) lock file. */ - writeFull(fdLock, (const unsigned char *) "d", 1); -#endif } @@ -569,7 +546,7 @@ struct CachingAtimeComparator : public std::binary_function }; -string showTime(const string & format, time_t t) +static string showTime(const string & format, time_t t) { char s[128]; strftime(s, sizeof s, format.c_str(), localtime(&t)); @@ -577,6 +554,21 @@ string showTime(const string & format, time_t t) } +static bool isLive(const Path & path, const PathSet & livePaths, + const PathSet & tempRoots, const PathSet & tempRootsClosed) +{ + if (livePaths.find(path) != livePaths.end() || + tempRootsClosed.find(path) != tempRootsClosed.end()) return true; + + /* A lock file belonging to a path that we're building right + now isn't garbage. */ + if (hasSuffix(path, ".lock") && tempRoots.find(string(path, 0, path.size() - 5)) != tempRoots.end()) + return true; + + return false; +} + + void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) { bool gcKeepOutputs = @@ -691,9 +683,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) Paths entries = readDirectory(nixStore); foreach (Paths::iterator, i, entries) { Path path = canonPath(nixStore + "/" + *i); - if (livePaths.find(path) == livePaths.end() && - tempRootsClosed.find(path) == tempRootsClosed.end()) - storePaths.insert(path); + if (!isLive(path, livePaths, tempRoots, tempRootsClosed)) storePaths.insert(path); } } @@ -701,10 +691,8 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) foreach (PathSet::iterator, i, options.pathsToDelete) { assertStorePath(*i); storePaths.insert(*i); - if (livePaths.find(*i) != livePaths.end()) + if (isLive(*i, livePaths, tempRoots, tempRootsClosed)) throw Error(format("cannot delete path `%1%' since it is still alive") % *i); - if (tempRootsClosed.find(*i) != tempRootsClosed.end()) - throw Error(format("cannot delete path `%1%' since it is temporarily in use") % *i); } } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 065d7d4c0..f6271d56a 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -96,7 +96,7 @@ static void opRealise(Strings opFlags, Strings opArgs) if (isDerivation(*i)) drvPaths.insert(*i); store->buildDerivations(drvPaths); - foreach (Strings::iterator, i,opArgs) + foreach (Strings::iterator, i, opArgs) cout << format("%1%\n") % realisePath(*i); } diff --git a/tests/Makefile.am b/tests/Makefile.am index c65f68584..ab6ac426a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -7,7 +7,7 @@ dependencies.sh gc.sh nix-push.sh nix-pull.in logging.sh nix-build.sh install-pa locking.sh: locking.nix parallel.sh: parallel.nix build-hook.sh: build-hook.nix -gc-concurrent.sh: gc-concurrent.nix gc-concurrent2.nix +gc-concurrent.sh: gc-concurrent.nix gc-concurrent2.nix simple.nix user-envs.sh: user-envs.nix fixed.sh: fixed.nix gc-runtime.sh: gc-runtime.nix diff --git a/tests/gc-concurrent.sh b/tests/gc-concurrent.sh index 6cf652078..fbc949720 100644 --- a/tests/gc-concurrent.sh +++ b/tests/gc-concurrent.sh @@ -8,8 +8,15 @@ outPath1=$($nixstore -q $drvPath1) drvPath2=$($nixinstantiate gc-concurrent2.nix) outPath2=$($nixstore -q $drvPath2) -rm -f "$NIX_STATE_DIR"/gcroots/foo +drvPath3=$($nixinstantiate simple.nix) +outPath3=$($nixstore -r $drvPath3) + +! test -e $outPath3.lock +touch $outPath3.lock + +rm -f "$NIX_STATE_DIR"/gcroots/foo* ln -s $drvPath2 "$NIX_STATE_DIR"/gcroots/foo +ln -s $outPath3 "$NIX_STATE_DIR"/gcroots/foo2 # Start build #1 in the background. It starts immediately. $nixstore -rvv "$drvPath1" & @@ -39,4 +46,13 @@ cat $outPath1/input-2/bar # derivation is a GC root. cat $outPath2/foobar -rm "$NIX_STATE_DIR"/gcroots/foo +rm -f "$NIX_STATE_DIR"/gcroots/foo* + +# The collector should have deleted lock files for paths that have +# been built previously. +! test -e $outPath3.lock + +# If we run the collector now, it should delete outPath1/2. +$NIX_BIN_DIR/nix-collect-garbage -vvvvv +! test -e $outPath1 +! test -e $outPath2