launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+441821@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad:fix-regresssion-in-oval-data-publishing into launchpad:master.

Commit message:
Do not try to delete the `by-hash` directory

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #2017522 in Launchpad itself: "Private PPA has a pending publication for hours"
  https://bugs.launchpad.net/launchpad/+bug/2017522

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/441821
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:fix-regresssion-in-oval-data-publishing into launchpad:master.

diff --git a/lib/lp/archivepublisher/scripts/publishdistro.py b/lib/lp/archivepublisher/scripts/publishdistro.py
index 754c0bf..e87b389 100644
--- a/lib/lp/archivepublisher/scripts/publishdistro.py
+++ b/lib/lp/archivepublisher/scripts/publishdistro.py
@@ -424,9 +424,11 @@ class PublishDistro(PublisherScript):
             )
             return False
 
-    def synchronizeSecondDirectoryWithFirst(self, first_dir, second_dir):
+    def synchronizeSecondDirectoryWithFirst(
+        self, first_dir, second_dir, ignore
+    ):
         """Synchronize the contents of the second directory with the first."""
-        comparison = dircmp(str(first_dir), str(second_dir))
+        comparison = dircmp(str(first_dir), str(second_dir), ignore=ignore)
         files_to_copy = (
             comparison.diff_files
             + comparison.left_only
@@ -464,7 +466,7 @@ class PublishDistro(PublisherScript):
                 )
                 dest_dir.mkdir(parents=True, exist_ok=True)
                 files_modified = self.synchronizeSecondDirectoryWithFirst(
-                    staged_oval_data_dir, dest_dir
+                    staged_oval_data_dir, dest_dir, ignore=["by_hash"]
                 )
                 if files_modified:
                     updated = True
diff --git a/lib/lp/archivepublisher/tests/test_publishdistro.py b/lib/lp/archivepublisher/tests/test_publishdistro.py
index dba384e..5f9777b 100644
--- a/lib/lp/archivepublisher/tests/test_publishdistro.py
+++ b/lib/lp/archivepublisher/tests/test_publishdistro.py
@@ -1762,3 +1762,47 @@ class TestPublishDistroMethods(TestCaseWithFactory):
                 ),
             ]
         )
+
+    def test_syncOVALDataFilesForSuite_skips_by_hash_directory(self):
+        """`by-hash` directory is generated by the archive indexing machinery
+
+        It must not be deleted, so we need to skip it."""
+        self.setUpOVALDataRsync()
+        self.useFixture(
+            MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+        )
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        incoming_dir = (
+            Path(self.oval_data_root)
+            / archive.reference
+            / "breezy-autotest"
+            / "main"
+        )
+        write_file(str(incoming_dir / "test"), b"test")
+        published_dir = (
+            Path(getPubConfig(archive).distsroot)
+            / "breezy-autotest"
+            / "main"
+            / "oval"
+        )
+        # create oval files and the `by_hash` dir with some test files
+        write_file(str(published_dir / "foo.oval.xml.bz2"), b"test")
+        write_file(str(published_dir / "foo2.oval.xml.bz2"), b"test")
+        by_hash_dir = published_dir / "by_hash"
+        by_hash_dir.mkdir()
+        (by_hash_dir / "a").touch()
+        (by_hash_dir / "b").touch()
+
+        script = self.makeScript()
+        script.txn = FakeTransaction()
+        script.findDistros = FakeMethod([archive.distribution])
+        script.getTargetArchives = FakeMethod([archive])
+        publisher = FakePublisher()
+        script.getPublisher = FakeMethod(publisher)
+
+        script.main()
+
+        # `by_hash` still exists and is indeed a directory
+        self.assertTrue(by_hash_dir.is_dir())
+        # and still contains the two test files
+        self.assertEqual(2, len(list(by_hash_dir.iterdir())))