[MERGE] Remove the basis_tree parameter to record_iter_changes.

James Westby jw+debian at jameswestby.net
Sat Mar 21 10:43:10 GMT 2009


On Fri, 2009-03-20 at 17:14 +1100, Robert Collins wrote:
> >   * 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.

Yes, see my bug on "commit --show-diff" showing a confusing diff when -x
is used, as show_diff_trees doesn't take an exclude list yet, so I think
making the change to iter_changes is the right way to go.

> >   * 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

 ^ that's the comment from the file. There is a bit more discussion
(and an "XXX" IIRC), so it seems that this could be cleaned up at the
same time.

To me it reads like specific_files=["non-existent"] currently errors
in commit.py, but wouldn't error if passed to iter_changes. Not too
difficult to fix I feel.

> >   * 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.

I was remembering wrong, the failing test is
bzrlib.tests.test_commit.TestCommit.test_commit_new_subdir_child_selective
which adds both, but only specifies the child.

Thanks,

James




More information about the bazaar mailing list