launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-680889-arch-wildcards into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #680889 Needs to handle "all linux-any" like "linux-any"
  https://bugs.launchpad.net/bugs/680889


As reported in bug #680889, lp.soyuz.pas.determineArchitecturesToBuild fails to properly handle valid architecture hint strings such as "all linux-any", creating no builds at all. This branch adjusts the "all", "any", and "linux-any" special cases to match even when they are not the sole component of the string.

It's a pretty simple change, except that I had to move the "all" check to the end, so hint strings like "all amd64" create an amd64 build, not an i386 arch-indep one.

I extended the tests to cover the specific problem reported and a few other edge cases.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-680889-arch-wildcards/+merge/42723
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-680889-arch-wildcards into lp:launchpad.

=== modified file 'lib/lp/soyuz/doc/package-arch-specific.txt'
--- lib/lp/soyuz/doc/package-arch-specific.txt	2010-10-14 18:42:19 +0000
+++ lib/lp/soyuz/doc/package-arch-specific.txt	2010-12-04 08:42:57 +0000
@@ -26,11 +26,6 @@
     >>> ignore = test_publisher.setUpDefaultDistroSeries(hoary)
     >>> test_publisher.addFakeChroots()
 
-Create an architecture-independent source.
-
-    >>> pub_all = test_publisher.getPubSource(
-    ...     sourcename='test-buildd', version='667')
-
 Create an architecture-specific source.
 
     >>> pub_any = test_publisher.getPubSource(
@@ -49,7 +44,22 @@
     ...     sourcename='test-buildd-4', version='670',
     ...     architecturehintlist="hppa")
 
+The architecture-independent value 'all' may appear alongside other
+architectures, but it has no effect unless it is the sole matching term.
+
+    >>> pub_all = test_publisher.getPubSource(
+    ...     sourcename='test-buildd', version='667')
+
+    >>> pub_one_and_all = test_publisher.getPubSource(
+    ...     sourcename='test-buildd-4', version='671',
+    ...     architecturehintlist="hppa all")
+
+    >>> pub_fictional_and_all = test_publisher.getPubSource(
+    ...     sourcename='test-buildd-4', version='672',
+    ...     architecturehintlist="fictional all")
+
 Create sources with 'any-$(ARCH)', 'linux-$(ARCH)' and 'linux-any' notation.
+Again, 'all' may occur alongside any of these, but it has no effect.
 
     >>> pub_any_foo = test_publisher.getPubSource(
     ...     sourcename='test-buildd-6', version='672',
@@ -63,6 +73,14 @@
     ...     sourcename='test-buildd-7', version='674',
     ...     architecturehintlist="linux-any")
 
+    >>> pub_all_linux_any = test_publisher.getPubSource(
+    ...     sourcename='test-buildd-7', version='675',
+    ...     architecturehintlist="all linux-any")
+
+    >>> pub_linux_all = test_publisher.getPubSource(
+    ...     sourcename='test-buildd-7', version='676',
+    ...     architecturehintlist="linux-all")
+
 Create a PPA source publication.
 
     >>> from lp.registry.interfaces.person import IPersonSet
@@ -107,9 +125,6 @@
     ...         pub, legal_archs, hoary, pas_verify)
     ...     print_architectures(allowed_architectures)
 
-    >>> print_build_architectures(pub_all)
-    i386
-
     >>> print_build_architectures(pub_any)
     hppa
     i386
@@ -121,6 +136,15 @@
     >>> print_build_architectures(pub_one)
     hppa
 
+    >>> print_build_architectures(pub_all)
+    i386
+
+    >>> print_build_architectures(pub_one_and_all)
+    hppa
+
+    >>> print_build_architectures(pub_fictional_and_all)
+    i386
+
 If an architecture is disabled for some reason, then the results from
 determineArchitecturesToBuild() will not include it.
 
@@ -155,6 +179,11 @@
     hppa
     i386
 
+    >>> print_build_architectures(pub_all_linux_any)
+    hppa
+    i386
+
+    >>> print_build_architectures(pub_linux_all)
 
 == PPA architectures ==
 

=== modified file 'lib/lp/soyuz/pas.py'
--- lib/lp/soyuz/pas.py	2010-10-06 11:46:51 +0000
+++ lib/lp/soyuz/pas.py	2010-12-04 08:42:57 +0000
@@ -167,23 +167,30 @@
     legal_arch_tags = set(
         arch.architecturetag for arch in legal_archseries if arch.enabled)
 
-    # We need to support arch tags like any-foo and linux-foo, so remove
-    # supported kernel prefixes, Also allow linux-any but not any-any.
-    # See bugs #73761 and #605002.
-    if hint_string in ['any', 'linux-any']:
+    hint_archs = set(hint_string.split())
+
+    # If a *-any architecture wildcard is present, build for everything
+    # we can. We only support Linux-based architectures at the moment,
+    # and any-any isn't a valid wildcard. See bug #605002.
+    if hint_archs.intersection(('any', 'linux-any')):
         package_tags = legal_arch_tags
-    elif hint_string == 'all':
-        nominated_arch = distroseries.nominatedarchindep
-        legal_archseries_ids = [arch.id for arch in legal_archseries]
-        assert nominated_arch.id in legal_archseries_ids, (
-            'nominatedarchindep is not present in legal_archseries: %s' %
-            ' '.join(legal_arch_tags))
-        package_tags = set([nominated_arch.architecturetag])
     else:
-        my_archs = hint_string.split()
-        for kernel in ['linux', 'any']:
-            my_archs = [arch.replace("%s-" % kernel, "") for arch in my_archs]
-        package_tags = set(my_archs).intersection(legal_arch_tags)
+        # We need to support arch tags like any-foo and linux-foo, so remove
+        # supported kernel prefixes. See bug #73761.
+        stripped_archs = hint_archs
+        for kernel in ('linux', 'any'):
+            stripped_archs = set(
+                arch.replace("%s-" % kernel, "") for arch in stripped_archs)
+        package_tags = stripped_archs.intersection(legal_arch_tags)
+
+        # 'all' is only used as a last resort, to create an arch-indep
+        # build where no builds would otherwise exist.
+        if len(package_tags) == 0 and 'all' in hint_archs:
+            nominated_arch = distroseries.nominatedarchindep
+            assert nominated_arch in legal_archseries, (
+                'nominatedarchindep is not present in legal_archseries: %s' %
+                ' '.join(legal_arch_tags))
+            package_tags = set([nominated_arch.architecturetag])
 
     if pas_verify:
         build_tags = set()