smart add should not require a full inventory write.

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

Jelmer Vernooij wrote:
> ** Changed in: bzr-svn (upstream)
> Target: None => 0.4.0

WorkingTree._write_inventory was "privatized" after it was already in
use by TreeTransform and smart_add, and no substitute has been made
available.

I think the bug is that _write_inventory is private.

Aaron

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

iD4DBQFGAVYF0F+nu1YWqI0RAiZUAKCF5DyV1cOf+lsGPkJ2ovSQofigvwCYm+o2
UblLTHxjMgL0knOBjRcLxA==
=KhV8
-----END PGP SIGNATURE-----

On Wed, 21 Mar 2007 15:57:06 -0000
Aaron Bentley <email address hidden> wrote:
> WorkingTree._write_inventory was "privatized" after it was already in
> use by TreeTransform and smart_add, and no substitute has been made
> available.
>
> I think the bug is that _write_inventory is private.
Implementing _write_inventory will be a major pain for bzr-svn, but I
can see why it's being used in smart_add() and TreeTransform. Something
like it would be required for merge and revert in working copies
in bzr-svn anyway.

  summary "WorkingTree._write_inventory() should be public"

Well, we are actually trying to move away from having Inventories when possible. WT4 only has an inventory on demand (because some other api expects one).

I think a better thing would be Aaron's other idea of 'apply_delta'. So that you could pass a WorkingTree a bunch of paths that need to be added and removed, and have the WT take care of it. Rather than doing a wholesale "here is what you have now" api.

It seems pretty reasonable to update smart_add_tree and TreeTransform to use that sort of api.

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

Jelmer Vernooij wrote:
> On Wed, 21 Mar 2007 15:57:06 -0000
> Aaron Bentley <email address hidden> wrote:
>> WorkingTree._write_inventory was "privatized" after it was already in
>> use by TreeTransform and smart_add, and no substitute has been made
>> available.
>>
>> I think the bug is that _write_inventory is private.
> Implementing _write_inventory will be a major pain for bzr-svn, but I
> can see why it's being used in smart_add() and TreeTransform. Something
> like it would be required for merge and revert in working copies
> in bzr-svn anyway.
>
> summary "WorkingTree._write_inventory() should be public"

Robert and I have just discussed a possible interface to substitute for
_write_inventory.

It would be called WorkingTree.apply_inventory_delta(). Its argument
would be a list of tuples of (old_path, new_path, new_entry). The new
entry would be a new InventoryEntry of the correct type, which whatever
updated fields were necessary. The delta would be applied as a single
operation, not as a sequence. So it would be legal to supply:

[('path_a', 'path_b', entry_1), ('path_b', 'path_a', entry_2)]

Even though applying the operations sequentially would result in entry_1
being renamed on top of entry_2, this would not happen because the delta
is considered to be applied atomically.

For WorkingTree2/3, this would just use write_inventory internally. For
WorkingTree4, I think Robert will write something.

Does this proposed interface make sense for you? Would it be reasonably
easy to implement on SVN?

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

iD8DBQFGAv1S0F+nu1YWqI0RAga8AJ9X5eOzEMX1W0ZuxcnuObfB90CcHwCfdahQ
hPLekf63uAClN4MsAFziAt4=
=fNym
-----END PGP SIGNATURE-----

On Thu, 22 Mar 2007 18:04:02 -0400
Aaron Bentley <email address hidden> wrote:
> Jelmer Vernooij wrote:
> > On Wed, 21 Mar 2007 15:57:06 -0000
> > Aaron Bentley <email address hidden> wrote:
> >> WorkingTree._write_inventory was "privatized" after it was already
> >> in use by TreeTransform and smart_add, and no substitute has been
> >> made available.
> >>
> >> I think the bug is that _write_inventory is private.
> > Implementing _write_inventory will be a major pain for bzr-svn, but
> > I can see why it's being used in smart_add() and TreeTransform.
> > Something like it would be required for merge and revert in working
> > copies in bzr-svn anyway.
> >
> > summary "WorkingTree._write_inventory() should be public"
>
> Robert and I have just discussed a possible interface to substitute
> for _write_inventory.
>
> It would be called WorkingTree.apply_inventory_delta(). Its argument
> would be a list of tuples of (old_path, new_path, new_entry). The new
> entry would be a new InventoryEntry of the correct type, which
> whatever updated fields were necessary. The delta would be applied
> as a single operation, not as a sequence. So it would be legal to
> supply:
>
> [('path_a', 'path_b', entry_1), ('path_b', 'path_a', entry_2)]
>
> Even though applying the operations sequentially would result in
> entry_1 being renamed on top of entry_2, this would not happen
> because the delta is considered to be applied atomically.
>
> For WorkingTree2/3, this would just use write_inventory internally.
> For WorkingTree4, I think Robert will write something.
>
> Does this proposed interface make sense for you? Would it be
> reasonably easy to implement on SVN?
Yes, I think that would make it significantly easier. It would still
convert this sequence to a lock
+ a bunch of add/remove/copy operations.

Later on, when we've got our own parser of the .svn/ directory we could
modify the XML directly. That's still some time away though

Cheers,

Jelmer
--
Jelmer Vernooij <email address hidden>
http://samba.org/~jelmer/

Because of the way smart_add's save parameter behaves, using _write_inventory is really the only sane choice. See:

http://thread.gmane.org/gmane.comp.version-control.bazaar-ng.general/23876

Some other things could be cleaned up in smart_add too: it really should just generate a sequence of addable files, which can then be given to MutableTree.add, printed, or whatever.

  affects /products/bzr
  status fixreleased

smart_add() is now part of MutableTree so overridable.

  affects /products/bzr-svn
  status fixcommitted

Now overrides SvnWorkingTree.smart_add().