← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~flacoste/launchpad/bug-781993-1 into lp:launchpad

 

Francis J. Lacoste has proposed merging lp:~flacoste/launchpad/bug-781993-1 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~flacoste/launchpad/bug-781993-1/+merge/64258

= Summary =

This is the first branch to fix bug 781993. That fix will allow us to file
bugs against package which have official package branches (no need for a
published package.)

This branch is a simplification of the guessPackageNames() which is
responsible for that check.

== Proposed fix ==

The check to see whether the package was published in the distribution is
done by guessPackageNames. That method returned a tuple of (sourcepackagename,
binarypackagename). Looking at all the call sites, it looks like that all but
one ignored the binarypackagename result. It was used to generated a 'Binary
package hint:' line in the bug description. After checking with the
distribution, that's really a unused feature.

This branch renames guessPackageNames to guessPublishedSourcePackageName() and
only returns the published sourcepackagename in the distribution. Or raises
NotFoundError if there is none.

== Pre-implementation notes ==

Bryce, Martin Pitt and Kate confirmed that the Binary package hint addition to
the bug description was totally noise.

== Implementation details ==

All call sites were updated to use the new method signature. A few call sites
weren't catching the correct exception. (That was fixed.)

Getting rid of the binary package hint allowed us to remove binarypackagename
from CreateBugsParams.

I simplified the logic of guessPackageNames and convert it to use only Storm
syntax.

I've added unit tests to test_distribution.py for the new method. Previous
tests were in a doctest in lp/soyuz/doc/distribution.txt The new code
coverage should be better.

I've added to the factory the possibility to pass strings for
sourcepackagename and binarypackagename. As well as specify the source to use
when creating binary. That simplifies the fixture setup a lot.

== Tests ==

./bin/test.py -vvt
'lp/soyuz/doc/distribution|test_guessPublished|xx-bug-reporting-tools|bugzilla-import'

== Demo and Q/A ==

* Test that you can still file bugs against sourcepackage by using the source
  or binary package name.

* We should probably QA bug watches and debsync but I don't think we have any
  QA insfrastructure for those.

* Once the next branch is completed: file a bug against the mediawiki
  sourcepacakge on the principia distribution (which has an official source
  pacakge branch)

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt
  lib/lp/bugs/browser/bugtarget.py
  lib/lp/bugs/doc/bugzilla-import.txt
  lib/lp/bugs/browser/widgets/bugtask.py
  lib/canonical/launchpad/interfaces/validation.py
  lib/lp/testing/factory.py
  lib/lp/soyuz/doc/distribution.txt
  cronscripts/update-debwatches.py
  lib/lp/bugs/doc/bugtask-package-widget.txt
  lib/lp/registry/interfaces/distribution.py
  lib/lp/app/widgets/launchpadtarget.py
  lib/canonical/launchpad/scripts/debsync.py
  lib/lp/registry/tests/test_distribution.py
  lib/lp/bugs/scripts/bugzilla.py
  lib/lp/registry/model/distribution.py
  lib/lp/bugs/xmlrpc/bug.py
  lib/lp/bugs/interfaces/bug.py

./lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt
       1: narrative uses a moin header.
      57: narrative uses a moin header.
     108: source exceeds 78 characters.
     146: narrative uses a moin header.
     174: narrative uses a moin header.
     231: narrative uses a moin header.
./lib/lp/bugs/doc/bugzilla-import.txt
      11: source has bad indentation.
      15: source has bad indentation.
      21: source has bad indentation.
      58: source exceeds 78 characters.
      65: source has bad indentation.
      67: source has bad indentation.
      81: source has bad indentation.
      88: source has bad indentation.
      93: source has bad indentation.
     107: source has bad indentation.
     117: source has bad indentation.
     123: source has bad indentation.
     129: source has bad indentation.
     183: source has bad indentation.
     198: want exceeds 78 characters.
     215: source has bad indentation.
     222: source has bad indentation.
     238: source has bad indentation.
     241: want exceeds 78 characters.
     246: source has bad indentation.
     260: want exceeds 78 characters.
     284: source has bad indentation.
     300: want exceeds 78 characters.
     330: source has bad indentation.
     346: want exceeds 78 characters.
     374: source has bad indentation.
     393: want exceeds 78 characters.
     418: source has bad indentation.
     442: source has bad indentation.
     466: source has bad indentation.
     500: source has bad indentation.
     514: source has bad indentation.
     518: source has bad indentation.
     522: source has bad indentation.
     523: want exceeds 78 characters.
     526: source has bad indentation.
     538: source has bad indentation.
     543: source has bad indentation.
     548: source has bad indentation.
./lib/canonical/launchpad/interfaces/validation.py
     237: Line exceeds 78 characters.
      98: E302 expected 2 blank lines, found 1
     237: E501 line too long (82 characters)
./lib/lp/testing/factory.py
    3472: W291 trailing whitespace
./cronscripts/update-debwatches.py
      45: Line exceeds 78 characters.
      53: Line exceeds 78 characters.
      72: Line exceeds 78 characters.
      75: Line exceeds 78 characters.
     104: Line exceeds 78 characters.
     135: Line exceeds 78 characters.
     165: Line exceeds 78 characters.
     172: Line exceeds 78 characters.
     196: Line exceeds 78 characters.
     204: Line exceeds 78 characters.
     208: Line exceeds 78 characters.
     163: local variable 'bugmsg' is assigned to but never used
       9: '_pythonpath' imported but unused
      53: E501 line too long (84 characters)
      72: E501 line too long (84 characters)
      75: E501 line too long (83 characters)
     132: W603 '<>' is deprecated, use '!='
     135: W603 '<>' is deprecated, use '!='
     165: E501 line too long (80 characters)
     172: E501 line too long (87 characters)
     196: E501 line too long (80 characters)
     204: E501 line too long (84 characters)
     208: E501 line too long (82 characters)
     216: W603 '<>' is deprecated, use '!='
     231: W391 blank line at end of file
./lib/canonical/launchpad/scripts/debsync.py
     107: local variable 'packagelist' is assigned to but never used
      64: E225 missing whitespace around operator
     162: E225 missing whitespace around operator
     163: E225 missing whitespace around operator
     204: W391 blank line at end of file
./lib/lp/registry/tests/test_distribution.py
     103: local variable 'bpph' is assigned to but never used
     117: local variable 'spph' is assigned to but never used
     146: local variable 'bpph' is assigned to but never used
     162: local variable 'new_bpph' is assigned to but never used
     158: local variable 'old_bpph' is assigned to but never used
./lib/lp/bugs/scripts/bugzilla.py
       8: Line exceeds 78 characters.
      62: E302 expected 2 blank lines, found 1
      69: E302 expected 2 blank lines, found 1
     211: E302 expected 2 blank lines, found 1
     451: E301 expected 1 blank line, found 0
     459: E303 too many blank lines (2)
     620: E301 expected 1 blank line, found 0
./lib/lp/bugs/interfaces/bug.py
     780: E261 at least two spaces before inline comment
     782: E261 at least two spaces before inline comment
-- 
https://code.launchpad.net/~flacoste/launchpad/bug-781993-1/+merge/64258
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~flacoste/launchpad/bug-781993-1 into lp:launchpad.
=== modified file 'cronscripts/update-debwatches.py'
--- cronscripts/update-debwatches.py	2011-05-29 01:36:05 +0000
+++ cronscripts/update-debwatches.py	2011-06-10 21:39:35 +0000
@@ -16,6 +16,7 @@
 from zope.component import getUtility
 
 from canonical.database.constants import UTC_NOW
+from lp.app.error import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.interfaces.bugtask import (
@@ -104,8 +105,9 @@
         # make sure we have updated their status and severity appropriately
         for packagename in debian_bug.packagelist():
             try:
-                srcpkgname, binpkgname = debian.guessPackageNames(packagename)
-            except ValueError:
+                srcpkgname = debian.guessPublishedSourcePackageName(
+                    packagename)
+            except NotFoundError:
                 self.logger.error(sys.exc_value)
                 continue
             search_params = BugTaskSearchParams(user=None, bug=malone_bug,
@@ -114,8 +116,8 @@
             bugtasks = bugtaskset.search(search_params)
             if len(bugtasks) == 0:
                 # we need a new task to link the bug to the debian package
-                self.logger.info('Linking %d and debian %s/%s' % (
-                    malone_bug.id, srcpkgname.name, binpkgname.name))
+                self.logger.info('Linking %d and debian %s' % (
+                    malone_bug.id, srcpkgname.name))
                 # XXX: kiko 2007-02-03:
                 # This code is completely untested and broken.
                 bugtask = malone_bug.addTask(

=== modified file 'lib/canonical/launchpad/interfaces/validation.py'
--- lib/canonical/launchpad/interfaces/validation.py	2011-04-11 02:32:50 +0000
+++ lib/canonical/launchpad/interfaces/validation.py	2011-06-10 21:39:35 +0000
@@ -198,7 +198,8 @@
         # If the distribution has at least one series, check that the
         # source package has been published in the distribution.
         try:
-            distribution.guessPackageNames(sourcepackagename.name)
+            distribution.guessPublishedSourcePackageName(
+                sourcepackagename.name)
         except NotFoundError, e:
             raise LaunchpadValidationError(e)
     new_source_package = distribution.getSourcePackage(sourcepackagename)

=== modified file 'lib/canonical/launchpad/scripts/debsync.py'
--- lib/canonical/launchpad/scripts/debsync.py	2011-05-27 21:12:25 +0000
+++ lib/canonical/launchpad/scripts/debsync.py	2011-06-10 21:39:35 +0000
@@ -17,6 +17,7 @@
 from zope.component import getUtility
 
 from canonical.database.sqlbase import flush_database_updates
+from lp.app.error import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.interfaces.bug import (
     CreateBugParams,
@@ -143,11 +144,11 @@
     # debian_bug.packagelist[0] is going to be a single package name for
     # sure. we work through the package list, try to find one we can
     # work with, otherwise give up
-    srcpkg = binpkg = pkgname = None
+    srcpkg = pkgname = None
     for pkgname in debian_bug.packagelist():
         try:
-            srcpkg, binpkg = ubuntu.guessPackageNames(pkgname)
-        except ValueError:
+            srcpkg = ubuntu.guessPublishedSourcePackageName(pkgname)
+        except NotFoundError:
             logger.error(sys.exc_value)
     if srcpkg is None:
         # none of the package names gave us a source package we can use

=== modified file 'lib/lp/app/widgets/launchpadtarget.py'
--- lib/lp/app/widgets/launchpadtarget.py	2011-05-27 21:12:25 +0000
+++ lib/lp/app/widgets/launchpadtarget.py	2011-06-10 21:39:35 +0000
@@ -135,8 +135,9 @@
                 if package_name is None:
                     return distribution
                 try:
-                    source_name, binary_name = distribution.guessPackageNames(
-                        package_name.name)
+                    source_name = (
+                        distribution.guessPublishedSourcePackageName(
+                            package_name.name))
                 except NotFoundError:
                     raise LaunchpadValidationError(
                         "There is no package name '%s' published in %s"

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2011-06-07 23:20:43 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2011-06-10 21:39:35 +0000
@@ -473,7 +473,7 @@
                     distribution = self.context.distribution
 
                 try:
-                    distribution.guessPackageNames(packagename)
+                    distribution.guessPublishedSourcePackageName(packagename)
                 except NotFoundError:
                     if distribution.series:
                         # If a distribution doesn't have any series,
@@ -590,13 +590,9 @@
             # package name, so let the Soyuz API figure it out for us.
             packagename = str(packagename.name)
             try:
-                sourcepackagename, binarypackagename = (
-                    context.guessPackageNames(packagename))
+                sourcepackagename = context.guessPublishedSourcePackageName(
+                    packagename)
             except NotFoundError:
-                # guessPackageNames may raise NotFoundError. It would be
-                # nicer to allow people to indicate a package even if
-                # never published, but the quick fix for now is to note
-                # the issue and move on.
                 notifications.append(
                     "The package %s is not published in %s; the "
                     "bug was targeted only to the distribution."
@@ -608,7 +604,6 @@
                         packagename, context.displayname))
             else:
                 context = context.getSourcePackage(sourcepackagename.name)
-                params.binarypackagename = binarypackagename
 
         extra_data = self.extra_data
         if extra_data.extra_description:

=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
--- lib/lp/bugs/browser/widgets/bugtask.py	2011-06-02 21:31:51 +0000
+++ lib/lp/bugs/browser/widgets/bugtask.py	2011-06-10 21:39:35 +0000
@@ -496,7 +496,7 @@
         distribution = self.getDistribution()
 
         try:
-            source, binary = distribution.guessPackageNames(input)
+            source = distribution.guessPublishedSourcePackageName(input)
         except NotFoundError:
             try:
                 return self.convertTokensToValues([input])[0]

=== modified file 'lib/lp/bugs/doc/bugtask-package-widget.txt'
--- lib/lp/bugs/doc/bugtask-package-widget.txt	2011-01-11 11:51:27 +0000
+++ lib/lp/bugs/doc/bugtask-package-widget.txt	2011-06-10 21:39:35 +0000
@@ -49,13 +49,13 @@
     u'linux-source-2.6.15'
 
 For some distribution we don't know exactly which source packages it
-contains, so IDistribution.guessPackageNames will return a
+contains, so IDistribution.guessPublishedSourcePackageName will raise a
 NotFoundError.
 
     >>> debian_task = bug_one.bugtasks[-1]
     >>> debian_task.distribution.name
     u'debian'
-    >>> debian_task.distribution.guessPackageNames('evolution')
+    >>> debian_task.distribution.guessPublishedSourcePackageName('evolution')
     Traceback (most recent call last):
     ...
     NotFoundError...

=== modified file 'lib/lp/bugs/doc/bugzilla-import.txt'
--- lib/lp/bugs/doc/bugzilla-import.txt	2011-05-27 19:53:20 +0000
+++ lib/lp/bugs/doc/bugzilla-import.txt	2011-06-10 21:39:35 +0000
@@ -232,12 +232,14 @@
    effect of the import, because they are subscribed to the bug.
  * The "RESOLVED WONTFIX" status is converted to a status of INVALID.
  * The fact that the "unknown" package does not exist in Ubuntu has
-   been logged, along with the exception raised by guessPackageNames().
+   been logged, along with the exception raised by
+   guessPublishedSourcePackageName().
 
   >>> print getUtility(IPersonSet).getByEmail('new.user@xxxxxxxxxxxxx')
   None
   >>> bug = bz.handleBug(2)
-  WARNING:lp.bugs.scripts.bugzilla:could not find package name for "unknown": 'Unknown package: unknown'
+  WARNING:lp.bugs.scripts.bugzilla:could not find package name for "unknown":
+  'Unknown package: unknown'
   >>> import transaction
   >>> transaction.commit()
 

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2011-05-24 08:58:38 +0000
+++ lib/lp/bugs/interfaces/bug.py	2011-06-10 21:39:35 +0000
@@ -98,7 +98,7 @@
 
     def __init__(self, owner, title, comment=None, description=None, msg=None,
                  status=None, datecreated=None, security_related=False,
-                 private=False, subscribers=(), binarypackagename=None,
+                 private=False, subscribers=(),
                  tags=None, subscribe_owner=True, filed_by=None,
                  importance=None, milestone=None, assignee=None):
         self.owner = owner
@@ -114,7 +114,6 @@
         self.product = None
         self.distribution = None
         self.sourcepackagename = None
-        self.binarypackagename = binarypackagename
         self.tags = tags
         self.subscribe_owner = subscribe_owner
         self.filed_by = filed_by
@@ -1129,9 +1128,6 @@
 
           * if either product or distribution is specified, an appropiate
             bug task will be created
-
-          * binarypackagename, if not None, will be added to the bug's
-            description
         """
 
     def createBugWithoutTarget(bug_params):

=== modified file 'lib/lp/bugs/scripts/bugzilla.py'
--- lib/lp/bugs/scripts/bugzilla.py	2011-05-27 21:12:25 +0000
+++ lib/lp/bugs/scripts/bugzilla.py	2011-06-10 21:39:35 +0000
@@ -369,8 +369,8 @@
 
         return person
 
-    def _getPackageNames(self, bug):
-        """Returns the source and binary package names for the given bug."""
+    def _getPackageName(self, bug):
+        """Returns the source package name for the given bug."""
         # we currently only support mapping Ubuntu bugs ...
         if bug.product != 'Ubuntu':
             raise AssertionError('product must be Ubuntu')
@@ -389,19 +389,17 @@
             pkgname = bug.component.encode('ASCII')
 
         try:
-            srcpkg, binpkg = self.ubuntu.guessPackageNames(pkgname)
+            return self.ubuntu.guessPublishedSourcePackageName(pkgname)
         except NotFoundError, e:
             logger.warning('could not find package name for "%s": %s',
                            pkgname, str(e))
-            srcpkg = binpkg = None
-
-        return srcpkg, binpkg
+            return None
 
     def getLaunchpadBugTarget(self, bug):
         """Returns a dictionary of arguments to createBug() that correspond
         to the given bugzilla bug.
         """
-        srcpkg, binpkg = self._getPackageNames(bug)
+        srcpkg = self._getPackageName(bug)
         return {
             'distribution': self.ubuntu,
             'sourcepackagename': srcpkg,
@@ -435,7 +433,7 @@
         This function relies on the package -> product linkage having been
         entered in advance.
         """
-        srcpkgname, binpkgname = self._getPackageNames(bug)
+        srcpkgname = self._getPackageName(bug)
         # find a product series
         series = None
         for series in self.ubuntu.series:

=== modified file 'lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt'
--- lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt	2011-04-20 12:59:55 +0000
+++ lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt	2011-06-10 21:39:35 +0000
@@ -121,12 +121,6 @@
     >>> user_browser.url
     'http://bugs.launchpad.dev/ubuntu/+source/mozilla-firefox/+bug/...'
 
-Some extra text was appended to the description.
-
-    >>> find_tag_by_id(
-    ...     user_browser.contents, 'edit-description').renderContents()
-    '...<p>Binary package hint: mozilla-firefox</p>\n<p>A bug description...'
-
 Two attachments were added.
 
     >>> attachment_portlet = find_portlet(

=== modified file 'lib/lp/bugs/xmlrpc/bug.py'
--- lib/lp/bugs/xmlrpc/bug.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/xmlrpc/bug.py	2011-06-10 21:39:35 +0000
@@ -70,7 +70,8 @@
 
             if package:
                 try:
-                    spname, bpname = distro_object.guessPackageNames(package)
+                    spname = distro_object.guessPublishedSourcePackageName(
+                        package)
                 except NotFoundError:
                     return faults.NoSuchPackage(package)
 

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2011-06-03 09:31:08 +0000
+++ lib/lp/registry/interfaces/distribution.py	2011-06-10 21:39:35 +0000
@@ -567,14 +567,17 @@
         Raises NotFoundError if it fails to find the named file.
         """
 
-    def guessPackageNames(pkgname):
-        """Try and locate source and binary package name objects that
-        are related to the provided name --  which could be either a
-        source or a binary package name. Returns a tuple of
-        (sourcepackagename, binarypackagename) based on the current
-        publishing status of these binary / source packages. Raises
-        NotFoundError if it fails to find any package published with
-        that name in the distribution.
+    def guessPublishedSourcePackageName(pkgname):
+        """Return the "published" SourcePackageName related to pkgname.
+
+        If pkgname is corresponds to a source package that was published in
+        any of the distribution series.
+
+        Otherwise, try to find a published binary package name and then return
+        the source package name from which it comes from.
+
+        :raises NotFoundError: when pkgname doesn't correspond to either a
+            published source or binary package name in this distribution.
         """
 
     def getAllPPAs():

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2011-06-08 23:31:41 +0000
+++ lib/lp/registry/model/distribution.py	2011-06-10 21:39:35 +0000
@@ -1370,7 +1370,7 @@
         # results will only see DSPCs
         return DecoratedResultSet(results, result_to_dspc)
 
-    def guessPackageNames(self, pkgname):
+    def guessPublishedSourcePackageName(self, pkgname):
         """See `IDistribution`"""
         assert isinstance(pkgname, basestring), (
             "Expected string. Got: %r" % pkgname)
@@ -1386,78 +1386,26 @@
                                 'published in it'
                                 % (self.displayname, pkgname))
 
-        # The way this method works is that is tries to locate a pair
-        # of packages related to that name. If it locates a source
-        # package it then tries to see if it has been published at any
-        # point, and gets the binary package from the publishing
-        # record.
-        #
-        # If that fails (no source package by that name, or not
-        # published) then it'll search binary packages, then find the
-        # source package most recently associated with it, first in
-        # the current distroseries and then across the whole
-        # distribution.
-        #
-        # XXX kiko 2006-07-28:
-        # Note that the strategy of falling back to previous
-        # distribution series might be revisited in the future; for
-        # instance, when people file bugs, it might actually be bad for
-        # us to allow them to be associated with obsolete packages.
-
-        bpph_location_clauses = [
-            DistroSeries.distribution == self,
-            DistroArchSeries.distroseriesID == DistroSeries.id,
-            BinaryPackagePublishingHistory.distroarchseriesID ==
-                DistroArchSeries.id,
-            BinaryPackagePublishingHistory.archiveID.is_in(
-                self.all_distro_archive_ids),
-            BinaryPackagePublishingHistory.dateremoved == None,
-            BinaryPackageRelease.id ==
-                BinaryPackagePublishingHistory.binarypackagereleaseID,
-            ]
-
         sourcepackagename = SourcePackageName.selectOneBy(name=pkgname)
         if sourcepackagename:
             # Note that in the source package case, we don't restrict
             # the search to the distribution release, making a best
             # effort to find a package.
-            publishing = SourcePackagePublishingHistory.selectFirst('''
-                SourcePackagePublishingHistory.distroseries =
-                    DistroSeries.id AND
-                DistroSeries.distribution = %s AND
-                SourcePackagePublishingHistory.archive IN %s AND
-                SourcePackagePublishingHistory.sourcepackagerelease =
-                    SourcePackageRelease.id AND
-                SourcePackageRelease.sourcepackagename = %s AND
-                SourcePackagePublishingHistory.status IN %s
-                ''' % sqlvalues(self,
-                                self.all_distro_archive_ids,
-                                sourcepackagename,
-                                (PackagePublishingStatus.PUBLISHED,
-                                 PackagePublishingStatus.PENDING)),
-                clauseTables=['SourcePackageRelease', 'DistroSeries'],
-                distinct=True,
-                orderBy="id")
+            publishing = IStore(SourcePackagePublishingHistory).find(
+                SourcePackagePublishingHistory,
+                SourcePackagePublishingHistory.archiveID == Archive.id,
+                Archive.distribution == self,
+                Archive.purpose.is_in(MAIN_ARCHIVE_PURPOSES),
+                SourcePackagePublishingHistory.sourcepackagereleaseID ==
+                    SourcePackageRelease.id,
+                SourcePackageRelease.sourcepackagename == sourcepackagename,
+                SourcePackagePublishingHistory.status.is_in(
+                    (PackagePublishingStatus.PUBLISHED,
+                     PackagePublishingStatus.PENDING)
+                    )).order_by(
+                        Desc(SourcePackagePublishingHistory.id)).first()
             if publishing is not None:
-                # Attempt to find a published binary package of the
-                # same name.
-                bpph = IStore(BinaryPackagePublishingHistory).find(
-                    BinaryPackagePublishingHistory,
-                    BinaryPackageRelease.binarypackagename ==
-                        BinaryPackageName.id,
-                    BinaryPackageName.name == sourcepackagename.name,
-                    BinaryPackageBuild.id == BinaryPackageRelease.buildID,
-                    SourcePackageRelease.id ==
-                        BinaryPackageBuild.source_package_release_id,
-                    SourcePackageRelease.sourcepackagename ==
-                        sourcepackagename,
-                    *bpph_location_clauses).any()
-                if bpph is not None:
-                    bpr = bpph.binarypackagerelease
-                    return (sourcepackagename, bpr.binarypackagename)
-                # No binary with a similar name, so just return None
-                # rather than returning some arbitrary binary package.
-                return (sourcepackagename, None)
+                return sourcepackagename
 
         # At this point we don't have a published source package by
         # that name, so let's try to find a binary package and work
@@ -1470,12 +1418,18 @@
             # the sourcepackagename from that.
             bpph = IStore(BinaryPackagePublishingHistory).find(
                 BinaryPackagePublishingHistory,
+                BinaryPackagePublishingHistory.archiveID == Archive.id,
+                Archive.distribution == self,
+                Archive.purpose.is_in(MAIN_ARCHIVE_PURPOSES),
+                BinaryPackagePublishingHistory.binarypackagereleaseID ==
+                    BinaryPackageRelease.id,
                 BinaryPackageRelease.binarypackagename == binarypackagename,
-                *bpph_location_clauses).order_by(
+                BinaryPackagePublishingHistory.dateremoved == None,
+                ).order_by(
                     Desc(BinaryPackagePublishingHistory.id)).first()
             if bpph is not None:
                 spr = bpph.binarypackagerelease.build.source_package_release
-                return (spr.sourcepackagename, binarypackagename)
+                return spr.sourcepackagename
 
         # We got nothing so signal an error.
         if sourcepackagename is None:

=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py	2011-05-14 15:02:13 +0000
+++ lib/lp/registry/tests/test_distribution.py	2011-06-10 21:39:35 +0000
@@ -7,6 +7,7 @@
 
 from lazr.lifecycle.snapshot import Snapshot
 import soupmatchers
+from testtools import ExpectedException
 from testtools.matchers import (
     MatchesAny,
     Not,
@@ -14,11 +15,13 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from canonical.database.constants import UTC_NOW
 from canonical.launchpad.webapp import canonical_url
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
+from lp.app.errors import NotFoundError
 from lp.registry.errors import NoSuchDistroSeries
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.person import IPersonSet
@@ -41,9 +44,6 @@
 
     layer = DatabaseFunctionalLayer
 
-    def setUp(self):
-        super(TestDistribution, self).setUp('foo.bar@xxxxxxxxxxxxx')
-
     def test_distribution_repr_ansii(self):
         # Verify that ANSI displayname is ascii safe.
         distro = self.factory.makeDistribution(
@@ -59,6 +59,115 @@
         ignore, displayname, name = repr(distro).rsplit(' ', 2)
         self.assertEqual("'\\u0170-distro'", displayname)
 
+    def test_guessPublishedSourcePackageName_no_distro_series(self):
+        # Distribution without a series raises NotFoundError
+        distro = self.factory.makeDistribution()
+        with ExpectedException(NotFoundError, '.*has no series.*'):
+            distro.guessPublishedSourcePackageName('package')
+
+    def test_guessPublishedSourcePackageName_invalid_name(self):
+        # Invalid name raises a NotFoundError
+        distro = self.factory.makeDistribution()
+        with ExpectedException(NotFoundError, "'Invalid package name.*"):
+            distro.guessPublishedSourcePackageName('a*package')
+
+    def test_guessPublishedSourcePackageName_nothing_published(self):
+        distroseries = self.factory.makeDistroSeries()
+        with ExpectedException(NotFoundError, "'Unknown package:.*"):
+            distroseries.distribution.guessPublishedSourcePackageName(
+                'a-package')
+
+    def test_guessPublishedSourcePackageName_ignored_removed(self):
+        # Removed binary package are ignored.
+        distroseries = self.factory.makeDistroSeries()
+        self.factory.makeBinaryPackagePublishingHistory(
+            archive=distroseries.main_archive,
+            binarypackagename='binary-package', dateremoved=UTC_NOW)
+        with ExpectedException(NotFoundError, ".*Binary package.*"):
+            distroseries.distribution.guessPublishedSourcePackageName(
+                'binary-package')
+
+    def test_guessPublishedSourcePackageName_sourcepackage_name(self):
+        distroseries = self.factory.makeDistroSeries()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=distroseries, sourcepackagename='my-package')
+        self.assertEquals(
+            spph.sourcepackagerelease.sourcepackagename,
+            distroseries.distribution.guessPublishedSourcePackageName(
+                'my-package'))
+
+    def test_guessPublishedSourcePackageName_binarypackage_name(self):
+        distroseries = self.factory.makeDistroSeries()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=distroseries, sourcepackagename='my-package')
+        bpph = self.factory.makeBinaryPackagePublishingHistory(
+            archive=distroseries.main_archive,
+            binarypackagename='binary-package',
+            source_package_release=spph.sourcepackagerelease)
+        self.assertEquals(
+            spph.sourcepackagerelease.sourcepackagename,
+            distroseries.distribution.guessPublishedSourcePackageName(
+                'binary-package'))
+
+    def test_guessPublishedSourcePackageName_exlude_ppa(self):
+        # Package published in PPAs are not considered to be part of the
+        # distribution.
+        distroseries = self.factory.makeUbuntuDistroSeries()
+        ppa_archive = self.factory.makeArchive()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=distroseries, sourcepackagename='my-package',
+            archive=ppa_archive)
+        with ExpectedException(NotFoundError, ".*not published in.*"):
+            distroseries.distribution.guessPublishedSourcePackageName(
+                'my-package')
+
+    def test_guessPublishedSourcePackageName_exlude_other_distro(self):
+        # Published source package are only found in the distro
+        # in which they were published.
+        distroseries1 = self.factory.makeDistroSeries()
+        distroseries2 = self.factory.makeDistroSeries()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=distroseries1, sourcepackagename='my-package')
+        self.assertEquals(
+            spph.sourcepackagerelease.sourcepackagename,
+            distroseries1.distribution.guessPublishedSourcePackageName(
+                'my-package'))
+        with ExpectedException(NotFoundError, ".*not published in.*"):
+            distroseries2.distribution.guessPublishedSourcePackageName(
+                'my-package')
+
+    def test_guessPublishedSourcePackageName_looks_for_source_first(self):
+        # If both a binary and source package name shares the same name,
+        # the source package will be returned (and the one from the unrelated
+        # binary).
+        distroseries = self.factory.makeDistroSeries()
+        my_spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=distroseries, sourcepackagename='my-package')
+        bpph = self.factory.makeBinaryPackagePublishingHistory(
+            archive=distroseries.main_archive,
+            binarypackagename='my-package', sourcepackagename='other-package')
+        self.assertEquals(
+            my_spph.sourcepackagerelease.sourcepackagename,
+            distroseries.distribution.guessPublishedSourcePackageName(
+                'my-package'))
+
+    def test_guessPublishedSourcePackageName_uses_latest(self):
+        # If multiple binaries match, it will return the source of the latest
+        # one published.
+        distroseries = self.factory.makeDistroSeries()
+        old_bpph = self.factory.makeBinaryPackagePublishingHistory(
+            archive=distroseries.main_archive,
+            sourcepackagename='old-source-name',
+            binarypackagename='my-package')
+        new_bpph = self.factory.makeBinaryPackagePublishingHistory(
+            archive=distroseries.main_archive,
+            sourcepackagename='new-source-name',
+            binarypackagename='my-package')
+        self.assertEquals(
+            'new-source-name',
+            distroseries.distribution.guessPublishedSourcePackageName(
+                'my-package').name)
+
 
 class TestDistributionCurrentSourceReleases(
     TestDistroSeriesCurrentSourceReleases):

=== modified file 'lib/lp/soyuz/doc/distribution.txt'
--- lib/lp/soyuz/doc/distribution.txt	2011-05-27 19:53:20 +0000
+++ lib/lp/soyuz/doc/distribution.txt	2011-06-10 21:39:35 +0000
@@ -5,62 +5,17 @@
 objects for the distribution.
 
 
-Guessing package names
-----------------------
-
-IDistribution allows us to retrieve packages by name, returning a tuple
-of Source/BinaryPackageName instances published within this
-distribution:
-
     >>> from lp.registry.interfaces.distribution import IDistributionSet
     >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
-    >>> from lp.registry.interfaces.sourcepackagename import ISourcePackageName
     >>> from lp.soyuz.enums import PackagePublishingStatus
-    >>> from lp.soyuz.interfaces.binarypackagename import IBinaryPackageName
-
-    >>> distroset = getUtility(IDistributionSet)
-    >>> gentoo = distroset.getByName("gentoo")
-    >>> ubuntu = distroset.get(1)
-
-    >>> source_name, bin_name = ubuntu.guessPackageNames('pmount')
-    >>> ISourcePackageName.providedBy(source_name)
-    True
-
-    >>> IBinaryPackageName.providedBy(bin_name)
-    True
-
-    >>> source_name.name, bin_name.name
-    (u'pmount', u'pmount')
-
-Prevents wrong usage by and assertion error:
-
-    >>> name_tuple = ubuntu.guessPackageNames(ubuntu)
-    Traceback (most recent call last):
-    ...
-    AssertionError: Expected string. Got: <Distribution ...>
-
-Raises NotFoundError for following conditions:
-
-    >>> name_tuple = ubuntu.guessPackageNames('@#$')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: 'Invalid package name: @#$'
-
-    >>> name_tuple = ubuntu.guessPackageNames('zeca')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: 'Unknown package: zeca'
-
-    >>> name_tuple = ubuntu.guessPackageNames('1234')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: 'Unknown package: 1234'
-
-Packages only published in PPAs will not be found in the Ubuntu archive.
-Here, 'at' is published in mark's PPA only:
+
+    >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
+    >>> debian = getUtility(IDistributionSet).getByName('debian')
+
+(Create some data that is depended upon by later tests. It was part of a
+test "narrative" that was converted to unit tests.... for obvious reasons.)
 
     >>> from lp.soyuz.tests.ppa import publishToPPA
-    >>> ubuntutest = distroset.getByName("ubuntutest")
     >>> publishToPPA(
     ...     person_name='cprov',
     ...     sourcepackage_name='at', sourcepackage_version='0.00',
@@ -68,131 +23,6 @@
     ...     distribution_name='ubuntutest',
     ...     distroseries_name='hoary-test',
     ...     publishing_status=PackagePublishingStatus.PUBLISHED)
-    >>> name_tuple = ubuntutest.guessPackageNames('at')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: u'Package at not published in ubuntutest'
-
-It also raises NotFoundError on distributions with no series:
-
-    >>> source_name, bin_name = gentoo.guessPackageNames('pmount')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: u"Gentoo has no series; 'pmount' was never published in it"
-
-A distroseries can only be created by the distro owner or the admin
-team.
-
-    >>> gentoo.newSeries('gentoo-two', 'Gentoo Two',
-    ...                  'Gentoo Two Dot Oh', 'Gentoo 2', 'G2',
-    ...                  '2.0', None, gentoo.owner)
-    Traceback (most recent call last):
-    ...
-    Unauthorized: (<Distribution...>, 'newSeries', 'launchpad.Moderate')
-
-    >>> login('mark@xxxxxxxxxxx')
-    >>> from lp.registry.interfaces.distroseries import IDistroSeriesSet
-    >>> distroseriesset = getUtility(IDistroSeriesSet)
-    >>> gentoo_two = gentoo.newSeries('gentoo-two', 'Gentoo Two',
-    ...                               'Gentoo Two Dot Oh', 'Gentoo 2', 'G2',
-    ...                               '2.0', None, gentoo.owner)
-
-    # Reverting the logged in user.
-
-    >>> login(ANONYMOUS)
-
-Even if we add a series to Gentoo, no packages have ever been published
-in it, and therefore guessPackageNames will still fail:
-
-    >>> source_name, bin_name = gentoo.guessPackageNames('pmount')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: u'Package pmount not published in Gentoo'
-
-It will find packages that are at the PENDING publishing state in
-addition to PUBLISHED ones:
-
-    >>> login("admin@xxxxxxxxxxxxx")
-    >>> from lp.soyuz.tests.test_publishing import (
-    ...     SoyuzTestPublisher)
-    >>> test_publisher = SoyuzTestPublisher()
-    >>> ignore = test_publisher.setUpDefaultDistroSeries(
-    ...     ubuntu['breezy-autotest'])
-    >>> ignore = test_publisher.getPubSource(
-    ...     sourcename="pendingpackage",
-    ...     status=PackagePublishingStatus.PENDING)
-    >>> login(ANONYMOUS)
-    >>> (source, binary) = ubuntu.guessPackageNames("pendingpackage")
-    >>> print source.name
-    pendingpackage
-
-It also works if we look for a package name which is the name of both
-binary and source packages but for which only the source is published:
-
-    >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
-    >>> debian = getUtility(ILaunchpadCelebrities).debian
-    >>> source_name, bin_name = debian.guessPackageNames('alsa-utils')
-    >>> print bin_name
-    None
-
-    >>> source_name.name
-    u'alsa-utils'
-
-It's possible for a binary package to have the same name as a source
-package, yet not be derived from that source package. In this case, we
-want to prefer the source package with that name.
-
-First, we need a function to help testing:
-
-    >>> def print_guessed_names(package_name):
-    ...     source, binary = ubuntu.guessPackageNames(package_name)
-    ...     print "source: %r" % source.name
-    ...     print "binary: %r" % getattr(binary, 'name', None)
-
-Note that source packages can produces lots of differently named binary
-packages so only return a match if it's got the same name as the source
-package rather than returning an arbitrary binary package:
-
-Both iceweasel and mozilla-firefox source packages produce mozilla-
-firefox binary packages.
-
-    >>> print_guessed_names('mozilla-firefox')
-    source: u'mozilla-firefox'
-    binary: u'mozilla-firefox'
-
-    >>> print_guessed_names('iceweasel')
-    source: u'iceweasel'
-    binary: None
-
-If we don't get a hit on the source package we search binary packages.
-Because there is a many to one relationship from binary packages to
-source packages we can always return a source package name even if it
-differs:
-
-    >>> print_guessed_names('linux-2.6.12')
-    source: u'linux-source-2.6.15'
-    binary: u'linux-2.6.12'
-
-If there are multiple matching binary packages, the source of the latest
-publication is used. If we create a new 'linux' source with a 'linux-2.6.12'
-binary, 'linux' will be returned instead of 'linux-source-2.6.15'.
-
-    >>> from lp.soyuz.tests.test_publishing import (
-    ...     SoyuzTestPublisher)
-    >>> test_publisher = SoyuzTestPublisher()
-    >>> hoary = test_publisher.setUpDefaultDistroSeries(
-    ...     ubuntu.getSeries('hoary'))
-    >>> fake_chroot = test_publisher.addMockFile('fake_chroot.tar.gz')
-    >>> unused = hoary['i386'].addOrUpdateChroot(fake_chroot)
-    >>> login('admin@xxxxxxxxxxxxx')
-    >>> test_publisher.getPubBinaries(
-    ...     'linux-2.6.12', architecturespecific=True)
-    [<BinaryPackagePublishingHistory ...>]
-    >>> login(ANONYMOUS)
-
-    >>> print_guessed_names('linux-2.6.12')
-    source: u'linux'
-    binary: u'linux-2.6.12'
 
 
 Handling Personal Package Archives

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-06-10 19:46:57 +0000
+++ lib/lp/testing/factory.py	2011-06-10 21:39:35 +0000
@@ -3424,8 +3424,10 @@
                 distribution=distroseries.distribution,
                 purpose=ArchivePurpose.PRIMARY)
 
-        if sourcepackagename is None:
-            sourcepackagename = self.makeSourcePackageName()
+        if (sourcepackagename is None or
+            isinstance(sourcepackagename, basestring)):
+            sourcepackagename = self.getOrMakeSourcePackageName(
+                sourcepackagename)
 
         if component is None:
             component = self.makeComponent()
@@ -3492,13 +3494,16 @@
 
     def makeBinaryPackageBuild(self, source_package_release=None,
             distroarchseries=None, archive=None, builder=None,
-            status=None, pocket=None, date_created=None, processor=None):
+            status=None, pocket=None, date_created=None, processor=None,
+            sourcepackagename=None):
         """Create a BinaryPackageBuild.
 
         If archive is not supplied, the source_package_release is used
         to determine archive.
         :param source_package_release: The SourcePackageRelease this binary
             build uses as its source.
+        :param sourcepackagename: when source_package_release is None, the
+            sourcepackagename from which the build will come. 
         :param distroarchseries: The DistroArchSeries to use.
         :param archive: The Archive to use.
         :param builder: An optional builder to assign.
@@ -3525,7 +3530,8 @@
             multiverse = self.makeComponent(name='multiverse')
             source_package_release = self.makeSourcePackageRelease(
                 archive, component=multiverse,
-                distroseries=distroarchseries.distroseries)
+                distroseries=distroarchseries.distroseries,
+                sourcepackagename=sourcepackagename)
             self.makeSourcePackagePublishingHistory(
                 distroseries=source_package_release.upload_distroseries,
                 archive=archive, sourcepackagerelease=source_package_release,
@@ -3623,12 +3629,15 @@
         return spph
 
     def makeBinaryPackagePublishingHistory(self, binarypackagerelease=None,
+                                           binarypackagename=None,
                                            distroarchseries=None,
                                            component=None, section_name=None,
                                            priority=None, status=None,
                                            scheduleddeletiondate=None,
                                            dateremoved=None,
-                                           pocket=None, archive=None):
+                                           pocket=None, archive=None,
+                                           source_package_release=None,
+                                           sourcepackagename=None):
         """Make a `BinaryPackagePublishingHistory`."""
         if distroarchseries is None:
             if archive is None:
@@ -3658,8 +3667,10 @@
             # in the same archive and suite.
             binarypackagebuild = self.makeBinaryPackageBuild(
                 archive=archive, distroarchseries=distroarchseries,
-                pocket=pocket)
+                pocket=pocket, source_package_release=source_package_release,
+                sourcepackagename=sourcepackagename)
             binarypackagerelease = self.makeBinaryPackageRelease(
+                binarypackagename=binarypackagename,
                 build=binarypackagebuild,
                 component=component,
                 section_name=section_name,
@@ -3723,8 +3734,10 @@
         """Make a `BinaryPackageRelease`."""
         if build is None:
             build = self.makeBinaryPackageBuild()
-        if binarypackagename is None:
-            binarypackagename = self.makeBinaryPackageName()
+        if (binarypackagename is None or
+            isinstance(binarypackagename, basestring)):
+            binarypackagename = self.getOrMakeBinaryPackageName(
+                binarypackagename)
         if version is None:
             version = build.source_package_release.version
         if binpackageformat is None:


Follow ups