launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:avoid-pristine-ppas-private into launchpad:master.

Commit message:
Make process-death-row and update-pkgcache check private PPAs again

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400822

My recent change to make them skip pristine PPAs also inadvertently caused them to skip private PPAs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:avoid-pristine-ppas-private into launchpad:master.

diff --git a/cronscripts/update-pkgcache.py b/cronscripts/update-pkgcache.py
index 12bd926..9b3d766 100755
--- a/cronscripts/update-pkgcache.py
+++ b/cronscripts/update-pkgcache.py
@@ -101,7 +101,7 @@ class PackageCacheUpdater(LaunchpadCronScript):
                 'Updating %s PPAs' % distribution.name)
             archives = getUtility(IArchiveSet).getArchivesForDistribution(
                 distribution, purposes=[ArchivePurpose.PPA],
-                exclude_pristine=True)
+                check_permissions=False, exclude_pristine=True)
             for archive in archives:
                 self.updateDistributionCache(distribution, archive)
                 archive.updateArchiveCache()
diff --git a/lib/lp/archivepublisher/scripts/processdeathrow.py b/lib/lp/archivepublisher/scripts/processdeathrow.py
index f95535d..f2d9e3b 100644
--- a/lib/lp/archivepublisher/scripts/processdeathrow.py
+++ b/lib/lp/archivepublisher/scripts/processdeathrow.py
@@ -49,7 +49,7 @@ class DeathRowProcessor(PublisherScript):
         if self.options.ppa:
             return getUtility(IArchiveSet).getArchivesForDistribution(
                 distribution, purposes=[ArchivePurpose.PPA],
-                exclude_pristine=True)
+                check_permissions=False, exclude_pristine=True)
         else:
             return distribution.all_distro_archives
 
diff --git a/lib/lp/archivepublisher/tests/test_processdeathrow.py b/lib/lp/archivepublisher/tests/test_processdeathrow.py
index 21160fe..575f79a 100644
--- a/lib/lp/archivepublisher/tests/test_processdeathrow.py
+++ b/lib/lp/archivepublisher/tests/test_processdeathrow.py
@@ -181,10 +181,16 @@ class TestProcessDeathRow(TestCaseWithFactory):
         ubuntutest = getUtility(IDistributionSet)["ubuntutest"]
         cprov_archive = getUtility(IPersonSet).getByName("cprov").archive
         mark_archive = getUtility(IPersonSet).getByName("mark").archive
+        # Private PPAs are included too.
+        private_archive = self.factory.makeArchive(
+            distribution=ubuntutest, private=True)
+        self.factory.makeSourcePackagePublishingHistory(
+            archive=private_archive)
+        # Empty PPAs are skipped.
         self.factory.makeArchive(distribution=ubuntutest)
         script = DeathRowProcessor(test_args=["-d", "ubuntutest", "--ppa"])
         self.assertContentEqual(
-            [cprov_archive, mark_archive],
+            [cprov_archive, mark_archive, private_archive],
             script.getTargetArchives(ubuntutest))
 
     def test_getTargetArchives_main(self):
diff --git a/lib/lp/soyuz/doc/archive.txt b/lib/lp/soyuz/doc/archive.txt
index db8b20f..87b2cd9 100644
--- a/lib/lp/soyuz/doc/archive.txt
+++ b/lib/lp/soyuz/doc/archive.txt
@@ -1538,6 +1538,17 @@ if requested:
     true-copy        copy-owner2   False    False
     ultimate-copy    copy-owner1   False    True
 
+Passing `check_permissions=False` skips the user permission checks:
+
+    >>> ubuntu_copy_archives = archive_set.getArchivesForDistribution(
+    ...     ubuntu, purposes=[ArchivePurpose.COPY], check_permissions=False)
+    >>> print_archive_names(ubuntu_copy_archives)
+    Name             Owner         Private  Enabled
+    fine-copy        copy-owner2   False    True
+    my-copy-archive  me-copy       True     True
+    team-archive     t1            True     True
+    ultimate-copy    copy-owner1   False    True
+
 If exclude_disabled is set to True no disabled archives will be
 included:
 
diff --git a/lib/lp/soyuz/interfaces/archive.py b/lib/lp/soyuz/interfaces/archive.py
index e84542e..ed35643 100644
--- a/lib/lp/soyuz/interfaces/archive.py
+++ b/lib/lp/soyuz/interfaces/archive.py
@@ -2445,7 +2445,9 @@ class IArchiveSet(Interface):
         """
 
     def getArchivesForDistribution(distribution, name=None, purposes=None,
-        user=None, exclude_disabled=True, exclude_pristine=False):
+                                   check_permissions=True, user=None,
+                                   exclude_disabled=True,
+                                   exclude_pristine=False):
         """Return a list of all the archives for a distribution.
 
         This will return all the archives for the given distribution, with
@@ -2456,10 +2458,14 @@ class IArchiveSet(Interface):
             the results to only those archives with this name.
         :param purposes: An optional archive purpose or list of purposes with
             which to filter the results.
+        :param check_permissions: If False, return both public and private
+            archives regardless of user permission checks. This is intended
+            for use by internal scripts.
         :param user: An optional `IPerson` who is requesting the archives,
             which is used to include private archives for which the user
             has permission. If it is not supplied, only public archives
-            will be returned.
+            will be returned. These checks are skipped if
+            `check_permissions` is False.
         :param exclude_disabled: Whether to exclude disabled archives.
         :param exclude_pristine: Whether to exclude archives that have never
             had any publications.
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index c3cbaa1..eab11f7 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -2894,7 +2894,8 @@ class ArchiveSet:
             Archive._private == True, Archive.purpose == ArchivePurpose.PPA)
 
     def getArchivesForDistribution(self, distribution, name=None,
-                                   purposes=None, user=None,
+                                   purposes=None,
+                                   check_permissions=True, user=None,
                                    exclude_disabled=True,
                                    exclude_pristine=False):
         """See `IArchiveSet`."""
@@ -2914,7 +2915,9 @@ class ArchiveSet:
         public_archive = And(Archive._private == False,
                              Archive._enabled == True)
 
-        if user is not None:
+        if not check_permissions:
+            pass
+        elif user is not None:
             admins = getUtility(ILaunchpadCelebrities).admin
             if not user.inTeam(admins):
                 # Enforce privacy-awareness for logged-in, non-admin users,