Bazaar

Merge lp://staging/~mbp/bzr/deprecation into lp://staging/bzr

  1. deprecation
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Work in progress
Proposed branch: lp://staging/~mbp/bzr/deprecation
Merge into: lp://staging/bzr
Diff against target: 503 lines (+119/-179)
7 files modified
bzrlib/add.py (+0/-95)
bzrlib/builtins.py (+16/-13)
bzrlib/mutabletree.py (+40/-18)
bzrlib/tests/per_workingtree/test_smart_add.py (+8/-16)
bzrlib/tests/test_smart_add.py (+2/-36)
bzrlib/tree.py (+47/-1)
doc/en/release-notes/bzr-2.3.txt (+6/-0)
To merge this branch: bzr merge lp://staging/~mbp/bzr/deprecation
Reviewer Review Type Date Requested Status
bzr-core Pending
Review via email: mp+38647@code.staging.launchpad.net

Description of the change

This cleans up some old and pretty crufty code around AddAction and smart_add. The existing api was sufficiently complicated, and as far I can tell not used by plugins, so I think it's better just to delete it. This is a bit cleaner on its own, but also detangles things a bit to allower a larger refactoring of smart_add to be smaller and to use an inventory delta rather than mutating the in-memory inventory. (bug 79336 asked for this a long time ago; it was worked-around in bzr-svn but it would be better to give them a simpler interface).

There are essentially two ways callers might want to customize smart_add: a way to generate file ids (for example to match those in an existing tree) and to report on what's being done (typically to a file). For no good reason they were tied together into a somewhat strange callable object callback. Instead, there are now just plain callables for each.

Before merging this I'll check whether it impacts other things that may be adding files or implementing smart_add, including bzr-svn, the package importer, and bzr builder. Generally things that are mechanically adding files shouldn't need to call smart_add.

It might be nice to have a reporter object that's told about more actions from inside add and can do more than just say the files that were added.

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

At the moment bzr-svn implements its own smart_add, which is a shame because I don't think there's anything very svn-specific in it. It ignores the 'action' parameter. This won't fix that but it might get closer to using a common implementation.

Reply
Revision history for this message
Robert Collins (lifeless) wrote :

It would also be good to check speed; add isn't a hugely common
operation, but benchmarkers do tend to test the time of 'bzr add' in
kernel/openoffice trees.

-Rob

Reply
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Hi Martin,

I think this is a nice improvement over the previous smart_add() overall.

For bzr-svn this is a useful improvement in that the action argument is no longer necessary. It still wouldn't be able to use the stock smart_add() because that calls _write_inventory().

The file_id_suggestion() callback on Tree seems a bit odd to me, mostly because of its signature. I wonder if it really makes sense as a public method on Tree (which already has a fair amount of methods) rather than as a helper method on cmd_add.

Reply
Revision history for this message
Martin Pool (mbp) wrote :

On 17 October 2010 21:52, Robert Collins <email address hidden> wrote:
> It would also be good to check speed; add isn't a hugely common
> operation, but benchmarkers do tend to test the time of 'bzr add' in
> kernel/openoffice trees.

Yes, good idea, I will. I may need to dust off usertest...

--
Martin

Reply
Revision history for this message
Martin Pool (mbp) wrote :

On 18 October 2010 10:54, Jelmer Vernooij <email address hidden> wrote:
> Hi Martin,
>
> I think this is a nice improvement over the previous smart_add() overall.

Great.

> For bzr-svn this is a useful improvement in that the action argument is no longer necessary. It still wouldn't be able to use the stock smart_add() because that calls _write_inventory().

Yes, I know. I am heading towards using an inventory delta instead,
which should make things easier for you, and this helps detangle it.

>
> The file_id_suggestion() callback on Tree seems a bit odd to me, mostly because of its signature. I wonder if it really makes sense as a public method on Tree (which already has a fair amount of methods) rather than as a helper method on cmd_add.

I was wondering about that too. I'll move it.

--
Martin

Reply

Unmerged revisions

5363. By Martin Pool

Move add-from-base-tree into file_id_suggestion method on Tree.

Delete remaining AddAction and all of bzrlib.add.

5362. By Martin Pool

Delete obsolete AddAction code

5361. By Martin Pool

Remove specific print-to-file arguments from smart_add in favor of reporter callback

5360. By Martin Pool

fix broken imports

5359. By Martin Pool

Document deprecation of smart_add(action=...)

5358. By Martin Pool

merge trunk

5357. By Martin Pool

trim imports

5356. By Martin Pool

news

5355. By Martin Pool

Delete obsolete and bad AddAction tests

5354. By Martin Pool

Don't pass an add action for simple command line add

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.