Merge lp:~jameinel/bzr/1.16-simple-win32 into lp:~bzr/bzr/trunk-old

[John A Meinel]

[John A Meinel (jameinel)]

This updates a per-repository test that was failing on Windows.
Older formats can't handle file-ids with ':' in them on Windows, because it isn't a valid filename character.
I also update the test a bit, to make it clearer which objects are *keys* (tuples) and which are *ids* (strings).

[John A Meinel (jameinel)]

Sorry for the noise of Resubmit, but I wanted to regenerate the diff without including Vincent's patch.

To summarize, this simply adds a couple of quick fixes to skip certain tests that fail on Win32 with old format repositories.

[Robert Collins (lifeless)]

On Wed, 2009-06-03 at 20:45 +0000, John A Meinel wrote:
> Sorry for the noise of Resubmit, but I wanted to regenerate the diff without including Vincent's patch.
>
> To summarize, this simply adds a couple of quick fixes to skip certain tests that fail on Win32 with old format repositories.

I can't see any new diff; the only thing I can see in this mp thread is
the change to clarify key and fileid variables in a couple of tests -
nothing that does 'skip'.

-Rob

[John A Meinel (jameinel)]

> On Wed, 2009-06-03 at 20:45 +0000, John A Meinel wrote:
> > Sorry for the noise of Resubmit, but I wanted to regenerate the diff without
> including Vincent's patch.
> >
> > To summarize, this simply adds a couple of quick fixes to skip certain tests
> that fail on Win32 with old format repositories.
>
> I can't see any new diff; the only thing I can see in this mp thread is
> the change to clarify key and fileid variables in a couple of tests -
> nothing that does 'skip'.
>
> -Rob

I think you missed this part:
21 + except errors.IllegalPath:
22 + raise TestNotApplicable('file_id %r cannot be stored on this'
23 + ' platform for this repo format' % (file_id,))

John

[Robert Collins (lifeless)]

On Thu, 2009-06-04 at 04:02 +0000, John A Meinel wrote:
> > On Wed, 2009-06-03 at 20:45 +0000, John A Meinel wrote:
> > > Sorry for the noise of Resubmit, but I wanted to regenerate the diff without
> > including Vincent's patch.
> > >
> > > To summarize, this simply adds a couple of quick fixes to skip certain tests
> > that fail on Win32 with old format repositories.
> >
> > I can't see any new diff; the only thing I can see in this mp thread is
> > the change to clarify key and fileid variables in a couple of tests -
> > nothing that does 'skip'.
> >
> > -Rob
>
>
> I think you missed this part:
> 21 + except errors.IllegalPath:
> 22 + raise TestNotApplicable('file_id %r cannot be stored on this'
> 23 + ' platform for this repo format' % (file_id,))

Well, I'm looking at my mail client - I'm pretty sure that hasn't been
in mail sent out. If its on the web UI only, then we should file a bug.

-Rob

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Thu, 2009-06-04 at 04:02 +0000, John A Meinel wrote:
>>> On Wed, 2009-06-03 at 20:45 +0000, John A Meinel wrote:
>>>> Sorry for the noise of Resubmit, but I wanted to regenerate the diff without
>>> including Vincent's patch.
>>>> To summarize, this simply adds a couple of quick fixes to skip certain tests
>>> that fail on Win32 with old format repositories.
>>>
>>> I can't see any new diff; the only thing I can see in this mp thread is
>>> the change to clarify key and fileid variables in a couple of tests -
>>> nothing that does 'skip'.
>>>
>>> -Rob
>>
>> I think you missed this part:
>> 21 + except errors.IllegalPath:
>> 22 + raise TestNotApplicable('file_id %r cannot be stored on this'
>> 23 + ' platform for this repo format' % (file_id,))
>
> Well, I'm looking at my mail client - I'm pretty sure that hasn't been
> in mail sent out. If its on the web UI only, then we should file a bug.
>
> -Rob
>

I see it in the email that was sent to me. I would assume you got the
same email.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkon6VcACgkQJdeBCYSNAAPk7ACdFfOCqe4D70cwWwaIozFYuMql
b4IAmwXyUNM/7Ryz3ATVAhB9DGXvu3Gn
=bSln
-----END PGP SIGNATURE-----

[Vincent Ladeuil (vila)]

>>>>> "jam" == John A Meinel <email address hidden> writes:

<snip/>

    >> Well, I'm looking at my mail client - I'm pretty sure that hasn't been
    >> in mail sent out. If its on the web UI only, then we should file a bug.
    >>
    >> -Rob
    >>

    jam> I see it in the email that was sent to me. I would assume you got the
    jam> same email.

Confirmed here. There was several emails in the thread, Robert
may have missed the one containing the patch since he replied to
a later one.

      Vincent

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
>>>>>> "jam" == John A Meinel <email address hidden> writes:
>
> <snip/>
>
> >> Well, I'm looking at my mail client - I'm pretty sure that hasn't been
> >> in mail sent out. If its on the web UI only, then we should file a bug.
> >>
> >> -Rob
> >>
>
> jam> I see it in the email that was sent to me. I would assume you got the
> jam> same email.
>
> Confirmed here. There was several emails in the thread, Robert
> may have missed the one containing the patch since he replied to
> a later one.
>
> Vincent

Ah, that makes more sense. See the bug I submitted about this fact (back
when I was Resubmitting, before Robert even got confused):
https://bugs.edge.launchpad.net/launchpad/+bug/383352

Namely, doing a Resubmit does not give you a chance to give a comment,
so you have to have a follow up step to have any comment in the review
system.

Which means that everyone gets 2 emails, and that obviously caused
difficulty for Robert, since my "comment on the patch" was disconnected
from the "diff for the patch".

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkooACIACgkQJdeBCYSNAAON4gCdHDD0A3D+l41wRcPLZ9TMx6Cx
eqIAoMUzRzPCCeCNV2VgcMJC4xDgKQl0
=ndTT
-----END PGP SIGNATURE-----

[John A Meinel (jameinel)]

This is trivially correct, so I'm just going to submit it.

review: Approve

=== modified file 'bzrlib/tests/per_repository/test_repository.py'
--- bzrlib/tests/per_repository/test_repository.py	2009-06-15 19:11:56 +0000
+++ bzrlib/tests/per_repository/test_repository.py	2009-06-16 02:36:26 +0000
@@ -151,22 +151,27 @@
         """Test the basic behaviour of the text store."""
         tree = self.make_branch_and_tree('tree')
         repo = tree.branch.repository
-        file_id = ("Foo:Bar",)
+        file_id = "Foo:Bar"
+        file_key = (file_id,)
         tree.lock_write()
         try:
             self.assertEqual(set(), set(repo.texts.keys()))
-            tree.add(['foo'], file_id, ['file'])
-            tree.put_file_bytes_non_atomic(file_id[0], 'content\n')
-            revid = (tree.commit("foo"),)
+            tree.add(['foo'], [file_id], ['file'])
+            tree.put_file_bytes_non_atomic(file_id, 'content\n')
+            try:
+                rev_key = (tree.commit("foo"),)
+            except errors.IllegalPath:
+                raise TestNotApplicable('file_id %r cannot be stored on this'
+                    ' platform for this repo format' % (file_id,))
             if repo._format.rich_root_data:
-                root_commit = (tree.get_root_id(),) + revid
+                root_commit = (tree.get_root_id(),) + rev_key
                 keys = set([root_commit])
                 parents = {root_commit:()}
             else:
                 keys = set()
                 parents = {}
-            keys.add(file_id + revid)
-            parents[file_id + revid] = ()
+            keys.add(file_key + rev_key)
+            parents[file_key + rev_key] = ()
             self.assertEqual(keys, set(repo.texts.keys()))
             self.assertEqual(parents,
                 repo.texts.get_parent_map(repo.texts.keys()))
@@ -175,22 +180,23 @@
         tree2 = self.make_branch_and_tree('tree2')
         tree2.pull(tree.branch)
         tree2.put_file_bytes_non_atomic('Foo:Bar', 'right\n')
-        right_id = (tree2.commit('right'),)
-        keys.add(file_id + right_id)
-        parents[file_id + right_id] = (file_id + revid,)
+        right_key = (tree2.commit('right'),)
+        keys.add(file_key + right_key)
+        parents[file_key + right_key] = (file_key + rev_key,)
         tree.put_file_bytes_non_atomic('Foo:Bar', 'left\n')
-        left_id = (tree.commit('left'),)
-        keys.add(file_id + left_id)
-        parents[file_id + left_id] = (file_id + revid,)
+        left_key = (tree.commit('left'),)
+        keys.add(file_key + left_key)
+        parents[file_key + left_key] = (file_key + rev_key,)
         tree.merge_from_branch(tree2.branch)
         tree.put_file_bytes_non_atomic('Foo:Bar', 'merged\n')
         try:
             tree.auto_resolve()
         except errors.UnsupportedOperation:
             pass
-        merge_id = (tree.commit('merged'),)
-        keys.add(file_id + merge_id)
-        parents[file_id + merge_id] = (file_id + left_id, file_id + right_id)
+        merge_key = (tree.commit('merged'),)
+        keys.add(file_key + merge_key)
+        parents[file_key + merge_key] = (file_key + left_key,
+                                         file_key + right_key)
         repo.lock_read()
         self.addCleanup(repo.unlock)
         self.assertEqual(keys, set(repo.texts.keys()))