[merge][rfc] small inventory apply_delta patch

John Arbash Meinel john at arbash-meinel.com
Tue Nov 11 09:50:32 GMT 2008


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

Martin Pool wrote:
> This patch goes a bit towards making BranchBuilder work with
> CHKInventory, but this particular small change is more conceptually
> interesting than useful.
> 
> I have some concern that in changing the internals of Inventory we'll
> hit some aliasing bugs, and history shows that they can be serious in
> impact, intermittently hit, and somewhat hard to track down.  Python
> is not well suited to using pure/immutable objects everywhere, but I
> do think it's important we be very clear about how and when these
> objects can be reused or mutated.
> 
> In this patch: MutableTree like most working trees has the concept of
> also making available a cached copy of the parent trees, and these are
> updated by commit through this method.  As you can see it gets the
> inventory from the old basis, updates it, and stores it back.
> 
> I would say this is not a good place to use mutation of the inventory.
>  After this is done, the mutated inventory is presumably still
> referenced by the object referenced by basis, and it seems strange or
> wrong to modify something held by that.  This is unlikely to be a
> problem in practice because this base implementation is rarely used in
> practice and anyhow it may be unlikely someone retains a reference to
> that object.  But still, we want to move towards apis that are hard to
> use wrongly and avoid relying on "probably that object's
> unreferenced."  So I'd say in this case, constructing a new one is
> better.
> 

I believe we discussed it a bit here, and I agree with you. The thought
I voiced here is that we probably want to get rid of mutating
inventories in place. I think we've been trying to move away from
operating on Inventories as a first-class object, to operating on Trees
and having them hide the details of the Inventory underneath.

For what it is worth, BB:approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkkZVWcACgkQJdeBCYSNAAPqsACfeWmI0T6XTqDMELEK1ZtlaPKX
XuAAoJZRxemKHvBsd0ghJs03Y5pP+k5Q
=Co53
-----END PGP SIGNATURE-----



More information about the bazaar mailing list