Rev 2899: Fix incorrect comparison in Dirstate.set_state_from_inventory, and bug #146176 in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Tue Oct 9 09:08:23 BST 2007
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 2899
revision-id: pqm at pqm.ubuntu.com-20071009080820-civ61gexahohpats
parent: pqm at pqm.ubuntu.com-20071009044446-uliu5z9a52bzmps8
parent: mbp at sourcefrog.net-20071009040940-yjwm33zkzp033j8q
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2007-10-09 09:08:20 +0100
message:
Fix incorrect comparison in Dirstate.set_state_from_inventory, and bug #146176
modified:
bzrlib/_dirstate_helpers_c.pyx dirstate_helpers.pyx-20070503201057-u425eni465q4idwn-3
bzrlib/_dirstate_helpers_py.py _dirstate_helpers_py-20070710145033-90nz6cqglsk150jy-1
bzrlib/dirstate.py dirstate.py-20060728012006-d6mvoihjb3je9peu-1
bzrlib/tests/test_dirstate.py test_dirstate.py-20060728012006-d6mvoihjb3je9peu-2
------------------------------------------------------------
revno: 2872.1.1.1.2.1.11
merged: mbp at sourcefrog.net-20071009040940-yjwm33zkzp033j8q
parent: mbp at sourcefrog.net-20071009033533-zq8njirj71qmm2td
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Tue 2007-10-09 14:09:40 +1000
message:
Review documentation cleanups
------------------------------------------------------------
revno: 2872.1.1.1.2.1.10
merged: mbp at sourcefrog.net-20071009033533-zq8njirj71qmm2td
parent: mbp at sourcefrog.net-20071009024202-3t29natdi3lvwenl
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Tue 2007-10-09 13:35:33 +1000
message:
docstrings for cmp_ functions seem to be backwards
------------------------------------------------------------
revno: 2872.1.1.1.2.1.9
merged: mbp at sourcefrog.net-20071009024202-3t29natdi3lvwenl
parent: mbp at sourcefrog.net-20071008094021-d2mdlo8u4naiftv3
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Tue 2007-10-09 12:42:02 +1000
message:
Use cmp_by_dirs in set_state_from_inventory rather than hardcoding the equivalent
------------------------------------------------------------
revno: 2872.1.1.1.2.1.8
merged: mbp at sourcefrog.net-20071008094021-d2mdlo8u4naiftv3
parent: mbp at sourcefrog.net-20071008093743-b8m904v0grm602kw
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Mon 2007-10-08 19:40:21 +1000
message:
Clear up test code
------------------------------------------------------------
revno: 2872.1.1.1.2.1.7
merged: mbp at sourcefrog.net-20071008093743-b8m904v0grm602kw
parent: mbp at sourcefrog.net-20071008092834-oz18bb2l2y8eeem4
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Mon 2007-10-08 19:37:43 +1000
message:
More cleanups of dirstate work
------------------------------------------------------------
revno: 2872.1.1.1.2.1.6
merged: mbp at sourcefrog.net-20071008092834-oz18bb2l2y8eeem4
parent: mbp at sourcefrog.net-20071008092743-m5kd304bxitipjb9
parent: pqm at pqm.ubuntu.com-20071008081200-04a0byt5lo5tyft9
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Mon 2007-10-08 19:28:34 +1000
message:
merge trunk
------------------------------------------------------------
revno: 2872.1.1.1.2.1.5
merged: mbp at sourcefrog.net-20071008092743-m5kd304bxitipjb9
parent: mbp at sourcefrog.net-20071008092114-em6foez3xs0vfsr2
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Mon 2007-10-08 19:27:43 +1000
message:
doc
------------------------------------------------------------
revno: 2872.1.1.1.2.1.4
merged: mbp at sourcefrog.net-20071008092114-em6foez3xs0vfsr2
parent: mbp at sourcefrog.net-20071008091737-lbed3avpxvwsbeoh
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Mon 2007-10-08 19:21:14 +1000
message:
Check parameters to update_minimal
------------------------------------------------------------
revno: 2872.1.1.1.2.1.3
merged: mbp at sourcefrog.net-20071008091737-lbed3avpxvwsbeoh
parent: mbp at sourcefrog.net-20071005094528-yi0g5jbe7adconwv
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Mon 2007-10-08 19:17:37 +1000
message:
Fix comparison for merge sort in Dirstate.set_state_from_inventory
------------------------------------------------------------
revno: 2872.1.1.1.2.1.2
merged: mbp at sourcefrog.net-20071005094528-yi0g5jbe7adconwv
parent: mbp at sourcefrog.net-20071005093611-sxy5dwz4k01g3cq0
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Fri 2007-10-05 19:45:28 +1000
message:
Treat set_state_from_inventory as evil, and del dead variables rather than just setting to null
------------------------------------------------------------
revno: 2872.1.1.1.2.1.1
merged: mbp at sourcefrog.net-20071005093611-sxy5dwz4k01g3cq0
parent: mbp at sourcefrog.net-20070928075828-vp07yjxa2x29ckjq
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: 146176-dirstate
timestamp: Fri 2007-10-05 19:36:11 +1000
message:
Add xfail test for #146176
=== modified file 'bzrlib/_dirstate_helpers_c.pyx'
--- a/bzrlib/_dirstate_helpers_c.pyx 2007-08-02 21:00:51 +0000
+++ b/bzrlib/_dirstate_helpers_c.pyx 2007-10-09 03:35:33 +0000
@@ -196,9 +196,9 @@
:param path1: first path
:param path2: second path
- :return: positive number if ``path1`` comes first,
+ :return: negative number if ``path1`` comes first,
0 if paths are equal,
- and negative number if ``path2`` sorts first
+ and positive number if ``path2`` sorts first
"""
if not PyString_CheckExact(path1):
raise TypeError("'path1' must be a plain string, not %s: %r"
@@ -224,9 +224,9 @@
:param path1: first path
:param path2: the second path
- :return: positive number if ``path1`` comes first,
+ :return: negative number if ``path1`` comes first,
0 if paths are equal
- and a negative number if ``path2`` sorts first
+ and a positive number if ``path2`` sorts first
"""
if not PyString_CheckExact(path1):
raise TypeError("'path1' must be a plain string, not %s: %r"
=== modified file 'bzrlib/_dirstate_helpers_py.py'
--- a/bzrlib/_dirstate_helpers_py.py 2007-07-18 20:30:14 +0000
+++ b/bzrlib/_dirstate_helpers_py.py 2007-10-09 03:35:33 +0000
@@ -144,9 +144,9 @@
:param path1: first path
:param path2: second path
- :return: positive number if ``path1`` comes first,
+ :return: negative number if ``path1`` comes first,
0 if paths are equal,
- and negative number if ``path2`` sorts first
+ and positive number if ``path2`` sorts first
"""
if not isinstance(path1, str):
raise TypeError("'path1' must be a plain string, not %s: %r"
@@ -166,9 +166,9 @@
:param path1: first path
:param path2: the second path
- :return: positive number if ``path1`` comes first,
+ :return: negative number if ``path1`` comes first,
0 if paths are equal
- and a negative number if ``path2`` sorts first
+ and a positive number if ``path2`` sorts first
"""
if not isinstance(path1, str):
raise TypeError("'path1' must be a plain string, not %s: %r"
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2007-09-28 07:09:05 +0000
+++ b/bzrlib/dirstate.py 2007-10-09 04:09:40 +0000
@@ -1860,10 +1860,20 @@
:param new_inv: The inventory object to set current state from.
"""
+ if 'evil' in debug.debug_flags:
+ trace.mutter_callsite(1,
+ "set_state_from_inventory called; please mutate the tree instead")
self._read_dirblocks_if_needed()
# sketch:
- # incremental algorithm:
- # two iterators: current data and new data, both in dirblock order.
+ # Two iterators: current data and new data, both in dirblock order.
+ # We zip them together, which tells about entries that are new in the
+ # inventory, or removed in the inventory, or present in both and
+ # possibly changed.
+ #
+ # You might think we could just synthesize a new dirstate directly
+ # since we're processing it in the right order. However, we need to
+ # also consider there may be any number of parent trees and relocation
+ # pointers, and we don't want to duplicate that here.
new_iterator = new_inv.iter_entries_by_dir()
# we will be modifying the dirstate, so we need a stable iterator. In
# future we might write one, for now we just clone the state into a
@@ -1894,10 +1904,15 @@
if current_new_minikind == 't':
fingerprint = current_new[1].reference_revision or ''
else:
+ # We normally only insert or remove records, or update
+ # them when it has significantly changed. Then we want to
+ # erase its fingerprint. Unaffected records should
+ # normally not be updated at all.
fingerprint = ''
else:
# for safety disable variables
- new_path_utf8 = new_dirname = new_basename = new_id = new_entry_key = None
+ new_path_utf8 = new_dirname = new_basename = new_id = \
+ new_entry_key = None
# 5 cases, we dont have a value that is strictly greater than everything, so
# we make both end conditions explicit
if not current_old:
@@ -1912,6 +1927,9 @@
current_old = advance(old_iterator)
elif new_entry_key == current_old[0]:
# same - common case
+ # We're looking at the same path and id in both the dirstate
+ # and inventory, so just need to update the fields in the
+ # dirstate from the one in the inventory.
# TODO: update the record if anything significant has changed.
# the minimal required trigger is if the execute bit or cached
# kind has changed.
@@ -1923,8 +1941,9 @@
# both sides are dealt with, move on
current_old = advance(old_iterator)
current_new = advance(new_iterator)
- elif (new_entry_key[0].split('/') < current_old[0][0].split('/')
- and new_entry_key[1:] < current_old[0][1:]):
+ elif (cmp_by_dirs(new_dirname, current_old[0][0]) < 0
+ or (new_dirname == current_old[0][0]
+ and new_entry_key[1:] < current_old[0][1:])):
# new comes before:
# add a entry for this and advance new
self.update_minimal(new_entry_key, current_new_minikind,
@@ -1932,7 +1951,8 @@
path_utf8=new_path_utf8, fingerprint=fingerprint)
current_new = advance(new_iterator)
else:
- # old comes before:
+ # we've advanced past the place where the old key would be,
+ # without seeing it in the new list. so it must be gone.
self._make_absent(current_old)
current_old = advance(old_iterator)
self._dirblock_state = DirState.IN_MEMORY_MODIFIED
@@ -1998,15 +2018,22 @@
:param minikind: The type for the entry ('f' == 'file', 'd' ==
'directory'), etc.
:param executable: Should the executable bit be set?
- :param fingerprint: Simple fingerprint for new entry.
- :param packed_stat: packed stat value for new entry.
+ :param fingerprint: Simple fingerprint for new entry: sha1 for files,
+ referenced revision id for subtrees, etc.
+ :param packed_stat: Packed stat value for new entry.
:param size: Size information for new entry
:param path_utf8: key[0] + '/' + key[1], just passed in to avoid doing
extra computation.
+
+ If packed_stat and fingerprint are not given, they're invalidated in
+ the entry.
"""
block = self._find_block(key)[1]
if packed_stat is None:
packed_stat = DirState.NULLSTAT
+ # XXX: Some callers pass '' as the packed_stat, and it seems to be
+ # sometimes present in the dirstate - this seems oddly inconsistent.
+ # mbp 20071008
entry_index, present = self._find_entry_index(key, block)
new_details = (minikind, fingerprint, size, executable, packed_stat)
id_index = self._get_id_index()
=== modified file 'bzrlib/tests/test_dirstate.py'
--- a/bzrlib/tests/test_dirstate.py 2007-10-08 04:24:19 +0000
+++ b/bzrlib/tests/test_dirstate.py 2007-10-09 04:09:40 +0000
@@ -700,6 +700,67 @@
# This will unlock it
self.check_state_with_reopen(expected_result, state)
+ def test_set_state_from_inventory_preserves_hashcache(self):
+ # https://bugs.launchpad.net/bzr/+bug/146176
+ # set_state_from_inventory should preserve the stat and hash value for
+ # workingtree files that are not changed by the inventory.
+
+ tree = self.make_branch_and_tree('.')
+ # depends on the default format using dirstate...
+ tree.lock_write()
+ try:
+ # make a dirstate with some valid hashcache data
+ # file on disk, but that's not needed for this test
+ foo_contents = 'contents of foo'
+ self.build_tree_contents([('foo', foo_contents)])
+ tree.add('foo', 'foo-id')
+
+ foo_stat = os.stat('foo')
+ foo_packed = dirstate.pack_stat(foo_stat)
+ foo_sha = osutils.sha_string(foo_contents)
+ foo_size = len(foo_contents)
+
+ # should not be cached yet, because the file's too fresh
+ self.assertEqual(
+ (('', 'foo', 'foo-id',),
+ [('f', '', 0, False, dirstate.DirState.NULLSTAT)]),
+ tree._dirstate._get_entry(0, 'foo-id'))
+ # poke in some hashcache information - it wouldn't normally be
+ # stored because it's too fresh
+ tree._dirstate.update_minimal(
+ ('', 'foo', 'foo-id'),
+ 'f', False, foo_sha, foo_packed, foo_size, 'foo')
+ # now should be cached
+ self.assertEqual(
+ (('', 'foo', 'foo-id',),
+ [('f', foo_sha, foo_size, False, foo_packed)]),
+ tree._dirstate._get_entry(0, 'foo-id'))
+
+ # extract the inventory, and add something to it
+ inv = tree._get_inventory()
+ # should see the file we poked in...
+ self.assertTrue(inv.has_id('foo-id'))
+ self.assertTrue(inv.has_filename('foo'))
+ inv.add_path('bar', 'file', 'bar-id')
+ # this used to cause it to lose its hashcache
+ tree._dirstate.set_state_from_inventory(inv)
+ finally:
+ tree.unlock()
+
+ tree.lock_read()
+ try:
+ # now check that the state still has the original hashcache value
+ state = tree._dirstate
+ foo_tuple = state._get_entry(0, path_utf8='foo')
+ self.assertEqual(
+ (('', 'foo', 'foo-id',),
+ [('f', foo_sha, len(foo_contents), False,
+ dirstate.pack_stat(foo_stat))]),
+ foo_tuple)
+ finally:
+ tree.unlock()
+
+
def test_set_state_from_inventory_mixed_paths(self):
tree1 = self.make_branch_and_tree('tree1')
self.build_tree(['tree1/a/', 'tree1/a/b/', 'tree1/a-b/',
More information about the bazaar-commits
mailing list