Skip to content

Commit

Permalink
Cleanup files.PublishedStorage after dropping a publish
Browse files Browse the repository at this point in the history
After dropping files.PublishedStorage there were some leftovers -
empty directory tree. If that directory tree is empty, it is now removed.

Fixes: #198
  • Loading branch information
aol-nnov committed Oct 23, 2024
1 parent 0e6f9c3 commit 5750df4
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 10 deletions.
19 changes: 13 additions & 6 deletions deb/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)

Check failure on line 1339 in deb/publish.go

View workflow job for this annotation

GitHub Actions / Test (Ubuntu 22.04)

not enough arguments in call to localStorage.CleanupPublishTree
}
}

batch := collection.db.CreateBatch()
Expand Down
25 changes: 25 additions & 0 deletions files/public.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,31 @@ func (storage *PublishedStorage) RemoveDirs(path string, progress aptly.Progress
return os.RemoveAll(filepath)
}

// Cleans up PublishedStorage tree towards the root up to first non-empty directory
func (storage *PublishedStorage) CleanupPublishTree(path string, progress aptly.Progress) {
/*
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 first non-empty directory (or any other os.Remove error)
*/
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.
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.
Expand Down
4 changes: 4 additions & 0 deletions system/t06_publish/PublishDrop21Test_gold
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Removing ${HOME}/.aptly/public/ppa/smira/dists...
Removing ${HOME}/.aptly/public/ppa/smira/pool...

Published repository has been removed successfully.
34 changes: 30 additions & 4 deletions system/t06_publish/drop.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down

0 comments on commit 5750df4

Please sign in to comment.