Merge lp:~abentley/bzr/find-proposal-revid into lp:bzr

[Aaron Bentley]

This branch changes the lp-find-proposal logic to use only the revision-id (not
revno and branch) to look up merge proposals.

Since the previous version used the branch in and revno API requests, it meant that lp-find-proposal only worked when the correct branch was specified. This way, it works no matter how the revision-id is determined.

The old code also required that the revision was on the mainline of the current branch, so for non-mainline revisions, it found the mainline revision that merged them. This code doesn't do that, but the user can prefix their revision spec with "mainline:" to restore that behaviour if it is desirable.

[Martin Packman (gz)]

Nice simplification. Wants a news entry given the slight behaviour change I'd say.

+ version='devel')

If there's ever going to be another 'release' of the launchpad api, a comment here saying "getMergeProposals(merged_revision=) not in 1.0" would be good.

review: Approve

=== modified file 'bzrlib/plugins/launchpad/cmds.py'
--- bzrlib/plugins/launchpad/cmds.py	2012-03-12 14:35:26 +0000
+++ bzrlib/plugins/launchpad/cmds.py	2012-07-23 19:21:21 +0000
@@ -28,7 +28,6 @@
     )
 from bzrlib.errors import (
     BzrCommandError,
-    InvalidRevisionSpec,
     InvalidURL,
     NoPublicBranch,
     NotBranchError,
@@ -335,16 +334,15 @@
     __doc__ = """Find the proposal to merge this revision.
 
     Finds the merge proposal(s) that discussed landing the specified revision.
-    This works only if the selected branch was the merge proposal target, and
-    if the merged_revno is recorded for the merge proposal.  The proposal(s)
-    are opened in a web browser.
+    This works only if the if the merged_revno was recorded for the merge
+    proposal.  The proposal(s) are opened in a web browser.
 
-    Any revision involved in the merge may be specified-- the revision in
-    which the merge was performed, or one of the revisions that was merged.
+    Only the revision specified is searched for.  To find the mainline
+    revision that merged it into mainline, use the "mainline" revision spec.
 
     So, to find the merge proposal that reviewed line 1 of README::
 
-      bzr lp-find-proposal -r annotate:README:1
+      bzr lp-find-proposal -r mainline:annotate:README:1
     """
 
     takes_options = ['revision']
@@ -357,8 +355,11 @@
         pb = ui.ui_factory.nested_progress_bar()
         b.lock_read()
         try:
-            revno = self._find_merged_revno(revision, b, pb)
-            merged = self._find_proposals(revno, b, pb)
+            if revision is None:
+                revision_id = b.last_revision()
+            else:
+                revision_id = revision[0].as_revision_id(b)
+            merged = self._find_proposals(revision_id, pb)
             if len(merged) == 0:
                 raise BzrCommandError(gettext('No review found.'))
             trace.note(gettext('%d proposals(s) found.') % len(merged))
@@ -368,43 +369,11 @@
             b.unlock()
             pb.finished()
 
-    def _find_merged_revno(self, revision, b, pb):
-        if revision is None:
-            return b.revno()
-        pb.update(gettext('Finding revision-id'))
-        revision_id = revision[0].as_revision_id(b)
-        # a revno spec is necessarily on the mainline.
-        if self._is_revno_spec(revision[0]):
-            merging_revision = revision_id
-        else:
-            graph = b.repository.get_graph()
-            pb.update(gettext('Finding merge'))
-            merging_revision = graph.find_lefthand_merger(
-                revision_id, b.last_revision())
-            if merging_revision is None:
-                raise InvalidRevisionSpec(revision[0].user_spec, b)
-        pb.update(gettext('Finding revno'))
-        return b.revision_id_to_revno(merging_revision)
-
-    def _find_proposals(self, revno, b, pb):
+    def _find_proposals(self, revision_id, pb):
         from bzrlib.plugins.launchpad import (lp_api, lp_registration)
-        launchpad = lp_api.login(lp_registration.LaunchpadService())
-        pb.update(gettext('Finding Launchpad branch'))
-        lpb = lp_api.LaunchpadBranch.from_bzr(launchpad, b,
-                                              create_missing=False)
+        # "devel" because branches.getMergeProposals is not part of 1.0 API.
+        launchpad = lp_api.login(lp_registration.LaunchpadService(),
+                                 version='devel')
         pb.update(gettext('Finding proposals'))
-        return list(lpb.lp.getMergeProposals(status=['Merged'],
-                                             merged_revnos=[revno]))
-
-
-    @staticmethod
-    def _is_revno_spec(spec):
-        try:
-            int(spec.user_spec)
-        except ValueError:
-            return False
-        else:
-            return True
-
-
-
+        return list(launchpad.branches.getMergeProposals(
+                    merged_revision=revision_id))
 
=== modified file 'bzrlib/plugins/launchpad/lp_api.py'
--- bzrlib/plugins/launchpad/lp_api.py	2012-07-23 19:21:20 +0000
+++ bzrlib/plugins/launchpad/lp_api.py	2012-07-23 19:21:21 +0000
@@ -112,7 +112,8 @@
         errors.BzrError.__init__(self, branch=branch, url=branch.base)
 
 
-def login(service, timeout=None, proxy_info=None):
+def login(service, timeout=None, proxy_info=None,
+          version=Launchpad.DEFAULT_VERSION):
     """Log in to the Launchpad API.
 
     :return: The root `Launchpad` object from launchpadlib.
@@ -120,7 +121,7 @@
     cache_directory = get_cache_directory()
     launchpad = Launchpad.login_with(
         'bzr', _get_api_url(service), cache_directory, timeout=timeout,
-        proxy_info=proxy_info)
+        proxy_info=proxy_info, version=version)
     # XXX: Work-around a minor security bug in launchpadlib < 1.6.3, which
     # would create this directory with default umask.
     osutils.chmod_if_possible(cache_directory, 0700)
 
=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt	2012-07-19 18:39:38 +0000
+++ doc/en/release-notes/bzr-2.6.txt	2012-07-23 19:21:21 +0000
@@ -24,8 +24,9 @@
 Improvements
 ************
 
-.. Improvements to existing commands, especially improved performance 
-   or memory usage, or better results.
+``bzr lp-find-proposal`` now only cares about the revision-id that is
+specified, not the branch you use.  This was enabled by a new API call in
+Launchpad's web service.  (Aaron Bentley)
 
 Bug Fixes
 *********
@@ -47,8 +48,8 @@
 Internals
 *********
 
-.. Major internal changes, unlikely to be visible to users or plugin 
-   developers, but interesting for bzr developers.
+* The launchpad plugin now requires API 1.6.0 or later.  This version shipped
+  with Ubuntu 9.10.  (Aaron Bentley)
 
 Testing
 *******