dirstate internals question

[Robert Collins]

On Tue, 2008-08-12 at 14:48 +1000, Martin Pool wrote:
> On Tue, Aug 12, 2008 at 1:10 PM, Robert Collins
> And this is in the context of https://bugs.launchpad.net/bzr/+bug/256409
> 
> As the docstring says, real_delete is set when this update to the
> dirstate indicates the entry is being removed rather than just moved.
> This code runs on the old name for this row, and the assertion is
> checking that the current tree minikind is a = absent.

Its worth noting that inventory deltas are less detailed: they simply
say
old_path, new_path, object

I think the code sets real_delete to differentiate from 'delete this row
in the dirstate due to a move' and 'delete this row in the dirstate due
to a delete'. So there is some heuristic in the calling code checking
this and setting real_delete appropriately.

> And this code is obviously mirrored from the next block down, handling
> the case of removal of entries *not* caused by deletes, where 'a' is
> not allowed.  It may be that I inserted that assertion when trying to
> refactor this code and it's not actually semantically needed.
> 
> I wonder why we have that requirement: if we're deleting it, why not
> just delete it?  Is it perhaps assuming that we only commit the
> removal of a file that has been removed from the wt?  That would be
> consistent with the docstring of update_basis_by_delta, which says
> that the delta can only have the effect of making the basis consistent
> with the wt.

I think the docstring is perhaps overly cautious, because commit of a
partial tree can result in a delta that does less than a full
synchronisation with the basis; and additionally we would benefit being
able to have pull() work in deltas eventually.

> If so, how do the tests for rm --keep pass?  They can pass because, rm
> has previously taken that file out of the working inventory.  It's
> still on disk but it's ignored (or unknown).  (But it might be useful
> to test this area more to reproduce the failure?)

The user in question had done an rm of a subtree before the commit that
raised an error, which does fit this area.

> On the other hand if we're marking the file absent, then separately
> going through to remove the row, that seems like an opportunity for
> improvement.
> 
> In almost every branch of _update_basis_apply_deletes, unless an error
> is raised, we end up deleting the entire entry from the dirblock,
> meaning that we no longer have any data about that filename in the
> working tree, basis tree, or any of the merged-in trees.

errata here - (filename, fileid) is the key for a entry in a dirblock. 

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080812/35a469e5/attachment.pgp