launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/delete-archiveroot into lp:launchpad with lp:~wgrant/launchpad/htaccess-in-archiveroot as a prerequisite.

Commit message:
Delete archiveroot instead of its parent (/wgrant/ppa/ubuntu instead of /wgrant/ppa). This lets us have PPAs for different distros with the same name.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1338439 in Launchpad itself: "PPA deletion deletes all PPAs with the same owner and name"
  https://bugs.launchpad.net/launchpad/+bug/1338439

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/delete-archiveroot/+merge/225797

Now that .htaccess and .htpasswd live in archiveroot (/wgrant/ppa/ubuntu/.htaccess instead of /wgrant/ppa/.htaccess) Publisher.deleteArchive can just remove /wgrant/ppa/ubuntu rather than /wgrant/ppa, letting PPAs with the same name but from other distros survive the purge.

The empty directory cleanup cron job will handle /wgrant/ppa when all the distros are gone, as it already handles /wgrant when all the PPAs are gone.
-- 
https://code.launchpad.net/~wgrant/launchpad/delete-archiveroot/+merge/225797
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/delete-archiveroot into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2014-07-07 09:57:22 +0000
+++ lib/lp/archivepublisher/publishing.py	2014-07-07 09:57:22 +0000
@@ -854,13 +854,10 @@
         be caught and an OOPS report generated.
         """
         assert self.archive.is_ppa
-        root_dir = os.path.join(
-            self._config.distroroot, self.archive.owner.name,
-            self.archive.name)
-
         self.log.info(
             "Attempting to delete archive '%s/%s' at '%s'." % (
-                self.archive.owner.name, self.archive.name, root_dir))
+                self.archive.owner.name, self.archive.name,
+                self._config.archiveroot))
 
         # Set all the publications to DELETED.
         sources = self.archive.getPublishedSources(
@@ -884,8 +881,7 @@
         for pub in self.archive.getAllPublishedBinaries(include_removed=False):
             pub.dateremoved = UTC_NOW
 
-        # XXX wgrant 2014-07-03: Needs checking for multi-distro sanity.
-        for directory in (root_dir, self._config.metaroot):
+        for directory in (self._config.archiveroot, self._config.metaroot):
             if not os.path.exists(directory):
                 continue
             try:

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2014-07-07 09:57:22 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2014-07-07 09:57:22 +0000
@@ -179,10 +179,9 @@
         os.makedirs(publisher._config.metaroot)
         open(os.path.join(publisher._config.metaroot, 'test'), 'w').close()
 
+        root_dir = publisher._config.archiveroot
+        self.assertTrue(os.path.exists(root_dir))
         publisher.deleteArchive()
-        root_dir = os.path.join(
-            publisher._config.distroroot, test_archive.owner.name,
-            test_archive.name)
         self.assertFalse(os.path.exists(root_dir))
         self.assertFalse(os.path.exists(publisher._config.metaroot))
         self.assertEqual(ArchiveStatus.DELETED, test_archive.status)
@@ -222,10 +221,9 @@
         open(os.path.join(
             publisher._config.archiveroot, 'test_file'), 'w').close()
 
+        root_dir = publisher._config.archiveroot
+        self.assertTrue(os.path.exists(root_dir))
         publisher.deleteArchive()
-        root_dir = os.path.join(
-            publisher._config.distroroot, test_archive.owner.name,
-            test_archive.name)
         self.assertFalse(os.path.exists(root_dir))
         self.assertNotIn('WARNING', logger.getLogBuffer())
         self.assertNotIn('ERROR', logger.getLogBuffer())