WT.update_basis_by_delta checks the current tree state to determine InvalidDelta

Bug #781168 reported by John A Meinel
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
High
John A Meinel

Bug Description

I'm just documenting what I'm running into, I'm not 100% sure what the correct fix is.

After working on bug #780544 I changed set_parent_trees() to call update_basis_by_delta when it could (when there was a single target tree, etc.)

This works in many cases, but fails with InvalidDelta sometimes.

For example:
 bzr init test
 cd test
 touch foo
 bzr add; bzr commit -m "added foo"
 touch bar
 bzr add; bzr commit -m "added bar"
 bzr uncommit --force

At this point, the WT should say that bar is versioned, but the basis tree should say that bar is unversioned (because it was just added). However, update_basis_by_delta says:
bzr: ERROR: An inconsistent delta was supplied involving 'bar', ...
reason: This was marked as a real delete, but the WT state claims that it still exists and is versioned.

It *is* a real delete, but it does still exist and is versioned.

Some thoughts:

1) Checking that the new basis tree matches the current tree state is intentional. (The code was originally written to support commit, which intends to create a tree that matches the current state.)

2) This *doesn't* match the concept of uncommit, which is intentionally changing the basis tree to be *different* from the current WT (without changing the WT).

3) Pull might be a better match, since it is mostly changing the tree to match. However, uncommitted changes have the possibility of causing conflicts without being an InvalidDelta. (You delete a file in your working tree, and then 'bzr pull' a patch that modifies it. This should create a conflict, but the delta saying the file was modified is perfectly valid.) Same with 'update'.

4) We could set a flag on update_basis_by_delta as to whether it should be checking the current tree state or not.
5) We could change update_basis_by_delta so that it always checks tree[1] aka the basis tree that it is actually modifying.

I'm thinking (4) is appropriate, because of the commit use case. I *think* the code is validating that the delta applied to the basis generates the same tree as the current WT state. Which is not valid for uncommit, and only loosely valid for pull and update.

Tags: dirstate

Related branches

Revision history for this message
John A Meinel (jameinel) wrote :

This is blocking landing the fix for bug #780544. So I'm putting it on my plate to figure it out. Being able to use a delta to update the WT basis tree is a huge improvement on large trees. (Drops pull time from 17s to 2-3s, uncommit times from 30s down to 2-3s, etc.)

Changed in bzr:
assignee: nobody → John A Meinel (jameinel)
Martin Pool (mbp)
Changed in bzr:
status: Confirmed → In Progress
John A Meinel (jameinel)
Changed in bzr:
milestone: none → 2.4b3
status: In Progress → 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.