Browse Source

non-existing directories don't need to be cleaned

Trying to clean up directories which do not exist seems rather silly if
you think about it, so let apt think about it and stop it.

Depends a bit on the caller if this is fixing anything for them as they
might try to acquire a lock or doing other clever things as apt does.

Closes: 807477
David Kalnischkies 8 years ago
parent
commit
dffc17ba83

+ 5 - 0
apt-pkg/clean.cc

@@ -38,6 +38,11 @@ bool pkgArchiveCleaner::Go(std::string Dir,pkgCache &Cache)
    if(Dir == "/")
       return _error->Error(_("Clean of %s is not supported"), Dir.c_str());
 
+   // non-existing directories are always clean
+   // we do not check for a directory explicitly to support symlinks
+   if (FileExists(Dir) == false)
+      return true;
+
    DIR *D = opendir(Dir.c_str());
    if (D == 0)
       return _error->Errno("opendir",_("Unable to read %s"),Dir.c_str());

+ 18 - 8
apt-private/private-download.cc

@@ -309,12 +309,18 @@ bool DoClean(CommandLine &)
    }
 
    pkgAcquire Fetcher;
-   Fetcher.GetLock(archivedir);
-   Fetcher.Clean(archivedir);
-   Fetcher.Clean(archivedir + "partial/");
+   if (archivedir.empty() == false && FileExists(archivedir) == true)
+   {
+      Fetcher.GetLock(archivedir);
+      Fetcher.Clean(archivedir);
+      Fetcher.Clean(archivedir + "partial/");
+   }
 
-   Fetcher.GetLock(listsdir);
-   Fetcher.Clean(listsdir + "partial/");
+   if (listsdir.empty() == false && FileExists(listsdir) == true)
+   {
+      Fetcher.GetLock(listsdir);
+      Fetcher.Clean(listsdir + "partial/");
+   }
 
    pkgCacheFile::RemoveCaches();
 
@@ -338,11 +344,15 @@ bool DoClean(CommandLine &)
 };
 bool DoAutoClean(CommandLine &)
 {
+   std::string const archivedir = _config->FindDir("Dir::Cache::Archives");
+   if (FileExists(archivedir) == false)
+      return true;
+
    // Lock the archive directory
    FileFd Lock;
    if (_config->FindB("Debug::NoLocking",false) == false)
    {
-      int lock_fd = GetLock(_config->FindDir("Dir::Cache::Archives") + "lock");
+      int lock_fd = GetLock(flCombine(archivedir, "lock"));
       if (lock_fd < 0)
 	 return _error->Error(_("Unable to lock the download directory"));
       Lock.Fd(lock_fd);
@@ -354,7 +364,7 @@ bool DoAutoClean(CommandLine &)
 
    LogCleaner Cleaner;
 
-   return Cleaner.Go(_config->FindDir("Dir::Cache::archives"),*Cache) &&
-      Cleaner.Go(_config->FindDir("Dir::Cache::archives") + "partial/",*Cache);
+   return Cleaner.Go(archivedir, *Cache) &&
+      Cleaner.Go(flCombine(archivedir, "partial/"), *Cache);
 }
 									/*}}}*/

+ 4 - 1
test/integration/framework

@@ -1510,7 +1510,10 @@ msgfailoutput() {
 			if expr match "$1" "$FILEFLAGS" >/dev/null; then
 				echo "#### stat(2) of file: $2 ####"
 				stat "$2" || true
-				if test -e "$2"; then
+				if test -d "$2"; then
+					echo "#### The directory contains: $2 ####"
+					ls >&2 "$2" || true
+				elif test -e "$2"; then
 					echo "#### Complete file: $2 ####"
 					cat >&2 "$2" || true
 				fi

+ 23 - 1
test/integration/test-apt-get-clean

@@ -36,4 +36,26 @@ testfailure test -e rootdir/var/cache/apt/archives/foo_2_all.deb
 testfailure test -e rootdir/var/cache/apt/archives/foo_3_all.deb
 testfailure test -e rootdir/var/cache/apt/archives/foo_4_all.deb
 
-
+directorygone() {
+	rm -rf "$1"
+	testsuccess apt autoclean
+	testfailure test -d "$1"
+	testsuccess apt clean
+	# clean creates an empty partial directory via GetLock
+	if [ "$(basename "$1")" = 'partial' ]; then
+		testsuccess test -d "$1"
+	else
+		testfailure test -d "$1"
+	fi
+}
+msgmsg 'Partial directory missing'
+directorygone 'rootdir/var/cache/apt/archives/partial'
+directorygone 'rootdir/var/lib/apt/lists/partial'
+
+msgmsg 'Archives directory missing'
+directorygone 'rootdir/var/cache/apt/archives'
+directorygone 'rootdir/var/lib/apt/lists'
+
+msgmsg 'apt directory missing'
+directorygone 'rootdir/var/cache/apt'
+directorygone 'rootdir/var/lib/apt'