[MERGE] commit of removals respects filespec (fixes #46635)

John Arbash Meinel john at arbash-meinel.com
Tue Jul 11 20:30:41 BST 2006


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

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> I think I understand what you mean.
>>> deletion => Deleted from disk, but not removed from the working
>>> inventory (these are currently 'auto deleted')
>>> removal => was removed from working inventory with 'bzr rm'. may or may
>>> not exist on disk.
> 
> Right.
> 
>>> I think it would be reasonable to have a function that can build up a
>>> new inventory from two trees and a filespec.
>>>
>>> compare_trees() isn't perfect at this point, because it goes through
>>> 'list_files()' which has no way to represent missing files, so they
>>> currently get marked as deleted.
> 
> I think that would actually be okay, for the purpose of commit.
> 
> What irked me about this bug is it happened because we reimplemented
> tree comparison, which has already been implemented several times.  If
> we do it right, we should only need one implementation of tree
> comparison, and we can make sure that one is really good instead of
> dividing our efforts among several.  We can layer the existing
> interfaces on top of that.

Yeah. I think part of the dirstate changes will be refactoring this
stuff into a nicer set of comparision functionality.

> 
>>> Seems okay. +1 from me. I don't really like the double iteration stuff,
>>> but for now, it fixes the bug. And we can do it more correctly as we
>>> modify the api.
> 
> I think comparing two trees requires you to iterate through both of
> them.  The only choice is whether you do it serially or in parallel.

Well, since we already had to do that, it seems like you are adding
another loop over them to catch things that we've missed. Or is it that
the commit builder didn't actually do any comparison, since it was
actually just building up a 'current state'. (Which is trickier when you
have partial changes, so you need to carry over your old state).

> 
> Another way of implementing this functionality would be to copy the base
> inventory, then iterate through the working inventory, and apply the
> changes in the working inventory for the specified files.  But copying
> the base inventory would require iteration, too.
> 
> Aaron

I think we ultimately want to update list_files() so that it does a
double iteration over its working inventory, and the filesystem. That
way it can detect missing files.
Then you add another double iteration against the basis-inventory so
that you can detect working changes (three trees iterated now).
For the dirstate changes we want to create an api that lets us do this
nicely, since we will be caching the basis inventory in the dirstate
file to make comparisons fast.
Actually, we might have more than 1 basis inventory in the case of
merges, so we may need to iterate 4+ trees at the same time, to generate
2+ deltas.

The only time we don't have to iterate the whole tree is if we start
utilizing deltas between trees. (For example RevisionTree against its
parent could probably just extract the knit diff, and process that,
rather than generating 2 full inventories and double iterating).

John
=:->

PS> Hopefully the pqm doesn't falsely reject your submission. So far
today I've had 2 falsely failed tests because of the hashcache issues.
And having to wait for a LP test in between submissions is really painful.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEs/xhJdeBCYSNAAMRAh7aAKCTIo1JXyJfwTJrfsWoV/SjZrZ6owCdEUm+
q6lXDH2JTVnPR6vaFG/Zm1I=
=qc+6
-----END PGP SIGNATURE-----




More information about the bazaar mailing list