nested trees design-approach : composite trees vs iter_changes

[Vincent Ladeuil]

>>>>> "robert" == Robert Collins <robert.collins at canonical.com> writes:

    robert> I've been concerned about CompositeTree as an approach for a while, but
    robert> we haven't been able to actually get anywhere in discussing it; today
    robert> Aaron asked broadly that I either veto, or stop complaining. I hadn't
    robert> wanted to veto the vote because that seems to block people vs trying to
    robert> change their mind; however Aaron feels that this will force some
    robert> discussion to resolve it... so I've rejected in BB, and this mail is
    robert> hopefully sufficient to get discussion going.

    robert> CompositeTree worries me for a number of reasons. 

    robert>  * Our behaviour will change in unexpected ways depending on whether
    robert>    a CompositeTree is being used.

Key point, +1 from me.

    robert> There is another concern, which is more weakly coupled to CT itself. I
    robert> think that CT makes this harder to change in the future, which is why I
    robert> mention it now:

    robert>  * The CT design enforces 'file ids are unique' across all the nested 
    robert>    trees; this perhaps not meant to be a long term constraint, but it 
    robert>    is one that we can't enforce - unlike our normal behaviour users
    robert>    will be able to make bzr break, and it won't be clear why, or how
    robert>    they should fix it (and arguably they won't have done anything
    robert>    wrong that would need fixing).

It also forbids using two (or more) different revisions of the
"same" nested tree. Imagine that you want to use the nested trees
for backward compatibility handling for example, by using a
nested tree for each released version of a particular
library. I'm sure other use cases exist.

    robert> Aaron has expressed concerns about supported nested trees inside our
    robert> tree code. As most recently expressed to me:

    robert>  * it makes everyone pay the cost of the feature
    robert>  * it makes recursion mandatory

    robert>  * it hides what's really going on/it makes the code
    robert>  trickier to debug.

That's what tests are for isn't it ? Whether we use CT or not,
the behaviors will have to be expressed in tests.

<snip/>

    robert>  * We can address the all-or-nothing issue by making recursion
    robert>    controlling parameters default to False initially, with a plan to 
    robert>    switch them to True as soon as that doesn't break the test suite.

Second key point and looks like the way to go.

Just my 2 cents,

     Vincent