Merge lp:~wgrant/launchpad/bug-897999 into lp:launchpad

[William Grant]

Rip out the restriction that non-virtualized main archives have to build on all restricted processor families. It doesn't make much sense, is a fair bit of code, and the form integration is buggy.

Presently a main archive (primary or partner) that builds on non-virtual builders is implicitly allowed to use all restricted processor families, regardless of the configuration in the database. The widgets on Archive:+admin and Distribution:+edit attempt to enforce this, but end up being both too strict and too lenient, resulting in problems like bug #897999.

This branch removes the main archive special case. It applies to a total of two archives on production which can be easily configured manually, it's a fair bit of code, and the corresponding UI has always been fairly broken. All archives can now have restricted families enabled or disabled as desired.

Ubuntu's primary and partner archives will need tweaking on production before this is deployed; I suspect that simply clicking Save on their respective +admin forms will be sufficient.

[Steve Kowalik (stevenk)]

Looks like excellent work to me.

review: Approve (code)

=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2012-08-24 05:09:51 +0000
+++ lib/lp/registry/browser/distribution.py	2012-09-21 07:05:28 +0000
@@ -888,10 +888,6 @@
         """The page title."""
         return self.label
 
-    def validate(self, data):
-        self.validate_enabled_restricted_families(
-            data, ENABLED_RESTRICTED_FAMILITES_ERROR_MSG)
-
     @property
     def initial_values(self):
         proc_family_set = getUtility(IProcessorFamilySet)
@@ -937,12 +933,6 @@
         self.next_url = canonical_url(distribution)
 
 
-ENABLED_RESTRICTED_FAMILITES_ERROR_MSG = (
-    u"This distribution's main archive can not be restricted to "
-    'certain architectures unless the archive is also set '
-    'to build on virtualized builders.')
-
-
 class DistributionEditView(RegistryEditFormView,
                            RequireVirtualizedBuildersMixin,
                            EnableRestrictedFamiliesMixin):
@@ -1004,11 +994,6 @@
         if not official_malone:
             data['enable_bug_expiration'] = False
 
-        # Validate enabled_restricted_families.
-        self.validate_enabled_restricted_families(
-            data,
-            ENABLED_RESTRICTED_FAMILITES_ERROR_MSG)
-
     def change_archive_fields(self, data):
         # Update context.main_archive.
         new_require_virtualized = data.get('require_virtualized')

=== modified file 'lib/lp/registry/browser/tests/test_distribution_views.py'
--- lib/lp/registry/browser/tests/test_distribution_views.py	2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_distribution_views.py	2012-09-21 07:05:28 +0000
@@ -169,39 +169,13 @@
     def test_add_distro_enabled_restricted_families(self):
         creation_form = self.getDefaultAddDict()
         creation_form['field.enabled_restricted_families'] = []
-        creation_form['field.require_virtualized'] = 'on'
         create_initialized_view(
             self.distributionset, '+add', principal=self.admin,
             method='POST', form=creation_form)
 
         distribution = self.distributionset.getByName('newbuntu')
-        self.assertEqual(
-            True,
-            distribution.main_archive.require_virtualized)
         self.assertContentEqual(
-            [],
-            distribution.main_archive.enabled_restricted_families)
-
-    def test_add_distro_enabled_restricted_families_error(self):
-        # If require_virtualized is False, enabled_restricted_families
-        # cannot be changed.
-        creation_form = self.getDefaultAddDict()
-        creation_form['field.enabled_restricted_families'] = []
-        creation_form['field.require_virtualized'] = ''
-        view = create_initialized_view(
-            self.distributionset, '+add', principal=self.admin,
-            method='POST', form=creation_form)
-
-        error_msg = (
-            u"This distribution's main archive can not be restricted to "
-            "certain architectures unless the archive is also set to build "
-            "on virtualized builders.")
-        self.assertEqual(
-           error_msg,
-           view.widget_errors.get('enabled_restricted_families'))
-        self.assertEqual(
-           error_msg,
-           view.widget_errors.get('require_virtualized'))
+            [], distribution.main_archive.enabled_restricted_families)
 
 
 class TestDistroEditView(TestCaseWithFactory):
@@ -227,7 +201,8 @@
             widget._getCurrentValue())
 
     def test_edit_distro_init_value_enabled_restricted_families(self):
-        self.distribution.main_archive.require_virtualized = False
+        self.distribution.main_archive.enabled_restricted_families = (
+            self.restricted_families)
         view = create_initialized_view(
             self.distribution, '+edit', principal=self.admin,
             method='GET')
@@ -264,16 +239,11 @@
             self.distribution.main_archive.require_virtualized)
 
     def test_change_enabled_restricted_families(self):
-        # If require_virtualized is True, enabled_restricted_families
-        # can be changed.
         edit_form = self.getDefaultEditDict()
-        edit_form['field.require_virtualized'] = 'on'
         edit_form['field.enabled_restricted_families'] = []
 
-        self.distribution.main_archive.require_virtualized = False
-        self.assertContentEqual(
-            self.restricted_families,
-            self.distribution.main_archive.enabled_restricted_families)
+        self.distribution.main_archive.enabled_restricted_families = (
+            self.restricted_families)
         create_initialized_view(
             self.distribution, '+edit', principal=self.admin,
             method='POST', form=edit_form)
@@ -282,27 +252,6 @@
             [],
             self.distribution.main_archive.enabled_restricted_families)
 
-    def test_cannot_change_enabled_restricted_families(self):
-        # If require_virtualized is False, enabled_restricted_families
-        # cannot be changed.
-        edit_form = self.getDefaultEditDict()
-        edit_form['field.require_virtualized'] = ''
-        edit_form['field.enabled_restricted_families'] = []
-
-        view = create_initialized_view(
-            self.distribution, '+edit', principal=self.admin,
-            method='POST', form=edit_form)
-        error_msg = (
-            u"This distribution's main archive can not be restricted to "
-            "certain architectures unless the archive is also set to build "
-            "on virtualized builders.")
-        self.assertEqual(
-           error_msg,
-           view.widget_errors.get('enabled_restricted_families'))
-        self.assertEqual(
-           error_msg,
-           view.widget_errors.get('require_virtualized'))
-
     def test_package_derivatives_email(self):
         # Test that the edit form allows changing package_derivatives_email
         edit_form = self.getDefaultEditDict()

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2012-09-13 04:11:09 +0000
+++ lib/lp/soyuz/browser/archive.py	2012-09-21 07:05:28 +0000
@@ -2086,9 +2086,7 @@
     """A mixin that provides enabled_restricted_families field support"""
 
     def createEnabledRestrictedFamilies(self, description=None):
-        """Creates the 'enabled_restricted_families' field.
-
-        """
+        """Creates the 'enabled_restricted_families' field."""
         terms = []
         for family in getUtility(IProcessorFamilySet).getRestricted():
             terms.append(SimpleTerm(
@@ -2103,22 +2101,6 @@
                      else description),
                  render_context=self.render_context)
 
-    def validate_enabled_restricted_families(self, data, error_msg):
-        enabled_restricted_families = data['enabled_restricted_families']
-        require_virtualized = data.get('require_virtualized', False)
-        proc_family_set = getUtility(IProcessorFamilySet)
-        if (not require_virtualized and
-            set(enabled_restricted_families) !=
-                set(proc_family_set.getRestricted())):
-            self.setFieldError('enabled_restricted_families', error_msg)
-            self.setFieldError('require_virtualized', error_msg)
-
-
-ARCHIVE_ENABLED_RESTRICTED_FAMILITES_ERROR_MSG = (
-    u'Main archives can not be restricted to certain '
-    'architectures unless they are set to build on '
-    'virtualized builders.')
-
 
 class ArchiveAdminView(BaseArchiveEditView, EnableRestrictedFamiliesMixin):
 
@@ -2187,20 +2169,6 @@
                 error_text = "\n".join(errors)
                 self.setFieldError('external_dependencies', error_text)
 
-        enabled_restricted_families = data.get('enabled_restricted_families')
-        require_virtualized = data.get('require_virtualized')
-        proc_family_set = getUtility(IProcessorFamilySet)
-        if (enabled_restricted_families and
-            not require_virtualized and
-            set(enabled_restricted_families) !=
-                set(proc_family_set.getRestricted())):
-            self.setFieldError(
-                'enabled_restricted_families',
-                ARCHIVE_ENABLED_RESTRICTED_FAMILITES_ERROR_MSG)
-            self.setFieldError(
-                'require_virtualized',
-                ARCHIVE_ENABLED_RESTRICTED_FAMILITES_ERROR_MSG)
-
     @property
     def owner_is_private_team(self):
         """Is the owner a private team?

=== modified file 'lib/lp/soyuz/browser/tests/test_archive_admin_view.py'
--- lib/lp/soyuz/browser/tests/test_archive_admin_view.py	2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_admin_view.py	2012-09-21 07:05:28 +0000
@@ -103,50 +103,3 @@
             'This archive already has published sources. '
             'It is not possible to switch the privacy.',
             view.errors[0])
-
-    def test_can_leave_restricted_families_unchecked(self):
-        # We should be able to leave restricted families unchecked and
-        # still submit the form. (see bug 888083)
-        self.factory.makeProcessorFamily(restricted=True)
-        self.factory.makeProcessorFamily(restricted=True)
-        form = {
-            "field.enabled": "on",
-            'field.private': 'off',
-            "field.require_virtualized": "off",
-            'field.enabled_restricted_families': [],
-            'field.actions.save': 'Save',
-            }
-
-        view = ArchiveAdminView(
-            self.ppa, LaunchpadTestRequest(method="POST", form=form))
-        view.initialize()
-
-        self.assertEqual(
-           None, view.widget_errors.get('enabled_restricted_families'))
-
-    def test_cannot_change_enabled_restricted_families(self):
-        # If require_virtualized is False, enabled_restricted_families
-        # cannot be changed.
-        pf1 = self.factory.makeProcessorFamily(restricted=True)
-        method = 'POST'
-        form = {
-            'field.enabled': 'on',
-            'field.require_virtualized': '',
-            'field.enabled_restricted_families': [pf1.name],
-            'field.actions.save': 'Save',
-            }
-
-        view = ArchiveAdminView(self.ppa, LaunchpadTestRequest(
-            method=method, form=form))
-        view.initialize()
-
-        error_msg = (
-            u'Main archives can not be restricted to certain '
-            'architectures unless they are set to build on '
-            'virtualized builders.')
-        self.assertEqual(
-           error_msg,
-           view.widget_errors.get('enabled_restricted_families'))
-        self.assertEqual(
-           error_msg,
-           view.widget_errors.get('require_virtualized'))

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2012-08-17 10:37:13 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2012-09-21 07:05:28 +0000
@@ -16,7 +16,6 @@
     'CannotCopy',
     'CannotSwitchPrivacy',
     'ComponentNotFound',
-    'CannotRestrictArchitectures',
     'CannotUploadToArchive',
     'CannotUploadToPPA',
     'CannotUploadToPocket',
@@ -180,10 +179,6 @@
     """Raised on some queries when version is specified but name is not."""
 
 
-class CannotRestrictArchitectures(Exception):
-    """The architectures for this archive can not be restricted."""
-
-
 @error_status(httplib.FORBIDDEN)
 class CannotUploadToArchive(Exception):
     """A reason for not being able to upload to an archive."""

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-09-17 05:25:38 +0000
+++ lib/lp/soyuz/model/archive.py	2012-09-21 07:05:28 +0000
@@ -125,7 +125,6 @@
     ArchiveDisabled,
     ArchiveNotPrivate,
     CannotCopy,
-    CannotRestrictArchitectures,
     CannotSwitchPrivacy,
     CannotUploadToPocket,
     CannotUploadToPPA,
@@ -168,7 +167,6 @@
     )
 from lp.soyuz.interfaces.packagecopyjob import IPlainPackageCopyJobSource
 from lp.soyuz.interfaces.packagecopyrequest import IPackageCopyRequestSet
-from lp.soyuz.interfaces.processor import IProcessorFamilySet
 from lp.soyuz.interfaces.publishing import (
     active_publishing_status,
     IPublishingSet,
@@ -2033,28 +2031,14 @@
     def _getEnabledRestrictedFamilies(self):
         """Retrieve the restricted architecture families this archive can
         build on."""
-        # Main archives are always allowed to build on restricted
-        # architectures if require_virtualized is False.
-        if self.is_main and not self.require_virtualized:
-            return getUtility(IProcessorFamilySet).getRestricted()
-        archive_arch_set = getUtility(IArchiveArchSet)
-        restricted_families = archive_arch_set.getRestrictedFamilies(self)
-        return [family for (family, archive_arch) in restricted_families
-                if archive_arch is not None]
+        families = getUtility(IArchiveArchSet).getRestrictedFamilies(self)
+        return [
+            family for (family, archive_arch) in families
+            if archive_arch is not None]
 
     def _setEnabledRestrictedFamilies(self, value):
         """Set the restricted architecture families this archive can
         build on."""
-        # Main archives are not allowed to build on restricted
-        # architectures unless they are set to build on virtualized
-        # builders.
-        if (self.is_main and not self.require_virtualized):
-            proc_family_set = getUtility(IProcessorFamilySet)
-            if set(value) != set(proc_family_set.getRestricted()):
-                raise CannotRestrictArchitectures(
-                    "Main archives can not be restricted to certain "
-                    "architectures unless they are set to build on "
-                    "virtualized builders")
         archive_arch_set = getUtility(IArchiveArchSet)
         restricted_families = archive_arch_set.getRestrictedFamilies(self)
         for (family, archive_arch) in restricted_families:

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2012-09-13 05:20:10 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2012-09-21 07:05:28 +0000
@@ -56,7 +56,6 @@
     ArchiveDependencyError,
     ArchiveDisabled,
     CannotCopy,
-    CannotRestrictArchitectures,
     CannotUploadToPocket,
     CannotUploadToPPA,
     IArchiveSet,
@@ -1038,35 +1037,6 @@
         self.archive_arch_set = getUtility(IArchiveArchSet)
         self.arm = getUtility(IProcessorFamilySet).getByName('arm')
 
-    def test_main_archive_can_use_restricted(self):
-        # Main archives for distributions can always use restricted
-        # architectures if they are not using virtual builders.
-        distro = self.factory.makeDistribution()
-        distro.main_archive.require_virtualized = False
-        self.assertContentEqual([self.arm],
-            distro.main_archive.enabled_restricted_families)
-
-    def test_main_archive_can_not_be_restricted_not_virtualized(self):
-        # A main archive can not be restricted to certain architectures
-        # (unless it's set to build on virtualized builders).
-        distro = self.factory.makeDistribution()
-        distro.main_archive.require_virtualized = False
-        # Restricting to all restricted architectures is fine
-        distro.main_archive.enabled_restricted_families = [self.arm]
-        self.assertRaises(
-            CannotRestrictArchitectures, setattr, distro.main_archive,
-            "enabled_restricted_families", [])
-
-    def test_main_virtualized_archive_can_be_restricted(self):
-        # A main archive can be restricted to certain architectures
-        # if it's set to build on virtualized builders.
-        distro = self.factory.makeDistribution()
-        distro.main_archive.require_virtualized = True
-
-        # Restricting to architectures is fine.
-        distro.main_archive.enabled_restricted_families = [self.arm]
-        distro.main_archive.enabled_restricted_families = []
-
     def test_default(self):
         """By default, ARM builds are not allowed as ARM is restricted."""
         self.assertEqual(0,