dirstate.set_state_from_inventory is broken with forward-edits.

Bug #395556 reported by Martitza
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
High
Robert Collins

Bug Description

bzr init t1
cd t1
touch 2
bzr add 2
bzr mv 2 1
bzr commit -m "call it 1"
bzr mv 1 2

At this point the dirstate is corrupt (bzr check should report this; tree._validate() does report it.

If we attempt to commit, we will get a traceback like the one reported below by Martitza in a comment on 2009-07-04. That is,

bzr commit -m "call it 2"

gives the traceback shown in the following comment.

Tags: dirstate
Revision history for this message
Martitza (martitzam) wrote :

The traceback follows. The file has zero length because I used 'touch' to craete the file but this is not relevant to the bug.

[15575] 2009-07-03 19:20:06.957 INFO: aborting commit write group: DuplicateFileId(File id {2-20090704021957-hr4tovbq40tdm0ia-1} already exists in inventory as InventoryFile('2-20090704021957-hr4tovbq40tdm0ia-1', '1', parent_id='TREE_ROOT', sha1='da39a3ee5e6b4b0d3255bfef95601890afd80709', len=0, <email address hidden>))
0.153 Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/bzrlib/commands.py", line 729, in exception_to_return_code
    return the_callable(*args, **kwargs)
  File "/usr/lib/python2.6/dist-packages/bzrlib/commands.py", line 924, in run_bzr
    ret = run(*run_argv)
  File "/usr/lib/python2.6/dist-packages/bzrlib/commands.py", line 560, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/usr/lib/python2.6/dist-packages/bzrlib/builtins.py", line 2937, in run
    exclude=safe_relpath_files(tree, exclude))
  File "/usr/lib/python2.6/dist-packages/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/usr/lib/python2.6/dist-packages/bzrlib/workingtree_4.py", line 226, in commit
    result = WorkingTree3.commit(self, message, revprops, *args, **kwargs)
  File "/usr/lib/python2.6/dist-packages/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/usr/lib/python2.6/dist-packages/bzrlib/mutabletree.py", line 228, in commit
    *args, **kwargs)
  File "/usr/lib/python2.6/dist-packages/bzrlib/commit.py", line 371, in commit
    self.builder.finish_inventory()
  File "/usr/lib/python2.6/dist-packages/bzrlib/repository.py", line 212, in finish_inventory
    self.parents)
  File "/usr/lib/python2.6/dist-packages/bzrlib/repository.py", line 1063, in add_inventory_by_delta
    basis_inv.apply_delta(delta)
  File "/usr/lib/python2.6/dist-packages/bzrlib/inventory.py", line 1169, in apply_delta
    self.add(new_entry)
  File "/usr/lib/python2.6/dist-packages/bzrlib/inventory.py", line 1259, in add
    self._byid[entry.file_id])
DuplicateFileId: File id {2-20090704021957-hr4tovbq40tdm0ia-1} already exists in inventory as InventoryFile('2-20090704021957-hr4tovbq40tdm0ia-1', '1', parent_id='TREE_ROOT', sha1='da39a3ee5e6b4b0d3255bfef95601890afd80709', len=0, <email address hidden>)

Revision history for this message
Martitza (martitzam) wrote :

I see that launchpad very removes email addresses to protect us against spam.

To make sure credit is given where due... the "Martin" who discovered this is the one who goes by gzlist.

Revision history for this message
Martitza (martitzam) wrote :

I should mention...

The bug depends on the first 'bzr mv' being applied while the file is in the added-but-not-comitted-yet state.

This may suggest that other ways of placing a file in a "pending" state may also cause issues if the pending changes are subjected to 'bzr mv'. I have not explored that.

~M

Revision history for this message
Martitza (martitzam) wrote :

Just to save some time:

The pass/fail status of the tests (promotion and demotion as defined above) does not depend on the order in which the tests are run. As strange as this bug is, at least it is not *that* strange.

Revision history for this message
Martin Pool (mbp) wrote : Re: [Bug 395556] Re: bzr mv (rename in place) causes commit to fail depending on specific filenames

2009/7/8 Martitza <email address hidden>:
> Just to save some time:
>
> The pass/fail status of the tests (promotion and demotion as defined
> above) does not depend on the order in which the tests are run.  As
> strange as this bug is, at least it is not *that* strange.

Thanks for checking that.

If you're keen, you could see if this same behaviour occurs in
previous releases.

I'll tweak Martin gzlist's tests and merge them.

--
Martin <http://launchpad.net/~mbp/>

Revision history for this message
Martin Pool (mbp) wrote : Re: bzr mv (rename in place) causes commit to fail depending on specific filenames

I've sent the tests patch to pqm.

The fact that the blackbox tests fail and the others pass is interesting; it may indicate something relating to an interaction between different wt and repository formats.

Changed in bzr:
importance: Undecided → High
status: New → Confirmed
Revision history for this message
Martitza (martitzam) wrote :

I have confirmed this issue affects formats 0.92 and 1.14 with bzr 1.16.1 on both ubuntu and win32.

I have not checked any other formats, but the presumption is that all formats are affected.

I have not checked any other versions of bzr, and I will do that later today if no one else does first.

Revision history for this message
Martitza (martitzam) wrote :

The win32 build of bzr 1.0 is not affected by this issue.
Further bisection results will follow...

Revision history for this message
Martitza (martitzam) wrote :

Bisection using the win32 stand-alone installers and format pack 0.92 reveals that the issue was introduced between bzr versions 1.13 and 1.14. The specific installers were bzr-setup-1.13-1.exe and bzr-setup-1.14-1.exe from https://bugs.launchpad.net/bzr/+download.

I have not attempted to narrow it down further yet, but this is probably enough for someone clever to isolate some code for review.

~M

Revision history for this message
Martitza (martitzam) wrote :

I have been able to spend only a few minutes on the code and spending most of that time learning a bit of the API. But maybe these notes will save an expert time in narrowing down how the changes between 1.13 and 1.14 may have caused the issue we are seeing.

I am currently focused on function apply_delta in inventory.py.

Where bzr 1.13 has

                new_entry.children = children.pop(new_entry.file_id, {})

bzr 1.14 has

                replacement = InventoryDirectory(new_entry.file_id,
                    new_entry.name, new_entry.parent_id)
                replacement.revision = new_entry.revision
                replacement.children = children.pop(replacement.file_id, {})
                new_entry = replacement

So the children are the same, naturally, but other new_entry members are being manipulated which were not before.

We get there in bzr 1.14 via finish_inventory ==> add_inventory_by_delta
whereas in bzr 1.13 we have finish_inventory ==> add_inventory

Here is the traceback from 1.14:

[ 2868] 2009-07-09 17:27:42.494 INFO: aborting commit write group: DuplicateFileId(File id {2-20090710002726-8f8ab83hol6cpulq-1} already exists in inventory as InventoryFile('2-20090710002726-8f8ab83hol6cpulq-1', '1', parent_id='TREE_ROOT', sha1='7e4a53de1f83762dd2febd39b818e2258bc83bc1', len=13, revision=martitza@rubuntu-20090710002735-37qmq4uutgy5l44s))
0.360 Traceback (most recent call last):
  File "bzrlib\commands.pyo", line 727, in exception_to_return_code
  File "bzrlib\commands.pyo", line 922, in run_bzr
  File "bzrlib\commands.pyo", line 559, in run_argv_aliases
  File "bzrlib\builtins.pyo", line 2878, in run
  File "bzrlib\decorators.pyo", line 192, in write_locked
  File "bzrlib\workingtree_4.pyo", line 226, in commit
  File "bzrlib\decorators.pyo", line 192, in write_locked
  File "bzrlib\mutabletree.pyo", line 228, in commit
  File "bzrlib\commit.pyo", line 371, in commit
  File "bzrlib\repository.pyo", line 212, in finish_inventory
  File "bzrlib\repository.pyo", line 1059, in add_inventory_by_delta
  File "bzrlib\inventory.pyo", line 1160, in apply_delta
  File "bzrlib\inventory.pyo", line 1250, in add
DuplicateFileId: File id {2-20090710002726-8f8ab83hol6cpulq-1} already exists in inventory as InventoryFile('2-20090710002726-8f8ab83hol6cpulq-1', '1', parent_id='TREE_ROOT', sha1='7e4a53de1f83762dd2febd39b818e2258bc83bc1', len=13, revision=martitza@rubuntu-20090710002735-37qmq4uutgy5l44s)

0.360 return code 3

tags: added: commit dirstate inventory
tags: removed: commit inventory
summary: - bzr mv (rename in place) causes commit to fail depending on specific
- filenames
+ bzr mv (rename in place) can corrupt dirstate with specific ordering
description: updated
Revision history for this message
Robert Collins (lifeless) wrote : Re: bzr mv (rename in place) can corrupt dirstate with specific ordering

The tree state is corrupted even in trees that do the dance successfully in trunk; commit has multiple code paths today, and I suspect that this is the root of the inconsistency. I'm moving the tests lower down.

Revision history for this message
Martitza (martitzam) wrote :

Robert updated the description to emphasize that the dirstate is corrupted by 'bzr mv' even before a subsequent commit.

But since the traceback in the comment on 2009-07-04 is actually generated in response to a second commit -- and because this is the context in which people are most likely to detect a problem (because bzr mv silently corrupts the dirstate) -- I have added some information to the end of the description.

description: updated
summary: - bzr mv (rename in place) can corrupt dirstate with specific ordering
+ dirstate.set_state_from_inventory is broken with forward-edits.
Changed in bzr:
status: Confirmed → Fix Committed
assignee: nobody → Robert Collins (lifeless)
status: Fix Committed → Confirmed
Revision history for this message
Martitza (martitzam) wrote :

It appears this may go even a little deeper! There is trouble even with no commit. Try this:

bzr init t1
cd t1
bzr check -q
touch 2
bzr add 2
bzr check -q
bzr mv 2 1
bzr check -q
ERROR: exceptions.AssertionError: entry (('', '2', '2-20090715011843-wl2n6j4yjvs4slgu-1'), [('r', '1', 0L, 0, '')]) has no data for any tree.

The specific ordering (demotion) is still required to trigger the defect.

Changed in bzr:
status: Confirmed → Fix Committed
Changed in bzr:
milestone: none → 1.18
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.