Rev 2469: Merge Johns fix for bug 110256. in http://people.ubuntu.com/~robertc/baz2.0/integration

Robert Collins robertc at robertcollins.net
Mon Apr 30 04:48:55 BST 2007


At http://people.ubuntu.com/~robertc/baz2.0/integration

------------------------------------------------------------
revno: 2469
revision-id: robertc at robertcollins.net-20070430034851-aik2bzpubf44oyjc
parent: john at arbash-meinel.com-20070426211345-fh5tuoii9mb7w15a
parent: pqm at pqm.ubuntu.com-20070430025942-y83xydh67a37zebd
committer: Robert Collins <robertc at robertcollins.net>
branch nick: integration
timestamp: Mon 2007-04-30 13:48:51 +1000
message:
  Merge Johns fix for bug 110256.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/bundle/bundle_data.py   read_changeset.py-20050619171944-c0d95aa685537640
  bzrlib/bundle/serializer/v08.py v06.py-20051119041339-ee43f97270b01823
  bzrlib/msgeditor.py            msgeditor.py-20050901111708-ef6d8de98f5d8f2f
  bzrlib/tests/test_bundle.py    test.py-20050630184834-092aa401ab9f039c
  bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
  bzrlib/transport/remote.py     ssh.py-20060608202016-c25gvf1ob7ypbus6-1
    ------------------------------------------------------------
    revno: 2465.1.3
    merged: pqm at pqm.ubuntu.com-20070430025942-y83xydh67a37zebd
    parent: pqm at pqm.ubuntu.com-20070430022343-wnbvslzfz6fpyyj7
    parent: andrew.bennetts at canonical.com-20070430022545-iubud5mcxc1mdsw9
    committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
    branch nick: +trunk
    timestamp: Mon 2007-04-30 03:59:42 +0100
    message:
      (Andrew Bennetts) Fix incompatibility with < 0.16 smart servers.
        ------------------------------------------------------------
        revno: 2465.1.1.2.4
        merged: andrew.bennetts at canonical.com-20070430022545-iubud5mcxc1mdsw9
        parent: andrew.bennetts at canonical.com-20070430020406-9vvmb4f9x8bdmvv8
        committer: Andrew Bennetts <andrew.bennetts at canonical.com>
        branch nick: hpss-older-server-compat-bug
        timestamp: Mon 2007-04-30 12:25:45 +1000
        message:
          Tweaks asked for by Robert.
        ------------------------------------------------------------
        revno: 2465.1.1.2.3
        merged: andrew.bennetts at canonical.com-20070430020406-9vvmb4f9x8bdmvv8
        parent: andrew.bennetts at canonical.com-20070430020205-ensbvu0t14yb3tk4
        committer: Andrew Bennetts <andrew.bennetts at canonical.com>
        branch nick: hpss-older-server-compat-bug
        timestamp: Mon 2007-04-30 12:04:06 +1000
        message:
          Remove another XXX.
        ------------------------------------------------------------
        revno: 2465.1.1.2.2
        merged: andrew.bennetts at canonical.com-20070430020205-ensbvu0t14yb3tk4
        parent: andrew.bennetts at canonical.com-20070427030144-vz2vna1t31ymdgz9
        committer: Andrew Bennetts <andrew.bennetts at canonical.com>
        branch nick: hpss-older-server-compat-bug
        timestamp: Mon 2007-04-30 12:02:05 +1000
        message:
          Add tests for RemoteTransport.is_readonly in the style of the other remote object tests.
        ------------------------------------------------------------
        revno: 2465.1.1.2.1
        merged: andrew.bennetts at canonical.com-20070427030144-vz2vna1t31ymdgz9
        parent: pqm at pqm.ubuntu.com-20070426211103-h84prqh7a4ad3ez2
        committer: Andrew Bennetts <andrew.bennetts at canonical.com>
        branch nick: hpss-older-server-compat-bug
        timestamp: Fri 2007-04-27 13:01:44 +1000
        message:
          Fix incompatibility with < 0.16 smart servers.
    ------------------------------------------------------------
    revno: 2465.1.2
    merged: pqm at pqm.ubuntu.com-20070430022343-wnbvslzfz6fpyyj7
    parent: pqm at pqm.ubuntu.com-20070426211103-h84prqh7a4ad3ez2
    parent: robertc at robertcollins.net-20070430014939-mjnji1bq7zulpjlv
    committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
    branch nick: +trunk
    timestamp: Mon 2007-04-30 03:23:43 +0100
    message:
      (robertc) Fix the bzr commit message to be in text mode. (Alexander Belchenko)
        ------------------------------------------------------------
        revno: 2465.1.1.1.1
        merged: robertc at robertcollins.net-20070430014939-mjnji1bq7zulpjlv
        parent: pqm at pqm.ubuntu.com-20070426211103-h84prqh7a4ad3ez2
        committer: Robert Collins <robertc at robertcollins.net>
        branch nick: integration
        timestamp: Mon 2007-04-30 11:49:39 +1000
        message:
          Fix the bzr commit message to be in text mode. (Alexander Belchenko)
    ------------------------------------------------------------
    revno: 2465.1.1
    merged: pqm at pqm.ubuntu.com-20070426211103-h84prqh7a4ad3ez2
    parent: pqm at pqm.ubuntu.com-20070426181928-ot99t6eyhjjfj5h3
    parent: john at arbash-meinel.com-20070426185333-i1xlyaeyf049kdxc
    committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
    branch nick: +trunk
    timestamp: Thu 2007-04-26 22:11:03 +0100
    message:
      (John Arbash Meinel) Fix bug #109613 by teaching Bundle how to properly read/write revision properties with no value.
    ------------------------------------------------------------
    revno: 2447.1.7
    merged: john at arbash-meinel.com-20070426185333-i1xlyaeyf049kdxc
    parent: john at arbash-meinel.com-20070426145512-xnda2pxcpfa86yld
    parent: pqm at pqm.ubuntu.com-20070426181928-ot99t6eyhjjfj5h3
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bundle_empty_properties
    timestamp: Thu 2007-04-26 13:53:33 -0500
    message:
      [merge] bzr.dev 2465
    ------------------------------------------------------------
    revno: 2447.1.6
    merged: john at arbash-meinel.com-20070426145512-xnda2pxcpfa86yld
    parent: john at arbash-meinel.com-20070425175514-il068rykvdulkm2k
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bundle_empty_properties
    timestamp: Thu 2007-04-26 09:55:12 -0500
    message:
      NEWS update for fixing bug #109613
    ------------------------------------------------------------
    revno: 2447.1.5
    merged: john at arbash-meinel.com-20070425175514-il068rykvdulkm2k
    parent: john at arbash-meinel.com-20070425174944-mti4294wv55nimd3
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bundle_empty_properties
    timestamp: Wed 2007-04-25 12:55:14 -0500
    message:
      NEWS
    ------------------------------------------------------------
    revno: 2447.1.4
    merged: john at arbash-meinel.com-20070425174944-mti4294wv55nimd3
    parent: john at arbash-meinel.com-20070425165625-d2act28kmjn5avel
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bundle_empty_properties
    timestamp: Wed 2007-04-25 12:49:44 -0500
    message:
      Add a test that we properly round-trip unicode properties.
      And fix the (unreported) bug :)
    ------------------------------------------------------------
    revno: 2447.1.3
    merged: john at arbash-meinel.com-20070425165625-d2act28kmjn5avel
    parent: john at arbash-meinel.com-20070425164125-nr7pmz4tfp5qkze2
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bundle_empty_properties
    timestamp: Wed 2007-04-25 11:56:25 -0500
    message:
      Change the default serializer to include a trailing whitespace for empty properties.
      This means that new bundles can be read by old versions of bzr
      (though they cannot read their own).
      On the flip side, the new bzr can read old outputs correctly.
    ------------------------------------------------------------
    revno: 2447.1.2
    merged: john at arbash-meinel.com-20070425164125-nr7pmz4tfp5qkze2
    parent: john at arbash-meinel.com-20070425143231-wy5jkw8g9tr471ji
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bundle_empty_properties
    timestamp: Wed 2007-04-25 11:41:25 -0500
    message:
      Add tests that we handle empty values whether they end in ': \n' or ':\n'.
      And fix the reader to handle the ':\n' case.
    ------------------------------------------------------------
    revno: 2447.1.1
    merged: john at arbash-meinel.com-20070425143231-wy5jkw8g9tr471ji
    parent: pqm at pqm.ubuntu.com-20070423170758-qd512ltqglzfo6w9
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bundle_empty_properties
    timestamp: Wed 2007-04-25 09:32:31 -0500
    message:
      For stability and ease of testing, write properties in sorted order.
      This doesn't really change the bundle format, since we read the
      properties into a dictionary, so their position in the file doesn't
      matter. Plus the Revision xml serializer also writes them in sorted
      order.
=== modified file 'NEWS'
--- a/NEWS	2007-04-26 21:13:45 +0000
+++ b/NEWS	2007-04-30 03:48:51 +0000
@@ -7,10 +7,16 @@
       status`` should show the removed file and an unknown file in its
       place. (John Arbash Meinel, #109993)
 
+    * Bundles properly read and write revision properties that have an
+      empty value. And when the value is not ASCII.
+      (John Arbash Meinel, #109613)
+
+    * Fix the bzr commit message to be in text mode.
+      (Alexander Belchenko, #110901)
+
     * Also handle when you rename a file and create a file where it used
       to be. (John Arbash Meinel, #110256)
 
-
 bzr 0.16rc1  2007-04-26
 
   NOTES WHEN UPGRADING:

=== modified file 'bzrlib/bundle/bundle_data.py'
--- a/bzrlib/bundle/bundle_data.py	2007-04-05 09:35:26 +0000
+++ b/bzrlib/bundle/bundle_data.py	2007-04-25 17:49:44 +0000
@@ -76,9 +76,13 @@
         if self.properties:
             for property in self.properties:
                 key_end = property.find(': ')
-                assert key_end is not None
-                key = property[:key_end].encode('utf-8')
-                value = property[key_end+2:].encode('utf-8')
+                if key_end == -1:
+                    assert property.endswith(':')
+                    key = str(property[:-1])
+                    value = ''
+                else:
+                    key = str(property[:key_end])
+                    value = property[key_end+2:]
                 rev.properties[key] = value
 
         return rev

=== modified file 'bzrlib/bundle/serializer/v08.py'
--- a/bzrlib/bundle/serializer/v08.py	2007-03-16 15:14:40 +0000
+++ b/bzrlib/bundle/serializer/v08.py	2007-04-25 16:56:25 +0000
@@ -135,14 +135,24 @@
         f.write('0.8\n')
         f.write('#\n')
 
-    def _write(self, key, value, indent=1):
-        """Write out meta information, with proper indenting, etc"""
+    def _write(self, key, value, indent=1, trailing_space_when_empty=False):
+        """Write out meta information, with proper indenting, etc.
+
+        :param trailing_space_when_empty: To work around a bug in earlier
+            bundle readers, when writing an empty property, we use "prop: \n"
+            rather than writing "prop:\n".
+            If this parameter is True, and value is the empty string, we will
+            write an extra space.
+        """
         assert indent > 0, 'indentation must be greater than 0'
         f = self.to_file
         f.write('#' + (' ' * indent))
         f.write(key.encode('utf-8'))
         if not value:
-            f.write(':\n')
+            if trailing_space_when_empty and value == '':
+                f.write(': \n')
+            else:
+                f.write(':\n')
         elif isinstance(value, str):
             f.write(': ')
             f.write(value)
@@ -225,8 +235,9 @@
             w('base id', base_rev)
         if rev.properties:
             self._write('properties', None, indent=1)
-            for name, value in rev.properties.items():
-                self._write(name, value, indent=3)
+            for name, value in sorted(rev.properties.items()):
+                self._write(name, value, indent=3,
+                            trailing_space_when_empty=True)
         
         # Add an extra blank space at the end
         self.to_file.write('\n')

=== modified file 'bzrlib/msgeditor.py'
--- a/bzrlib/msgeditor.py	2007-02-26 21:48:05 +0000
+++ b/bzrlib/msgeditor.py	2007-04-30 01:49:39 +0000
@@ -103,7 +103,9 @@
 
     msgfilename = None
     try:
-        tmp_fileno, msgfilename = tempfile.mkstemp(prefix='bzr_log.', dir=u'.')
+        tmp_fileno, msgfilename = tempfile.mkstemp(prefix='bzr_log.',
+                                                   dir=u'.',
+                                                   text=True)
         msgfile = os.fdopen(tmp_fileno, 'w')
         try:
             if start_message is not None:

=== modified file 'bzrlib/tests/test_bundle.py'
--- a/bzrlib/tests/test_bundle.py	2007-04-04 21:39:03 +0000
+++ b/bzrlib/tests/test_bundle.py	2007-04-25 17:49:44 +0000
@@ -905,6 +905,116 @@
         self.assertTrue(branch2.repository.has_revision('rev2a'))
         self.assertEqual('rev2a', target_revision)
 
+    def test_bundle_empty_property(self):
+        """Test serializing revision properties with an empty value."""
+        tree = self.make_branch_and_memory_tree('tree')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+        tree.add([''], ['TREE_ROOT'])
+        tree.commit('One', revprops={'one':'two', 'empty':''}, rev_id='rev1')
+        self.b1 = tree.branch
+        bundle_sio, revision_ids = self.create_bundle_text(None, 'rev1')
+        self.assertContainsRe(bundle_sio.getvalue(),
+                              '# properties:\n'
+                              '#   branch-nick: tree\n'
+                              '#   empty: \n'
+                              '#   one: two\n'
+                             )
+        bundle = read_bundle(bundle_sio)
+        revision_info = bundle.revisions[0]
+        self.assertEqual('rev1', revision_info.revision_id)
+        rev = revision_info.as_revision()
+        self.assertEqual({'branch-nick':'tree', 'empty':'', 'one':'two'},
+                         rev.properties)
+
+    def test_bundle_empty_property_alt(self):
+        """Test serializing revision properties with an empty value.
+
+        Older readers had a bug when reading an empty property.
+        They assumed that all keys ended in ': \n'. However they would write an
+        empty value as ':\n'. This tests make sure that all newer bzr versions
+        can handle th second form.
+        """
+        tree = self.make_branch_and_memory_tree('tree')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+        tree.add([''], ['TREE_ROOT'])
+        tree.commit('One', revprops={'one':'two', 'empty':''}, rev_id='rev1')
+        self.b1 = tree.branch
+        bundle_sio, revision_ids = self.create_bundle_text(None, 'rev1')
+        txt = bundle_sio.getvalue()
+        loc = txt.find('#   empty: ') + len('#   empty:')
+        # Create a new bundle, which strips the trailing space after empty
+        bundle_sio = StringIO(txt[:loc] + txt[loc+1:])
+
+        self.assertContainsRe(bundle_sio.getvalue(),
+                              '# properties:\n'
+                              '#   branch-nick: tree\n'
+                              '#   empty:\n'
+                              '#   one: two\n'
+                             )
+        bundle = read_bundle(bundle_sio)
+        revision_info = bundle.revisions[0]
+        self.assertEqual('rev1', revision_info.revision_id)
+        rev = revision_info.as_revision()
+        self.assertEqual({'branch-nick':'tree', 'empty':'', 'one':'two'},
+                         rev.properties)
+
+    def test_bundle_sorted_properties(self):
+        """For stability the writer should write properties in sorted order."""
+        tree = self.make_branch_and_memory_tree('tree')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+
+        tree.add([''], ['TREE_ROOT'])
+        tree.commit('One', rev_id='rev1',
+                    revprops={'a':'4', 'b':'3', 'c':'2', 'd':'1'})
+        self.b1 = tree.branch
+        bundle_sio, revision_ids = self.create_bundle_text(None, 'rev1')
+        self.assertContainsRe(bundle_sio.getvalue(),
+                              '# properties:\n'
+                              '#   a: 4\n'
+                              '#   b: 3\n'
+                              '#   branch-nick: tree\n'
+                              '#   c: 2\n'
+                              '#   d: 1\n'
+                             )
+        bundle = read_bundle(bundle_sio)
+        revision_info = bundle.revisions[0]
+        self.assertEqual('rev1', revision_info.revision_id)
+        rev = revision_info.as_revision()
+        self.assertEqual({'branch-nick':'tree', 'a':'4', 'b':'3', 'c':'2',
+                          'd':'1'}, rev.properties)
+
+    def test_bundle_unicode_properties(self):
+        """We should be able to round trip a non-ascii property."""
+        tree = self.make_branch_and_memory_tree('tree')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+
+        tree.add([''], ['TREE_ROOT'])
+        # Revisions themselves do not require anything about revision property
+        # keys, other than that they are a basestring, and do not contain
+        # whitespace.
+        # However, Testaments assert than they are str(), and thus should not
+        # be Unicode.
+        tree.commit('One', rev_id='rev1',
+                    revprops={'omega':u'\u03a9', 'alpha':u'\u03b1'})
+        self.b1 = tree.branch
+        bundle_sio, revision_ids = self.create_bundle_text(None, 'rev1')
+        self.assertContainsRe(bundle_sio.getvalue(),
+                              '# properties:\n'
+                              '#   alpha: \xce\xb1\n'
+                              '#   branch-nick: tree\n'
+                              '#   omega: \xce\xa9\n'
+                             )
+        bundle = read_bundle(bundle_sio)
+        revision_info = bundle.revisions[0]
+        self.assertEqual('rev1', revision_info.revision_id)
+        rev = revision_info.as_revision()
+        self.assertEqual({'branch-nick':'tree', 'omega':u'\u03a9',
+                          'alpha':u'\u03b1'}, rev.properties)
+
 
 class V09BundleKnit2Tester(V08BundleTester):
 

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2007-04-26 08:15:40 +0000
+++ b/bzrlib/tests/test_remote.py	2007-04-30 02:04:06 +0000
@@ -44,6 +44,7 @@
 from bzrlib.smart import server, medium
 from bzrlib.smart.client import _SmartClient
 from bzrlib.transport.memory import MemoryTransport
+from bzrlib.transport.remote import RemoteTransport
 
 
 class BasicRemoteObjectTests(tests.TestCaseWithTransport):
@@ -60,12 +61,6 @@
         self.transport.disconnect()
         tests.TestCaseWithTransport.tearDown(self)
 
-    def test_is_readonly(self):
-        # XXX: this is a poor way to test RemoteTransport, but currently there's
-        # no easy way to substitute in a fake client on a transport like we can
-        # with RemoteBzrDir/Branch/Repository.
-        self.assertEqual(self.transport.is_readonly(), False)
-
     def test_create_remote_bzrdir(self):
         b = remote.RemoteBzrDir(self.transport)
         self.assertIsInstance(b, BzrDir)
@@ -104,20 +99,6 @@
         self.assertIsInstance(d, BzrDir)
 
 
-class ReadonlyRemoteTransportTests(tests.TestCaseWithTransport):
-
-    def setUp(self):
-        self.transport_server = server.ReadonlySmartTCPServer_for_testing
-        super(ReadonlyRemoteTransportTests, self).setUp()
-
-    def test_is_readonly_yes(self):
-        # XXX: this is a poor way to test RemoteTransport, but currently there's
-        # no easy way to substitute in a fake client on a transport like we can
-        # with RemoteBzrDir/Branch/Repository.
-        transport = self.get_readonly_transport()
-        self.assertEqual(transport.is_readonly(), True)
-
-
 class FakeProtocol(object):
     """Lookalike SmartClientRequestProtocolOne allowing body reading tests."""
 
@@ -404,6 +385,44 @@
             client._calls)
 
 
+class TestTransportIsReadonly(tests.TestCase):
+
+    def test_true(self):
+        client = FakeClient([(('yes',), '')])
+        transport = RemoteTransport('bzr://example.com/', medium=False,
+                                    _client=client)
+        self.assertEqual(True, transport.is_readonly())
+        self.assertEqual(
+            [('call', 'Transport.is_readonly', ())],
+            client._calls)
+
+    def test_false(self):
+        client = FakeClient([(('no',), '')])
+        transport = RemoteTransport('bzr://example.com/', medium=False,
+                                    _client=client)
+        self.assertEqual(False, transport.is_readonly())
+        self.assertEqual(
+            [('call', 'Transport.is_readonly', ())],
+            client._calls)
+
+    def test_error_from_old_server(self):
+        """bzr 0.15 and earlier servers don't recognise the is_readonly verb.
+        
+        Clients should treat it as a "no" response, because is_readonly is only
+        advisory anyway (a transport could be read-write, but then the
+        underlying filesystem could be readonly anyway).
+        """
+        client = FakeClient([(
+            ('error', "Generic bzr smart protocol error: "
+                      "bad request 'Transport.is_readonly'"), '')])
+        transport = RemoteTransport('bzr://example.com/', medium=False,
+                                    _client=client)
+        self.assertEqual(False, transport.is_readonly())
+        self.assertEqual(
+            [('call', 'Transport.is_readonly', ())],
+            client._calls)
+
+
 class TestRemoteRepository(tests.TestCase):
     """Base for testing RemoteRepository protocol usage.
     

=== modified file 'bzrlib/transport/remote.py'
--- a/bzrlib/transport/remote.py	2007-04-26 06:19:07 +0000
+++ b/bzrlib/transport/remote.py	2007-04-30 02:25:45 +0000
@@ -74,11 +74,17 @@
     # RemoteTransport is an adapter from the Transport object model to the 
     # SmartClient model, not an encoder.
 
-    def __init__(self, url, clone_from=None, medium=None):
+    def __init__(self, url, clone_from=None, medium=None, _client=None):
         """Constructor.
 
+        :param clone_from: Another RemoteTransport instance that this one is
+            being cloned from.  Attributes such as credentials and the medium
+            will be reused.
         :param medium: The medium to use for this RemoteTransport. This must be
             supplied if clone_from is None.
+        :param _client: Override the _SmartClient used by this transport.  This
+            should only be used for testing purposes; normally this is
+            determined from the medium.
         """
         ### Technically super() here is faulty because Transport's __init__
         ### fails to take 2 parameters, and if super were to choose a silly
@@ -97,6 +103,10 @@
             # reuse same connection
             self._medium = clone_from._medium
         assert self._medium is not None
+        if _client is None:
+            self._client = client._SmartClient(self._medium)
+        else:
+            self._client = _client
 
     def abspath(self, relpath):
         """Return the full url to the given relative path.
@@ -123,6 +133,12 @@
             return True
         elif resp == ('no', ):
             return False
+        elif resp == ('error', "Generic bzr smart protocol error: "
+                               "bad request 'Transport.is_readonly'"):
+            # XXX: nasty hack: servers before 0.16 don't have a
+            # 'Transport.is_readonly' verb, so we do what clients before 0.16
+            # did: assume False.
+            return False
         else:
             self._translate_error(resp)
         assert False, 'weird response %r' % (resp,)
@@ -160,12 +176,11 @@
 
     def _call2(self, method, *args):
         """Call a method on the remote server."""
-        return client._SmartClient(self._medium).call(method, *args)
+        return self._client.call(method, *args)
 
     def _call_with_body_bytes(self, method, args, body):
         """Call a method on the remote server with body bytes."""
-        smart_client = client._SmartClient(self._medium)
-        return smart_client.call_with_body_bytes(method, args, body)
+        return self._client.call_with_body_bytes(method, args, body)
 
     def has(self, relpath):
         """Indicate whether a remote file of the given name exists or not.



More information about the bazaar-commits mailing list