From caed9c234dd87b202a1d60bc01157b1a3987f9bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Roth?= Date: Sat, 13 Jun 2026 12:42:00 +0200 Subject: [PATCH] publish: support MultiDist toggle --- api/publish.go | 24 ++++++ deb/publish.go | 46 ++++++++++++ system/t12_api/publish.py | 154 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 224 insertions(+) diff --git a/api/publish.go b/api/publish.go index 5ab7ccdb..80859680 100644 --- a/api/publish.go +++ b/api/publish.go @@ -530,6 +530,9 @@ func apiPublishUpdateSwitch(c *gin.Context) { return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err) } + // Capture MultiDist before mutations to detect a false→true transition. + prevMultiDist := published.MultiDist + // Apply field mutations on the freshly loaded object. if b.SkipContents != nil { published.SkipContents = *b.SkipContents @@ -589,6 +592,15 @@ func apiPublishUpdateSwitch(c *gin.Context) { if err != nil { return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err) } + // When MultiDist is toggled, the old pool layout still has files that + // CleanupPrefixComponentFiles won't touch (it only scans the new layout). + // Run a second pass over the previous layout to remove stale files. + if prevMultiDist != published.MultiDist { + err = taskCollection.CleanupAfterMultiDistToggle(context, published, prevMultiDist, cleanComponents, taskCollectionFactory, out) + if err != nil { + return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to clean legacy pool: %s", err) + } + } } return &task.ProcessReturnValue{Code: http.StatusOK, Value: published}, nil @@ -1196,6 +1208,9 @@ func apiPublishUpdate(c *gin.Context) { return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err) } + // Capture MultiDist before mutations to detect a false→true transition. + prevMultiDist := published.MultiDist + // Apply field mutations on the freshly loaded object. if b.SkipContents != nil { published.SkipContents = *b.SkipContents @@ -1244,6 +1259,15 @@ func apiPublishUpdate(c *gin.Context) { if err != nil { return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err) } + // When MultiDist is toggled, the old pool layout still has files that + // CleanupPrefixComponentFiles won't touch (it only scans the new layout). + // Run a second pass over the previous layout to remove stale files. + if prevMultiDist != published.MultiDist { + err = taskCollection.CleanupAfterMultiDistToggle(context, published, prevMultiDist, cleanComponents, taskCollectionFactory, out) + if err != nil { + return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to clean legacy pool: %s", err) + } + } } return &task.ProcessReturnValue{Code: http.StatusOK, Value: published}, nil diff --git a/deb/publish.go b/deb/publish.go index dbaa81a7..a17dae81 100644 --- a/deb/publish.go +++ b/deb/publish.go @@ -1598,6 +1598,52 @@ func (collection *PublishedRepoCollection) listReferencedFilesByComponent(prefix return referencedFiles, nil } +// CleanupAfterMultiDistToggle cleans up stale pool files left behind when the +// MultiDist flag is toggled on a published repository. +// +// - false→true: Publish() wrote packages into pool/// +// but the old flat pool// files were not removed because +// CleanupPrefixComponentFiles only scans the new MultiDist tree. +// A second pass with MultiDist=false cleans the legacy flat layout by +// reusing the existing orphan-detection logic (the repo is now MultiDist=true +// so it is excluded from the referenced-files scan, making its old pool +// entries appear orphaned). +// +// - true→false: Publish() wrote packages into pool// but the old +// per-distribution pool/// directories were not +// removed. The orphan-detection approach cannot be used here because the +// repo's RefList still contains all packages (they just moved locations). +// Instead we directly remove each pool/// directory. +// This is safe because per-distribution pool dirs are exclusive to a single +// prefix+distribution combination — no other published repo can share them. +func (collection *PublishedRepoCollection) CleanupAfterMultiDistToggle(publishedStorageProvider aptly.PublishedStorageProvider, + published *PublishedRepo, prevMultiDist bool, cleanComponents []string, collectionFactory *CollectionFactory, progress aptly.Progress) error { + if prevMultiDist == published.MultiDist { + return nil + } + + if !prevMultiDist && published.MultiDist { + // false→true: use orphan-detection via the existing cleanup, but with + // MultiDist temporarily set to false so it scans the flat pool layout. + legacy := *published + legacy.MultiDist = false + return collection.CleanupPrefixComponentFiles(publishedStorageProvider, &legacy, cleanComponents, collectionFactory, progress) + } + + // true→false: directly remove the per-distribution pool directories. + publishedStorage := publishedStorageProvider.GetPublishedStorage(published.Storage) + for _, component := range cleanComponents { + poolDir := filepath.Join(published.Prefix, "pool", published.Distribution, component) + if err := publishedStorage.RemoveDirs(poolDir, progress); err != nil { + return err + } + } + // Remove the distribution-level pool dir if it is now empty. + distPoolDir := filepath.Join(published.Prefix, "pool", published.Distribution) + _ = publishedStorage.RemoveDirs(distPoolDir, progress) + return nil +} + // CleanupPrefixComponentFiles removes all unreferenced files in published storage under prefix/component pair func (collection *PublishedRepoCollection) CleanupPrefixComponentFiles(publishedStorageProvider aptly.PublishedStorageProvider, published *PublishedRepo, cleanComponents []string, collectionFactory *CollectionFactory, progress aptly.Progress) error { diff --git a/system/t12_api/publish.py b/system/t12_api/publish.py index cd92f775..e83ae2c4 100644 --- a/system/t12_api/publish.py +++ b/system/t12_api/publish.py @@ -666,6 +666,160 @@ class PublishUpdateAPIMultiDist(APITest): self.check_not_exists("public/" + prefix + "dists/") +class PublishUpdateAPIMultiDistToggle(APITest): + """ + POST /publish/:prefix with MultiDist=false, then PUT to enable MultiDist=true + """ + fixtureGpg = True + + def check(self): + repo_name = self.random_name() + self.check_equal(self.post( + "/api/repos", json={"Name": repo_name, "DefaultDistribution": "bookworm"}).status_code, 201) + + d = self.random_name() + self.check_equal(self.upload("/api/files/" + d, + "libboost-program-options-dev_1.49.0.1_i386.deb", "pyspi_0.6.1-1.3.dsc", + "pyspi_0.6.1-1.3.diff.gz", "pyspi_0.6.1.orig.tar.gz", + "pyspi-0.6.1-1.3.stripped.dsc").status_code, 200) + + task = self.post_task("/api/repos/" + repo_name + "/file/" + d) + self.check_task(task) + + # Publish with MultiDist=false (default) + prefix = self.random_name() + task = self.post_task( + "/api/publish/" + prefix, + json={ + "Architectures": ["i386", "source"], + "SourceKind": "local", + "Sources": [{"Name": repo_name}], + "Signing": DefaultSigningOptions, + "MultiDist": False, + } + ) + self.check_task(task) + + repo_expected = { + 'AcquireByHash': False, + 'Architectures': ['i386', 'source'], + 'Codename': '', + 'Distribution': 'bookworm', + 'Label': '', + 'Origin': '', + 'Version': '', + 'NotAutomatic': '', + 'ButAutomaticUpgrades': '', + 'Path': prefix + '/' + 'bookworm', + 'Prefix': prefix, + 'SignedBy': '', + 'SkipContents': False, + 'MultiDist': False, + 'SourceKind': 'local', + 'Sources': [{'Component': 'main', 'Name': repo_name}], + 'Storage': '', + 'Suite': ''} + + all_repos = self.get("/api/publish") + self.check_equal(all_repos.status_code, 200) + self.check_in(repo_expected, all_repos.json()) + + # With MultiDist=false packages are stored under pool/main/... + self.check_exists("public/" + prefix + "/dists/bookworm/Release") + self.check_exists("public/" + prefix + + "/dists/bookworm/main/binary-i386/Packages") + self.check_exists("public/" + prefix + + "/dists/bookworm/main/source/Sources") + self.check_exists( + "public/" + prefix + "/pool/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb") + self.check_exists( + "public/" + prefix + "/pool/main/p/pyspi/pyspi-0.6.1-1.3.stripped.dsc") + # MultiDist-style per-distribution pool must not exist yet + self.check_not_exists( + "public/" + prefix + "/pool/bookworm/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb") + + # Now update the published repo enabling MultiDist=true + task = self.put_task( + "/api/publish/" + prefix + "/bookworm", + json={ + "MultiDist": True, + "Signing": DefaultSigningOptions, + } + ) + self.check_task(task) + + repo_expected_multidist = { + 'AcquireByHash': False, + 'Architectures': ['i386', 'source'], + 'Codename': '', + 'Distribution': 'bookworm', + 'Label': '', + 'Origin': '', + 'Version': '', + 'NotAutomatic': '', + 'ButAutomaticUpgrades': '', + 'Path': prefix + '/' + 'bookworm', + 'Prefix': prefix, + 'SignedBy': '', + 'SkipContents': False, + 'MultiDist': True, + 'SourceKind': 'local', + 'Sources': [{'Component': 'main', 'Name': repo_name}], + 'Storage': '', + 'Suite': ''} + + all_repos = self.get("/api/publish") + self.check_equal(all_repos.status_code, 200) + self.check_in(repo_expected_multidist, all_repos.json()) + + # After enabling MultiDist, packages are stored under pool//main/... + self.check_exists("public/" + prefix + "/dists/bookworm/Release") + self.check_exists("public/" + prefix + + "/dists/bookworm/main/binary-i386/Packages") + self.check_exists("public/" + prefix + + "/dists/bookworm/main/source/Sources") + self.check_exists( + "public/" + prefix + "/pool/bookworm/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb") + self.check_exists( + "public/" + prefix + "/pool/bookworm/main/p/pyspi/pyspi-0.6.1-1.3.stripped.dsc") + # Flat pool must not exist while MultiDist is on + self.check_not_exists( + "public/" + prefix + "/pool/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb") + + # Switch MultiDist back to false + task = self.put_task( + "/api/publish/" + prefix + "/bookworm", + json={ + "MultiDist": False, + "Signing": DefaultSigningOptions, + } + ) + self.check_task(task) + + repo_expected["MultiDist"] = False + all_repos = self.get("/api/publish") + self.check_equal(all_repos.status_code, 200) + self.check_in(repo_expected, all_repos.json()) + + # Packages are back under the flat pool/main/... + self.check_exists("public/" + prefix + "/dists/bookworm/Release") + self.check_exists("public/" + prefix + + "/dists/bookworm/main/binary-i386/Packages") + self.check_exists("public/" + prefix + + "/dists/bookworm/main/source/Sources") + self.check_exists( + "public/" + prefix + "/pool/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb") + self.check_exists( + "public/" + prefix + "/pool/main/p/pyspi/pyspi-0.6.1-1.3.stripped.dsc") + # Per-distribution pool must be gone + self.check_not_exists( + "public/" + prefix + "/pool/bookworm/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb") + + task = self.delete_task("/api/publish/" + prefix + "/bookworm") + self.check_task(task) + self.check_not_exists("public/" + prefix + "dists/") + + class PublishConcurrentUpdateAPITestRepo(APITest): """ PUT /publish/:prefix/:distribution (local repos), DELETE /publish/:prefix/:distribution