From 0c9fe00367aebc0ad6f72d678f792a4c83f6b334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BD=D0=B4=D1=80=D0=B5=D0=B9=20=D0=9B=D1=83=D1=85?= =?UTF-8?q?=D0=BD=D0=BE=D0=B2?= Date: Tue, 22 Oct 2024 17:27:36 +0300 Subject: [PATCH] Cleanup files.PublishedStorage after dropping a publish After dropping files.PublishedStorage there were some leftovers - empty directory tree. If that directory tree is empty, it is now removed. Fixes: https://github.com/aptly-dev/aptly/issues/198 --- deb/publish.go | 19 +++++++++---- files/public.go | 32 +++++++++++++++++++++ system/t06_publish/PublishDrop21Test_gold | 5 ++++ system/t06_publish/drop.py | 34 ++++++++++++++++++++--- 4 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 system/t06_publish/PublishDrop21Test_gold diff --git a/deb/publish.go b/deb/publish.go index 79c47cb83..5105097b2 100644 --- a/deb/publish.go +++ b/deb/publish.go @@ -17,6 +17,7 @@ import ( "github.com/aptly-dev/aptly/aptly" "github.com/aptly-dev/aptly/database" + "github.com/aptly-dev/aptly/files" "github.com/aptly-dev/aptly/pgp" "github.com/aptly-dev/aptly/utils" ) @@ -1323,14 +1324,20 @@ func (collection *PublishedRepoCollection) Remove(publishedStorageProvider aptly collection.list[len(collection.list)-1], collection.list[repoPosition], collection.list = nil, collection.list[len(collection.list)-1], collection.list[:len(collection.list)-1] - if !skipCleanup && len(cleanComponents) > 0 { - err = collection.CleanupPrefixComponentFiles(repo.Prefix, cleanComponents, - publishedStorageProvider.GetPublishedStorage(storage), collectionFactory, progress) - if err != nil { - if !force { - return fmt.Errorf("cleanup failed, use -force-drop to override: %s", err) + if !skipCleanup { + publishedStorage := publishedStorageProvider.GetPublishedStorage(storage) + if len(cleanComponents) > 0 { + err = collection.CleanupPrefixComponentFiles(repo.Prefix, cleanComponents, publishedStorage, collectionFactory, progress) + if err != nil { + if !force { + return fmt.Errorf("cleanup failed, use -force-drop to override: %s", err) + } } } + + if localStorage, ok := publishedStorage.(*files.PublishedStorage); ok { + localStorage.CleanupPublishTree(repo.Prefix, progress) + } } batch := collection.db.CreateBatch() diff --git a/files/public.go b/files/public.go index 848f0c23c..f95a4d61e 100644 --- a/files/public.go +++ b/files/public.go @@ -119,6 +119,38 @@ func (storage *PublishedStorage) RemoveDirs(path string, progress aptly.Progress return os.RemoveAll(filepath) } +// Cleans up PublishedStorage tree towards the publish storage root up to first non-empty directory +func (storage *PublishedStorage) CleanupPublishTree(path string, progress aptly.Progress) { + path = strings.TrimLeft(path, "./") + if path == "" { + return + } + /* + Say, `path` is "a/b/c/d", let's remove all empty dirs: + storage.rootPath + "a/b/c/d" + storage.rootPath + "a/b/c" + storage.rootPath + "a/b" + ... and so on, up to storage.rootPath, first non-empty directory or any other os.Remove error, whichever comes first + */ + + segments := strings.Split(path, "/") + + for segmentsCount := len(segments); segmentsCount > 0; segmentsCount-- { + fullPath := filepath.Join(storage.rootPath, strings.Join(segments[:segmentsCount], "/")) + + if err := os.Remove(fullPath); err != nil { + // We stop crawling the tree on any os.Remove error. + // Among all of the return values there might be 'directory not empty', + // which means, we reached non-empty dir, which is okay. + // Any other errors are logged for informational reasons. + if progress != nil { + progress.Printf("CleanupPublishTree: %s. '%s' not removed\n", err, fullPath) + } + break + } + } +} + // LinkFromPool links package file from pool to dist's pool location // // publishedPrefix is desired prefix for the location in the pool. diff --git a/system/t06_publish/PublishDrop21Test_gold b/system/t06_publish/PublishDrop21Test_gold new file mode 100644 index 000000000..0a9fa6915 --- /dev/null +++ b/system/t06_publish/PublishDrop21Test_gold @@ -0,0 +1,5 @@ +Removing ${HOME}/.aptly/public/ppa/smira/dists... +Removing ${HOME}/.aptly/public/ppa/smira/pool... +CleanupPublishTree: remove ${HOME}/.aptly/public/ppa: directory not empty. '${HOME}/.aptly/public/ppa' not removed + +Published repository has been removed successfully. diff --git a/system/t06_publish/drop.py b/system/t06_publish/drop.py index 3c361cc31..0c2ad79ae 100644 --- a/system/t06_publish/drop.py +++ b/system/t06_publish/drop.py @@ -25,6 +25,7 @@ def check(self): class PublishDrop2Test(BaseTest): """ publish drop: under prefix + if remaining tree is empty, it is now removed """ requiresGPG2 = True fixtureDB = True @@ -39,9 +40,32 @@ class PublishDrop2Test(BaseTest): def check(self): super(PublishDrop2Test, self).check() - self.check_not_exists('public/ppa/smira/dists/') - self.check_not_exists('public/ppa/smira/pool/') - self.check_exists('public/ppa/smira/') + self.check_not_exists('public/ppa/') + self.check_exists('public/') + + +class PublishDrop21Test(BaseTest): + """ + publish drop: under prefix + if some publishes have common part of the prefix, only empty parts are cleared + """ + requiresGPG2 = True + fixtureDB = True + fixturePool = True + fixtureCmds = [ + "aptly snapshot create snap1 from mirror gnuplot-maverick", + "aptly snapshot create snap2 from mirror gnuplot-maverick", + "aptly publish snapshot -keyring=${files}/aptly.pub -secret-keyring=${files}/aptly.sec snap1 ppa/smira", + "aptly publish snapshot -keyring=${files}/aptly.pub -secret-keyring=${files}/aptly.sec snap2 ppa/another", + ] + runCmd = "aptly publish drop maverick ppa/smira" + gold_processor = BaseTest.expand_environ + + def check(self): + super(PublishDrop21Test, self).check() + + self.check_not_exists('public/ppa/smira/') + self.check_exists('public/ppa/another') class PublishDrop3Test(BaseTest): @@ -132,6 +156,7 @@ class PublishDrop6Test(BaseTest): class PublishDrop7Test(BaseTest): """ publish drop: under prefix with trailing & leading slashes + see comment in PublishDrop2Test """ requiresGPG2 = True fixtureDB = True @@ -148,7 +173,8 @@ def check(self): self.check_not_exists('public/ppa/smira/dists/') self.check_not_exists('public/ppa/smira/pool/') - self.check_exists('public/ppa/smira/') + self.check_not_exists('public/ppa/smira/') + self.check_exists('public/') class PublishDrop8Test(BaseTest):