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

Proposed by John A Meinel on 2009-06-03
Status: Merged
Merged at revision: not available
Proposed branch: lp:~jameinel/bzr/1.16-simple-win32
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 71 lines
To merge this branch: bzr merge lp:~jameinel/bzr/1.16-simple-win32
Reviewer Review Type Date Requested Status
John A Meinel Approve on 2009-06-17
Review via email: mp+7046@code.launchpad.net

This proposal supersedes a proposal from 2009-06-02.

To post a comment you must log in.
John A Meinel (jameinel) wrote : Posted in a previous version of this proposal

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) 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.

Robert Collins (lifeless) 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

John A Meinel (jameinel) 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,))

John

Robert Collins (lifeless) 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

John A Meinel (jameinel) wrote :

-----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) 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

John A Meinel (jameinel) wrote :

-----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) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/tests/per_repository/test_repository.py'
2--- bzrlib/tests/per_repository/test_repository.py 2009-06-15 19:11:56 +0000
3+++ bzrlib/tests/per_repository/test_repository.py 2009-06-16 02:36:26 +0000
4@@ -151,22 +151,27 @@
5 """Test the basic behaviour of the text store."""
6 tree = self.make_branch_and_tree('tree')
7 repo = tree.branch.repository
8- file_id = ("Foo:Bar",)
9+ file_id = "Foo:Bar"
10+ file_key = (file_id,)
11 tree.lock_write()
12 try:
13 self.assertEqual(set(), set(repo.texts.keys()))
14- tree.add(['foo'], file_id, ['file'])
15- tree.put_file_bytes_non_atomic(file_id[0], 'content\n')
16- revid = (tree.commit("foo"),)
17+ tree.add(['foo'], [file_id], ['file'])
18+ tree.put_file_bytes_non_atomic(file_id, 'content\n')
19+ try:
20+ rev_key = (tree.commit("foo"),)
21+ except errors.IllegalPath:
22+ raise TestNotApplicable('file_id %r cannot be stored on this'
23+ ' platform for this repo format' % (file_id,))
24 if repo._format.rich_root_data:
25- root_commit = (tree.get_root_id(),) + revid
26+ root_commit = (tree.get_root_id(),) + rev_key
27 keys = set([root_commit])
28 parents = {root_commit:()}
29 else:
30 keys = set()
31 parents = {}
32- keys.add(file_id + revid)
33- parents[file_id + revid] = ()
34+ keys.add(file_key + rev_key)
35+ parents[file_key + rev_key] = ()
36 self.assertEqual(keys, set(repo.texts.keys()))
37 self.assertEqual(parents,
38 repo.texts.get_parent_map(repo.texts.keys()))
39@@ -175,22 +180,23 @@
40 tree2 = self.make_branch_and_tree('tree2')
41 tree2.pull(tree.branch)
42 tree2.put_file_bytes_non_atomic('Foo:Bar', 'right\n')
43- right_id = (tree2.commit('right'),)
44- keys.add(file_id + right_id)
45- parents[file_id + right_id] = (file_id + revid,)
46+ right_key = (tree2.commit('right'),)
47+ keys.add(file_key + right_key)
48+ parents[file_key + right_key] = (file_key + rev_key,)
49 tree.put_file_bytes_non_atomic('Foo:Bar', 'left\n')
50- left_id = (tree.commit('left'),)
51- keys.add(file_id + left_id)
52- parents[file_id + left_id] = (file_id + revid,)
53+ left_key = (tree.commit('left'),)
54+ keys.add(file_key + left_key)
55+ parents[file_key + left_key] = (file_key + rev_key,)
56 tree.merge_from_branch(tree2.branch)
57 tree.put_file_bytes_non_atomic('Foo:Bar', 'merged\n')
58 try:
59 tree.auto_resolve()
60 except errors.UnsupportedOperation:
61 pass
62- merge_id = (tree.commit('merged'),)
63- keys.add(file_id + merge_id)
64- parents[file_id + merge_id] = (file_id + left_id, file_id + right_id)
65+ merge_key = (tree.commit('merged'),)
66+ keys.add(file_key + merge_key)
67+ parents[file_key + merge_key] = (file_key + left_key,
68+ file_key + right_key)
69 repo.lock_read()
70 self.addCleanup(repo.unlock)
71 self.assertEqual(keys, set(repo.texts.keys()))