[MERGE] move smart_add_tree to MutableTree, tested on WorkingTree..

[Aaron Bentley]

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

Robert Collins wrote:
> This moves smart_add_tree as I discussed yesterday. Because its not
> fully refactored - it still uses inventory, and uses os specific
> functions not transport functions while it is on MutableTree (where i
> think it should be), its only functional on WorkingTree today.

In that case, I would expect MutableTree.smart_add_tree to raise
NotImplementedError, and WorkingTree to implement it.


> # Bazaar merge directive format 1
> # revision_id: robertc at robertcollins.net-20070703023836-\
> #   etkrpxm2rmbhqgaj
> # target_branch: http://bazaar-vcs.org/bzr/bzr.dev
> # testament_sha1: 78604b8a043e3386e4f782001f3118e92379c465
> # timestamp: 2007-07-03 12:39:32 +1000
> # source_branch: sftp://rookery/~/public_html/baz2.0/add

^^^ could you please set the public branch to one I can use?

> +    * ``bzrlib.add.glob_expand_for_win32`` is now
> +      ``bzrlib.win32utils.glob_expand``.  (Robert Collins)

This removes a public symbol without a deprecation period, so this would
reset the API compatibility to 0, right?

> +    * ``bzrlib.add.FastPath`` is now private and moved to 
> +      ``bzrlib.mutabletree._FastPath``. (Robert Collins, Martin Pool)

(As above)

> +    * New method ``_glob_expand_file_list_if_needed`` on the ``Command`` class
> +      for dealing with unexpanded glob lists - e.g. on the win32 platform. This
> +      was moved from ``bzrlib.add._prepare_file_list``. (Robert Collins)

This is probably correct, but it makes me wonder where else we should be
doing this, and how we can ensure that it happens in the right places as
we develop new code.  Any ideas?


vvv Could we shorten this name?  I think "file list" is implied.  I'd
like _maybe_glob_expand.

> +    def _glob_expand_file_list_if_needed(self, file_list):
> +        """Glob expand file_list if the platform does not do that itself.
> +        

> === modified file bzrlib/inventory.py // last-changed:robertc at robertcollins.net
> ... -20070703023332-jz0qdt2dwhzjuhlc
> --- bzrlib/inventory.py
> +++ bzrlib/inventory.py
> @@ -1047,6 +1047,10 @@
>                          child_dirs.append((child_relpath+'/', child_ie))
>              stack.extend(reversed(child_dirs))
>  
> +    def make_entry(self, kind, name, parent_id, file_id=None):
> +        """Simple thunk to bzrlib.inventory.make_entry."""
> +        return make_entry(kind, name, parent_id, file_id)

If you want it to be a method of Inventory, wouldn't it make sense to
migrate the code there?  bzrlib.inventory.make_entry can invoke
Inventory.make_entry if it's a static method.

> +from bzrlib.lazy_import import lazy_import
> +lazy_import(globals(), """
> +import os
> +
> +from bzrlib import add
> +from bzrlib import bzrdir
> +from bzrlib.osutils import dirname
> +from bzrlib.trace import mutter

Isn't it usual to do

from bzrlib import (
    add,
    bzrdir,
)

?

Also, isn't it a bad idea to combine direct imports with lazy_import?

> @@ -260,3 +270,246 @@
>              parent tree - i.e. a ghost.
>          """
>          raise NotImplementedError(self.set_parent_trees)
> +
> +    @needs_tree_write_lock
> +    def smart_add(self, file_list, recurse=True, action=None, save=True):
> +        """Version file_list, optionally recursing into directories.
> +
> +        This is designed more towards DWIM for humans than API clarity.
> +        For the specific behaviour see the help for cmd_add().
> +
> +        Returns the number of files added.

^^^ needs to be updated.

> +        :param save: Save the inventory after completing the adds. If False
> +            this provides dry-run functionality by doing the add and not saving
> +            the inventory.  Note that the modified inventory is left in place,
> +            allowing further dry-run tasks to take place. To restore the
> +            original inventory call self.read_working_inventory().

^^^ Retaining the modified inventory doesn't mesh well with dirstate
trees.  I think if we're introducing this as a new function, it would
make sense change this behavior.

> +        # validate user file paths and convert all paths to tree 
> +        # relative : its cheaper to make a tree relative path an abspath

^^^ "...it's cheaper..."


> === modified file bzrlib/workingtree.py // last-changed:robertc at robertcollins.n
> ... et-20070702224620-rptec516zwv3opwn
> --- bzrlib/workingtree.py
> +++ bzrlib/workingtree.py
> @@ -615,7 +615,7 @@
>          # should probably put it back with the previous ID.
>          # the read and write working inventory should not occur in this 
>          # function - they should be part of lock_write and unlock.
> -        inv = self.read_working_inventory()
> +        inv = self.inventory
>          for f, file_id, kind in zip(files, ids, kinds):
>              assert kind is not None
>              if file_id is None:
> @@ -623,7 +623,7 @@
>              else:
>                  file_id = osutils.safe_file_id(file_id)
>                  inv.add_path(f, kind=kind, file_id=file_id)
> -        self._write_inventory(inv)
> +            self._inventory_is_modified = True

It would be nice to use apply_inventory_delta instead.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGioJU0F+nu1YWqI0RAubPAJwMYN7M3276DRmMRB0WAPdf6CIkcACfbrov
BUpXjol/PBy0x+vRHCB7xRM=
=I7X5
-----END PGP SIGNATURE-----