[MERGE] Remove the basis_tree parameter to record_iter_changes.

Robert Collins robert.collins at canonical.com
Fri Mar 20 06:14:37 GMT 2009


On Fri, 2009-03-20 at 15:53 +1000, James Westby wrote:
> On Wed, 2009-03-18 at 13:01 +1100, Robert Collins wrote:
> > Why isn't this used by commit.py yet? Well, for xml based inventories
> > the dirstate is a much better place to get parent information - its
> > already there - and accessing inventories isn't that cheap. For CHK
> > based inventories, its extremely cheap to get a delta between the basis
> > and all the pending merges, so we only want to use this when the
> > repository is a CHK based repository (or other repository where it will
> > be faster than the current code).
> 
> The other night I did a hacksaw job of making commit.py use (a slightly
> older version of) this code. Some things that I noted along the way.

Thanks James.

>   * There was no reporting of the changes. It seems like 
>     record_iter_changes should do this, to save 2 passes over the 
>     changes.

I don't think so; its problematic layering.

def report_changes(changes):
    for change in changes:
        <mutter>
        yield change
...
record_iter_changes(report_changes(changes))

>   * There is no way to specify the exclude list to Tree.iter_changes, so
>     this either needs to be layered on top by converting to  
>     specific_files (bad), iter_changes extended, or record_iter_changes
>     made to exclude them.

For now:
   if exclude_list:
       old_code()
   else:
       new_code()

the fix is to teach iter_changes how to exclude. status and diff will
thus benefit from this.

>   * The current commit code expends some effort to go from 
>     specific_files to specific_file_ids, that doesn't seem totally 
>     redundant:

It should be - in what way isn't it? 

> > # If provided, ensure the specified files are versioned
> 
>   * Tree.add(['a/b']) where 'a' is not added leads to an iter_changes
>     that just adds 'a/b' without adding 'a', and the current code fails
>     to commit this as it never adds the parent. Something needs to 
>     change here.

Tree.add(['a/b']) should error loudly if a is not added - please file a
critical bug on this.
For tests, either add a and b, or use tree.smart_add which will add 'a'
if needed.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090320/22d55023/attachment.pgp 


More information about the bazaar mailing list