Merge lp:~jameinel/bzr/2.4-tree-root-dirstate-830947 into lp:bzr/2.4

Proposed by John A Meinel on 2013-05-19
Status: Merged
Approved by: John A Meinel on 2013-05-23
Approved revision: 6074
Merged at revision: 6075
Proposed branch: lp:~jameinel/bzr/2.4-tree-root-dirstate-830947
Merge into: lp:bzr/2.4
Diff against target: 53 lines (+14/-7)
2 files modified
bzrlib/dirstate.py (+10/-7)
doc/en/release-notes/bzr-2.4.txt (+4/-0)
To merge this branch: bzr merge lp:~jameinel/bzr/2.4-tree-root-dirstate-830947
Reviewer Review Type Date Requested Status
Richard Wilbur 2013-05-19 Approve on 2013-05-20
Review via email: mp+164617@code.launchpad.net

Commit message

Fix bug #830947. Dirstate.set_root_id() should update the id_index correctly.

The symptom is that if you have a tree with the file_id TREE_ROOT somewhere other than at '', it could cause 'bzr co' to fail because it tries to lookup the file in the id_index map, but it incorrectly points to a location ('') where the id never existed.

Description of the change

This fixes bug #830947.

Basically, there was a patch from Ian that I wasn't sure about, but we merged it. Turns out it introduced a bug. Specifically, when changing the root id of a newly created tree, it was forcing the old root id into the id_index map. However, that entry doesn't actually exist (because we are forcibly removing it.) So we don't want to add it.

update_minimal should be doing the right thing anyway.

This updates Dirstate._validate to validate the id_index more carefully.

I backported the patch the 2.4 because that is the first release that has the bug.

To post a comment you must log in.
Richard Wilbur (richard-wilbur) wrote :

Thanks for the fix and for back-porting it to 2.4. Does this affect 2.5 and trunk?
Seems like this would be a great candidate for merging into all affected, maintained versions.
+1

review: Approve
John A Meinel (jameinel) wrote :

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

On 2013-05-20 9:15, Richard Wilbur wrote:
> Review: Approve
>
> Thanks for the fix and for back-porting it to 2.4. Does this
> affect 2.5 and trunk? Seems like this would be a great candidate
> for merging into all affected, maintained versions. +1
>

Yes it does. Which is why I targeted 2.4 and intend to merge it up the
stack.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlGZyhcACgkQJdeBCYSNAAO06QCfZRz46AsH5cRhGp5LssIjUCHt
2wsAoLzd1Pu6MVxB2irZYDgN/yVAjulG
=DSG5
-----END PGP SIGNATURE-----

John A Meinel (jameinel) wrote :

sent to pqm by email

6075. By John A Meinel on 2013-05-23

Fix the release notes for bug #830947

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/dirstate.py'
2--- bzrlib/dirstate.py 2011-09-28 15:41:38 +0000
3+++ bzrlib/dirstate.py 2013-05-23 08:02:28 +0000
4@@ -2609,13 +2609,6 @@
5 self.update_minimal(('', '', new_id), 'd',
6 path_utf8='', packed_stat=entry[1][0][4])
7 self._mark_modified()
8- # XXX: This was added by Ian, we need to make sure there
9- # are tests for it, because it isn't in bzr.dev TRUNK
10- # It looks like the only place it is called is in setting the root
11- # id of the tree. So probably we never had an _id_index when we
12- # don't even have a root yet.
13- if self._id_index is not None:
14- self._add_to_id_index(self._id_index, entry[0])
15
16 def set_parent_trees(self, trees, ghosts):
17 """Set the parent trees for the dirstate.
18@@ -3328,10 +3321,20 @@
19 if self._id_index is not None:
20 for file_id, entry_keys in self._id_index.iteritems():
21 for entry_key in entry_keys:
22+ # Check that the entry in the map is pointing to the same
23+ # file_id
24 if entry_key[2] != file_id:
25 raise AssertionError(
26 'file_id %r did not match entry key %s'
27 % (file_id, entry_key))
28+ # And that from this entry key, we can look up the original
29+ # record
30+ block_index, present = self._find_block_index_from_key(entry_key)
31+ if not present:
32+ raise AssertionError('missing block for entry key: %r', entry_key)
33+ entry_index, present = self._find_entry_index(entry_key, self._dirblocks[block_index][1])
34+ if not present:
35+ raise AssertionError('missing entry for key: %r', entry_key)
36 if len(entry_keys) != len(set(entry_keys)):
37 raise AssertionError(
38 'id_index contained non-unique data for %s'
39
40=== modified file 'doc/en/release-notes/bzr-2.4.txt'
41--- doc/en/release-notes/bzr-2.4.txt 2012-06-08 06:59:50 +0000
42+++ doc/en/release-notes/bzr-2.4.txt 2013-05-23 08:02:28 +0000
43@@ -35,6 +35,10 @@
44 * Cope with Unix filesystems, such as smbfs, where chmod gives 'permission
45 denied'. (Martin Pool, #606537)
46
47+* Fix a traceback when trying to checkout a tree that also has an entry
48+ with file-id `TREE_ROOT` somewhere other than at the root directory.
49+ (John Arbash Meinel, #830947)
50+
51 * When the ``limbo`` or ``pending-deletion`` directories exist, typically
52 because of an interrupted tree update, but are empty, bzr no longer
53 errors out, because there is nothing for the user to clean up. Also,

Subscribers

People subscribed via source and target branches