Merge lp:~mbp/bzr/403523-status-crash into lp:~bzr/bzr/trunk-old

Proposed by Martin Pool
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/403523-status-crash
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 54 lines
To merge this branch: bzr merge lp:~mbp/bzr/403523-status-crash
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
John A Meinel Needs Fixing
Review via email: mp+9739@code.launchpad.net
To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

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

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/403523-status-crash into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>

No test cases?

I believe we have a per_inventory test suite, so you could test it at
that level.

Otherwise:

 review needs_fixing
 merge approve

John
=:->

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

iEYEARECAAYFAkp60x8ACgkQJdeBCYSNAAP0CACbBlSzUz04l/7piQcvTtWpc1Q0
V64An2icYIPmiAVlvdOGVxR8GJoXUztl
=60uL
-----END PGP SIGNATURE-----

review: Needs Fixing
Revision history for this message
Vincent Ladeuil (vila) wrote :

I was about to ask for tests then I saw https://bugs.edge.launchpad.net/bzr/+bug/403523/comments/1

From there I conclude you got that patch covered by some failing test[s].

review: Approve
Revision history for this message
Martin Pool (mbp) wrote :

It's true this is covered by a blackbox test in 2a, but I'll look at doing a per-inventory one. If it's hard I'll probably skip it though.

Revision history for this message
Robert Collins (lifeless) wrote :

> It's true this is covered by a blackbox test in 2a, but I'll look at doing a
> per-inventory one. If it's hard I'll probably skip it though.

test_inv has a comprehensive set of parameterised tests I added for apply_delta; should be trivial to add another case there - gimme a shout if its not obvious.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-08-11 02:58:23 +0000
+++ NEWS 2009-08-11 10:35:50 +0000
@@ -123,6 +123,9 @@
123 running send and similar commands on 2a formats.123 running send and similar commands on 2a formats.
124 (Martin Pool, #408201)124 (Martin Pool, #408201)
125 125
126* Fix crash in some invocations of ``bzr status`` in format 2a.
127 (Martin Pool, #403523)
128
126* Fixed export to existing directory: if directory is empty then export 129* Fixed export to existing directory: if directory is empty then export
127 will succeed, otherwise it fails with error.130 will succeed, otherwise it fails with error.
128 (Alexander Belchenko, #406174)131 (Alexander Belchenko, #406174)
129132
=== modified file 'bzrlib/inventory.py'
--- bzrlib/inventory.py 2009-08-05 02:30:59 +0000
+++ bzrlib/inventory.py 2009-08-11 10:35:50 +0000
@@ -743,6 +743,9 @@
743 """743 """
744 return self.has_id(file_id)744 return self.has_id(file_id)
745745
746 def has_filename(self, filename):
747 return bool(self.path2id(filename))
748
746 def id2path(self, file_id):749 def id2path(self, file_id):
747 """Return as a string the path to file_id.750 """Return as a string the path to file_id.
748751
@@ -1363,9 +1366,6 @@
1363 yield ie1366 yield ie
1364 file_id = ie.parent_id1367 file_id = ie.parent_id
13651368
1366 def has_filename(self, filename):
1367 return bool(self.path2id(filename))
1368
1369 def has_id(self, file_id):1369 def has_id(self, file_id):
1370 return (file_id in self._byid)1370 return (file_id in self._byid)
13711371
13721372
=== modified file 'bzrlib/status.py'
--- bzrlib/status.py 2009-03-23 14:59:43 +0000
+++ bzrlib/status.py 2009-08-11 10:35:50 +0000
@@ -156,11 +156,11 @@
156 to_file.write("%s %s\n" % (prefix, nonexistent))156 to_file.write("%s %s\n" % (prefix, nonexistent))
157 if (new_is_working_tree and show_pending):157 if (new_is_working_tree and show_pending):
158 show_pending_merges(new, to_file, short, verbose=verbose)158 show_pending_merges(new, to_file, short, verbose=verbose)
159 if nonexistents:
160 raise errors.PathsDoNotExist(nonexistents)
159 finally:161 finally:
160 old.unlock()162 old.unlock()
161 new.unlock()163 new.unlock()
162 if nonexistents:
163 raise errors.PathsDoNotExist(nonexistents)
164 finally:164 finally:
165 wt.unlock()165 wt.unlock()
166166