Merge lp:~jameinel/bzr/2.2-is-up-to-date into lp:bzr

[John A Meinel]

Bug #609187: warn the user if a package-import branch doesn't have the most recent source package.

This adds a Branch.open hook for the Launchpad plugin. Thanks to Maxb for the nice reference code for querying Launchpad's getPublishedSources.

I wrote this against bzr/2.2 just to give us the maximum opportunity, in case we want to get it backported into something like Lucid or Natty. I don't know whether we will, so certainly I'm proposing this against 2.5 to start.

Points for discussion:

1) This seems to add very little overhead. I added some timing mutters, and it looks like it adds 100-200ms for the getPublishedSeries call (obviously heavily dependent on latency), and about that much again for the get_tag_dict call.

Unfortunately, I don't think the tags are going to end up cached, because at the point Branch.open is hooked we haven't locked the Branch. But we don't (yet) have a good hook point for "I'm fetching from this branch".

Anyway, the ssh handshake time is multiple seconds, so I felt this was reasonable to be on-by-default. Especially since it should only encounter any overhead when we believe the branch is a package-importer branch.

2) Is my search regex reasonable:
    r'bazaar.launchpad.net.*/(?P<archive>ubuntu|debian)/(?P<series>[^/]+/)?'
    r'(?P<project>[^/]+)(?P<branch>/[^/]+)?'

It actually matches based on the expanded URL (not lp:ubuntu/bzr but bazaar.launchpad.net/+branch/ubuntu/bzr). But I'm just listing the short forms that I've tested:
 lp:ubuntu/bzr
 lp:ubuntu/natty/bzr
 lp:~ubuntu-branches/ubuntu/natty/bzr/natty

The one thing I'm not sure about, is that you might want to not get the message if someone is working on a personal branch. However, maybe they *would* like to be informed that their personal branch doesn't have the latest published source.

3) I think it would make more sense to trigger when you are fetching from a branch. This will also trigger when pushing to a branch (maybe you are pushing up the very revision that is missing). However, that would require defining a new hook point, which seemed a lot more invasive if we ever did want to backport this.

4) Wording and information reporting. We may want a flag so that we only tell the users something if it is out of date. However, I thought in the short term, telling them it was up-to-date is reasonable, because it starts letting them know that we're checking for them.
We could enable this in bzr.dev, however I don't know how many people use bzr.dev + work with packaging branches, such that we'd get real feedback about it.

Martin mentioned that some of the wording could be confusing. I'm certainly not attached to any of it. The current output is:

$ bzr info lp:~ubuntu-branches/ubuntu/natty/bzr/natty
Branch not up-to-date. The most recent published version is 2.3.3-0ubuntu1,
but it is not in the branch tags for:
  bzr+ssh://bazaar.launchpad.net/~ubuntu-branches/ubuntu/natty/bzr/natty/
...

$ bzr info lp:ubuntu/bzr
Found most recent published version: 2.4.0~beta4-4ubuntu2
  in bzr+ssh://bazaar.launchpad.net/%2Bbranch/ubuntu/bzr/
...

$ bzr info lp:ubuntu/natty/bzr # With an intentionally buggy query
Could not find a published version for:
  bzr+ssh://bazaar.launchpad.net/%2Bbranch/ubuntu/bzr/
...

I think Martin had a good point that the first case could be clearer about the package import being out of date.

5) I have pretty good test coverage of the LatestPublication class. I have 3 tests that are slow, which I commented out. One testing a failed connection (by attempting to connect to the loopback). For some reason it takes 1s to timeout here, and I didn't want to inflict that in the general case. 2 tests would connect to Launchpad itself. I don't think we want to run them by default, though having them run somehow would be a good way to alert us if Launchpad changes the api somehow.

6) I don't have test coverage of the branch hook. It isn't clear to me what would make a reliable, useful automated test.

[John A Meinel (jameinel)]

Superseded by: https://code.launchpad.net/~jameinel/bzr/2.5-up-to-date-609187/+merge/67844
Can't actually mark superseded, so I'll set this one back to WIP.

=== modified file 'bzrlib/plugins/launchpad/__init__.py'
--- bzrlib/plugins/launchpad/__init__.py	2011-04-05 01:12:15 +0000
+++ bzrlib/plugins/launchpad/__init__.py	2011-07-13 16:24:36 +0000
@@ -40,19 +40,28 @@
 
 # see http://wiki.bazaar.canonical.com/Specs/BranchRegistrationTool
 
+import time
+
 # Since we are a built-in plugin we share the bzrlib version
-from bzrlib import version_info
 
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
 from bzrlib import (
+<<<<<<< TREE
     branch as _mod_branch,
     ui,
+=======
+>>>>>>> MERGE-SOURCE
     trace,
     )
 """)
 
-from bzrlib import bzrdir
+from bzrlib import (
+    branch as _mod_branch,
+    bzrdir,
+    lazy_regex,
+    version_info,
+    )
 from bzrlib.commands import (
     Command,
     register_command,
@@ -463,11 +472,55 @@
 _register_directory()
 
 
+package_branch = lazy_regex.lazy_compile(
+    r'bazaar.launchpad.net.*/(?P<archive>ubuntu|debian)/(?P<series>[^/]+/)?'
+    r'(?P<project>[^/]+)(?P<branch>/[^/]+)?'
+    )
+def _check_is_up_to_date(the_branch):
+    m = package_branch.search(the_branch.base)
+    if m is None:
+        return
+    from bzrlib.plugins.launchpad import lp_api_lite
+    archive, series, project = m.group('archive', 'series', 'project')
+    if series is not None:
+        # series is optional, so the regex includes the extra '/', we don't
+        # want to send that on (it causes Internal Server Errors.)
+        series = series.strip('/')
+    t = time.time()
+    latest_pub = lp_api_lite.LatestPublication(archive, series, project)
+    latest_ver = latest_pub.get_latest_version()
+    t_latest_ver = time.time() - t
+    trace.mutter('LatestPublication.get_latest_version took %.3fs'
+                 % (t_latest_ver,))
+    if latest_ver is None:
+        trace.note('Could not find a published version for:\n  %s'
+                   % (t, the_branch.base,))
+        return
+    t = time.time()
+    tags = the_branch.tags.get_tag_dict()
+    t_tag_dict = time.time() - t
+    trace.mutter('LatestPublication get_tag_dict took: %.3fs', t_tag_dict)
+    if latest_ver in tags:
+        trace.note('Found most recent published version: %s\n  in %s'
+                   % (latest_ver, the_branch.base))
+    else:
+        trace.warning('Branch not up-to-date. The most recent published'
+                      ' version is %s,\nbut it is not in the branch'
+                      ' tags for:\n  %s' % (latest_ver, the_branch.base))
+
+def _register_hooks():
+    _mod_branch.Branch.hooks.install_named_hook('open',
+        _check_is_up_to_date, 'package-branch-up-to-date')
+
+
+_register_hooks()
+
 def load_tests(basic_tests, module, loader):
     testmod_names = [
         'test_account',
         'test_register',
         'test_lp_api',
+        'test_lp_api_lite',
         'test_lp_directory',
         'test_lp_login',
         'test_lp_open',

=== added file 'bzrlib/plugins/launchpad/lp_api_lite.py'
--- bzrlib/plugins/launchpad/lp_api_lite.py	1970-01-01 00:00:00 +0000
+++ bzrlib/plugins/launchpad/lp_api_lite.py	2011-07-13 16:24:36 +0000
@@ -0,0 +1,170 @@
+# Copyright (C) 2011 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Tools for dealing with the Launchpad API without using launchpadlib.
+
+The api itself is a RESTful interface, so we can make HTTP queries directly.
+loading launchpadlib itself has a fairly high overhead (just calling
+Launchpad.login_anonymously() takes a 500ms once the WADL is cached, and 5+s to
+get the WADL.
+"""
+
+try:
+    # Use simplejson if available, much faster, and can be easily installed in
+    # older versions of python
+    import simplejson as json
+except ImportError:
+    # Is present since python 2.6
+    try:
+        import json
+    except ImportError:
+        json = None
+
+import urllib
+import urllib2
+
+from bzrlib import trace
+
+
+DEFAULT_SERIES = 'oneiric'
+
+class LatestPublication(object):
+    """Encapsulate how to find the latest publication for a given project."""
+
+    LP_API_ROOT = 'https://api.launchpad.net/1.0'
+
+    def __init__(self, archive, series, project):
+        self._archive = archive
+        self._project = project
+        self._setup_series_and_pocket(series)
+
+    def _setup_series_and_pocket(self, series):
+        """Parse the 'series' info into a series and a pocket.
+
+        eg::
+            _setup_series_and_pocket('natty-proposed')
+            => _series == 'natty'
+               _pocket == 'Proposed'
+        """
+        self._series = series
+        self._pocket = None
+        if self._series is not None and '-' in self._series:
+            self._series, self._pocket = self._series.split('-', 1)
+            self._pocket = self._pocket.title()
+        else:
+            self._pocket = 'Release'
+
+    def _archive_URL(self):
+        """Return the Launchpad 'Archive' URL that we will query.
+        This is everything in the URL except the query parameters.
+        """
+        return '%s/%s/+archive/primary' % (self.LP_API_ROOT, self._archive)
+
+    def _publication_status(self):
+        """Handle the 'status' field.
+        It seems that Launchpad tracks all 'debian' packages as 'Pending', while
+        for 'ubuntu' we care about the 'Published' packages.
+        """
+        if self._archive == 'debian':
+            # Launchpad only tracks debian packages as "Pending", it doesn't mark
+            # them Published
+            return 'Pending'
+        return 'Published'
+
+    def _query_params(self):
+        """Get the parameters defining our query.
+        This defines the actions we are making against the archive.
+        :return: A dict of query parameters.
+        """
+        params = {'ws.op': 'getPublishedSources',
+                  'exact_match': 'true',
+                  # If we need to use "" shouldn't we quote the project somehow?
+                  'source_name': '"%s"' % (self._project,),
+                  'status': self._publication_status(),
+                  # We only need the latest one, the results seem to be properly
+                  # most-recent-debian-version sorted
+                  'ws.size': '1',
+        }
+        if self._series is not None:
+            params['distro_series'] = '/%s/%s' % (self._archive, self._series)
+        if self._pocket is not None:
+            params['pocket'] = self._pocket
+        return params
+
+    def _query_URL(self):
+        """Create the full URL that we need to query, including parameters."""
+        params = self._query_params()
+        # We sort to give deterministic results for testing
+        encoded = urllib.urlencode(sorted(params.items()))
+        return '%s?%s' % (self._archive_URL(), encoded)
+
+    def _get_lp_info(self):
+        """Place an actual HTTP query against the Launchpad service."""
+        if json is None:
+            return None
+        query_URL = self._query_URL()
+        try:
+            req = urllib2.Request(query_URL)
+            response = urllib2.urlopen(req)
+            json_info = response.read()
+        # XXX: We haven't tested the HTTPError
+        except (urllib2.URLError, urllib2.HTTPError), e:
+            trace.mutter('failed to place query to %r' % (query_URL,))
+            trace.log_exception_quietly()
+            return None
+        return json_info
+
+    def _parse_json_info(self, json_info):
+        """Parse the json response from Launchpad into objects."""
+        if json is None:
+            return None
+        try:
+            return json.loads(json_info)
+        except Exception:
+            trace.mutter('Failed to parse json info: %r' % (json_info,))
+            trace.log_exception_quietly()
+            return None
+
+    def get_latest_version(self):
+        """Get the latest published version for the given package."""
+        json_info = self._get_lp_info()
+        if json_info is None:
+            return None
+        info = self._parse_json_info(json_info)
+        if info is None:
+            return None
+        try:
+            entries = info['entries']
+            if len(entries) == 0:
+                return None
+            return entries[0]['source_package_version']
+        except KeyError:
+            trace.log_exception_quietly()
+            return None
+
+
+def get_latest_publication(archive, series, project):
+    """Get the most recent publication for a given project.
+
+    :param archive: Either 'ubuntu' or 'debian'
+    :param series: Something like 'natty', 'sid', etc. Can be set as None. Can
+        also include a pocket such as 'natty-proposed'.
+    :param project: Something like 'bzr'
+    :return: A version string indicating the most-recent version published in
+        Launchpad. Might return None if there is an error.
+    """
+    lp = LatestPublication(archive, series, project)
+    return lp.get_latest_version()

=== added file 'bzrlib/plugins/launchpad/test_lp_api_lite.py'
--- bzrlib/plugins/launchpad/test_lp_api_lite.py	1970-01-01 00:00:00 +0000
+++ bzrlib/plugins/launchpad/test_lp_api_lite.py	2011-07-13 16:24:36 +0000
@@ -0,0 +1,262 @@
+# Copyright (C) 2011 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Tools for dealing with the Launchpad API without using launchpadlib.
+"""
+
+import socket
+
+from bzrlib import tests
+from bzrlib.plugins.launchpad import lp_api_lite
+
+class _JSONParserFeature(tests.Feature):
+
+    def _probe(self):
+        return lp_api_lite.json is not None
+
+    def feature_name(self):
+        return 'simplejson or json'
+
+JSONParserFeature = _JSONParserFeature()
+
+_example_response = r"""
+{
+    "total_size": 2,
+    "start": 0,
+    "next_collection_link": "https://api.launchpad.net/1.0/ubuntu/+archive/primary?distro_series=%2Fubuntu%2Flucid&exact_match=true&source_name=%22bzr%22&status=Published&ws.op=getPublishedSources&ws.start=1&ws.size=1",
+    "entries": [
+        {
+            "package_creator_link": "https://api.launchpad.net/1.0/~maxb",
+            "package_signer_link": "https://api.launchpad.net/1.0/~jelmer",
+            "source_package_name": "bzr",
+            "removal_comment": null,
+            "display_name": "bzr 2.1.4-0ubuntu1 in lucid",
+            "date_made_pending": null,
+            "source_package_version": "2.1.4-0ubuntu1",
+            "date_superseded": null,
+            "http_etag": "\"9ba966152dec474dc0fe1629d0bbce2452efaf3b-5f4c3fbb3eaf26d502db4089777a9b6a0537ffab\"",
+            "self_link": "https://api.launchpad.net/1.0/ubuntu/+archive/primary/+sourcepub/1750327",
+            "distro_series_link": "https://api.launchpad.net/1.0/ubuntu/lucid",
+            "component_name": "main",
+            "status": "Published",
+            "date_removed": null,
+            "pocket": "Updates",
+            "date_published": "2011-05-30T06:09:58.653984+00:00",
+            "removed_by_link": null,
+            "section_name": "devel",
+            "resource_type_link": "https://api.launchpad.net/1.0/#source_package_publishing_history",
+            "archive_link": "https://api.launchpad.net/1.0/ubuntu/+archive/primary",
+            "package_maintainer_link": "https://api.launchpad.net/1.0/~ubuntu-devel-discuss-lists",
+            "date_created": "2011-05-30T05:19:12.233621+00:00",
+            "scheduled_deletion_date": null
+        }
+    ]
+}"""
+
+_no_versions_response = '{"total_size": 0, "start": 0, "entries": []}'
+
+
+class TestLatestPublication(tests.TestCase):
+
+    def make_latest_publication(self, archive='ubuntu', series='natty',
+                                project='bzr'):
+        return lp_api_lite.LatestPublication(archive, series, project)
+
+    def test_init(self):
+        latest_pub = self.make_latest_publication()
+        self.assertEqual('ubuntu', latest_pub._archive)
+        self.assertEqual('natty', latest_pub._series)
+        self.assertEqual('bzr', latest_pub._project)
+        self.assertEqual('Release', latest_pub._pocket)
+
+    def test__archive_URL(self):
+        latest_pub = self.make_latest_publication()
+        self.assertEqual(
+            'https://api.launchpad.net/1.0/ubuntu/+archive/primary',
+            latest_pub._archive_URL())
+
+    def test__publication_status_for_ubuntu(self):
+        latest_pub = self.make_latest_publication()
+        self.assertEqual('Published', latest_pub._publication_status())
+
+    def test__publication_status_for_debian(self):
+        latest_pub = self.make_latest_publication(archive='debian')
+        self.assertEqual('Pending', latest_pub._publication_status())
+
+    def test_pocket(self):
+        latest_pub = self.make_latest_publication(series='natty-proposed')
+        self.assertEqual('natty', latest_pub._series)
+        self.assertEqual('Proposed', latest_pub._pocket)
+
+    def test_series_None(self):
+        latest_pub = self.make_latest_publication(series=None)
+        self.assertEqual('ubuntu', latest_pub._archive)
+        self.assertEqual(None, latest_pub._series)
+        self.assertEqual('bzr', latest_pub._project)
+        self.assertEqual('Release', latest_pub._pocket)
+
+    def test__query_params(self):
+        latest_pub = self.make_latest_publication()
+        self.assertEqual({'ws.op': 'getPublishedSources',
+                          'exact_match': 'true',
+                          'source_name': '"bzr"',
+                          'status': 'Published',
+                          'ws.size': '1',
+                          'distro_series': '/ubuntu/natty',
+                          'pocket': 'Release',
+                         }, latest_pub._query_params())
+
+    def test__query_params_no_series(self):
+        latest_pub = self.make_latest_publication(series=None)
+        self.assertEqual({'ws.op': 'getPublishedSources',
+                          'exact_match': 'true',
+                          'source_name': '"bzr"',
+                          'status': 'Published',
+                          'ws.size': '1',
+                          'pocket': 'Release',
+                         }, latest_pub._query_params())
+
+    def test__query_params_pocket(self):
+        latest_pub = self.make_latest_publication(series='natty-proposed')
+        self.assertEqual({'ws.op': 'getPublishedSources',
+                          'exact_match': 'true',
+                          'source_name': '"bzr"',
+                          'status': 'Published',
+                          'ws.size': '1',
+                          'distro_series': '/ubuntu/natty',
+                          'pocket': 'Proposed',
+                         }, latest_pub._query_params())
+
+    def test__query_URL(self):
+        latest_pub = self.make_latest_publication()
+        # we explicitly sort params, so we can be sure this URL matches exactly
+        self.assertEqual(
+            'https://api.launchpad.net/1.0/ubuntu/+archive/primary'
+            '?distro_series=%2Fubuntu%2Fnatty&exact_match=true'
+            '&pocket=Release&source_name=%22bzr%22&status=Published'
+            '&ws.op=getPublishedSources&ws.size=1',
+            latest_pub._query_URL())
+
+    def DONT_test__gracefully_handle_failed_rpc_connection(self):
+        # TODO: This test kind of sucks. We intentionally create an arbitrary
+        #       port and don't listen to it, because we want the request to fail.
+        #       However, it seems to take 1s for it to timeout. Is there a way
+        #       to make it fail faster?
+        latest_pub = self.make_latest_publication()
+        s = socket.socket()
+        s.bind(('127.0.0.1', 0))
+        addr, port = s.getsockname()
+        latest_pub.LP_API_ROOT = 'http://%s:%s/' % (addr, port)
+        s.close()
+        self.assertIs(None, latest_pub._get_lp_info())
+
+    def DONT_test__query_launchpad(self):
+        # TODO: This is a test that we are making a valid request against
+        #       launchpad. This seems important, but it is slow, requires net
+        #       access, and requires launchpad to be up and running. So for
+        #       now, it is commented out for production tests.
+        latest_pub = self.make_latest_publication()
+        json_txt = latest_pub._get_lp_info()
+        self.assertIsNot(None, json_txt)
+        if lp_api_lite.json is None:
+            # We don't have a way to parse the text
+            return
+        # The content should be a valid json result
+        content = lp_api_lite.json.loads(json_txt)
+        entries = content['entries'] # It should have an 'entries' field.
+        # ws.size should mean we get 0 or 1, and there should be something
+        self.assertEqual(1, len(entries))
+        entry = entries[0]
+        self.assertEqual('bzr', entry['source_package_name'])
+        version = entry['source_package_version']
+        self.assertIsNot(None, version)
+
+    def test__get_lp_info_no_json(self):
+        # If we can't parse the json, we don't make the query.
+        self.overrideAttr(lp_api_lite, 'json', None)
+        latest_pub = self.make_latest_publication()
+        self.assertIs(None, latest_pub._get_lp_info())
+
+    def test__parse_json_info_no_module(self):
+        # If a json parsing module isn't available, we just return None here.
+        self.overrideAttr(lp_api_lite, 'json', None)
+        latest_pub = self.make_latest_publication()
+        self.assertIs(None, latest_pub._parse_json_info(_example_response))
+
+    def test__parse_json_example_response(self):
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        content = latest_pub._parse_json_info(_example_response)
+        self.assertIsNot(None, content)
+        self.assertEqual(2, content['total_size'])
+        entries = content['entries']
+        self.assertEqual(1, len(entries))
+        entry = entries[0]
+        self.assertEqual('bzr', entry['source_package_name'])
+        self.assertEqual("2.1.4-0ubuntu1", entry["source_package_version"])
+
+    def test__parse_json_not_json(self):
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        self.assertIs(None, latest_pub._parse_json_info('Not_valid_json'))
+
+    def test_get_latest_version_no_response(self):
+        latest_pub = self.make_latest_publication()
+        latest_pub._get_lp_info = lambda: None
+        self.assertEqual(None, latest_pub.get_latest_version())
+
+    def test_get_latest_version_no_json(self):
+        self.overrideAttr(lp_api_lite, 'json', None)
+        latest_pub = self.make_latest_publication()
+        self.assertEqual(None, latest_pub.get_latest_version())
+
+    def test_get_latest_version_invalid_json(self):
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        latest_pub._get_lp_info = lambda: "not json"
+        self.assertEqual(None, latest_pub.get_latest_version())
+
+    def test_get_latest_version_no_versions(self):
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        latest_pub._get_lp_info = lambda: _no_versions_response
+        self.assertEqual(None, latest_pub.get_latest_version())
+
+    def test_get_latest_version_missing_entries(self):
+        # Launchpad's no-entries response does have an empty entries value.
+        # However, lets test that we handle other failures without tracebacks
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        latest_pub._get_lp_info = lambda: '{}'
+        self.assertEqual(None, latest_pub.get_latest_version())
+
+    def test_get_latest_version_invalid_entries(self):
+        # Make sure we sanely handle a json response we don't understand
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        latest_pub._get_lp_info = lambda: '{"entries": {"a": 1}}'
+        self.assertEqual(None, latest_pub.get_latest_version())
+
+    def test_get_latest_version_example(self):
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        latest_pub._get_lp_info = lambda: _example_response
+        self.assertEqual("2.1.4-0ubuntu1", latest_pub.get_latest_version())
+
+    def DONT_test_get_latest_version_from_launchpad(self):
+        self.requireFeature(JSONParserFeature)
+        latest_pub = self.make_latest_publication()
+        self.assertIsNot(None, latest_pub.get_latest_version())