computing the resulting inventory from the merge changeset

[duchier at ps.uni-sb.de]

Robert Collins <robertc at robertcollins.net> writes:

> What is this used for ? I mean, I see where its used in the code, but
> whats the win ?

The main win is that I am starting to understand the issues and how
the merge code works ;-) In particular, I didn't feel at all confident
in my ability to perform the BzrCherrypickMetadata brain surgery for
merge without that understanding.

I think there is also the potential for code simplification.
Currently, regen_inventory requires to be handed an auxiliary
datastructure that describes changes in the form of a mapping from ids
to paths; it also performs lots of path-related string manipulations.

My version does not need this auxiliary datastructure: it directly
uses the changeset that we compute initially.  It also performs no
string manipulations.

I think that, instead of the aforementioned auxiliary datatructure,
apply_changeset should directly return a new inventory describing the
resulting tree.

I was also thinking that we may wish to associate metadata properties
with entries - metadata that perhaps cannot be directly attached to
files on the filesystem.  If such metadata resides in the inventory,
then changeset application must indeed compute the resulting inventory
using the changeset.  As designed, regen_inventory could not do it: or
at least it would need a more complex auxiliary datastructure; and
then why not use the merge changeset?

Btw, I haven't yet had time to look into it, but I have been wondering
what currently happens, on a system that cannot represent the
"execute" bit, when we perform a merge that involves a metadata change
of that bit? regen_inventory will compute an inventory that does not
reflect this change.

Using my technique, all metadata changes are reflected in the
resulting inventory.

>> === modified file 'bzrlib/inventory.py'
>> --- bzrlib/inventory.py 
>> +++ bzrlib/inventory.py 
>> @@ -407,6 +407,44 @@
>>          # first requested, or preload them if they're already known
>>          pass            # nothing to do by default
>>  
>> +    def check_eq(self, other):
>> +        if self.kind <> other.kind:
>> +            raise BzrError ("DENYS: %s (kind) %s -> %s" %
>> (self.file_id,self.kind,other.kind))
>> +        if self.name <> other.name:
>> +            raise BzrError ("DENYS: %s (name) %s -> %s" %
>> (self.file_id,self.name,other.name))
>> +        if self.parent_id <> other.parent_id:
>> +            raise BzrError ("DENYS: %s (parent_id) %s -> %s" %
>> (self.file_id,self.parent_id,other.parent_id))
>> +        #if self.revision <> other.revision:
>> +        #    raise BzrError ("DENYS: %s (revision) %s -> %s" %
>> (self.file_id,self.revision,other.revision))
>
> equality checking should always be done via __eq__.

Yes, and this is not full equality checking: it is equality in the
sense of what regen_inventory is computing, and that involves only a
subset of the available attributes.  Also, it is much preferable if
debugging code does not chance inadvertantly changing object semantics.
Redefining equality might very well have unexpected consequences ;-)

Cheers,

--Denys