Issue 1060236: Multi extends which set the same option, and += behaviour

[benoitbryon]

This pull-request is related to https://bugs.launchpad.net/zc.buildout/+bug/1060236

This is an implementation of the proposal described as first example of the ticket above:

• when using multiple inheritance...
• bases are merged with each other
• distinct sections and options are preserved as is
• duplicate sections and options are combined

.. code-block:: cfg
   :filename: buildout.cfg

   [buildout]
   extends = base1.cfg base2.cfg
   parts += foo

.. code-block:: cfg
   :filename: base1.cfg

   [buildout]
   parts = base1

.. code-block:: cfg
   :filename: base2.cfg

   [buildout]
   parts = base2

We get parts = base1 base2 foo (in fact: "base1\nbase2\nfoo").

If the feature is accepted, I'm ok to add more documentation if you like.

[benoitbryon]

Important note: the feature seems to introduce notable changes, and could be considered as backward incompatible.
As an example, I had to adapt the doctests because the "multiple inheritance" behaviour changed. See doctests diff

[Natim]

For me extends bases shouldn't know about each other content.

I cannot see a use case when from extends you want to replace a part with a second extends.

I you want this behaviour, you would prefer to do something like this :

.. code-block:: cfg
   :filename: buildout.cfg

   [buildout]
   extends = base1.cfg base2.cfg
   parts = base2 foo

.. code-block:: cfg
   :filename: base1.cfg

   [buildout]
   parts = base1

.. code-block:: cfg
   :filename: base2.cfg

   [buildout]
   parts = base2

Then parts = base2 foo and you remove the magic.
For me this pull requests is a bug fix.

[claytron]

I think this changes the expected behavior of buildout quite a bit.

.. code-block:: cfg
   :filename: buildout.cfg

   [buildout]
   extends = base1.cfg base2.cfg
   parts += foo

.. code-block:: cfg
   :filename: base1.cfg

   [buildout]
   parts = base1

.. code-block:: cfg
   :filename: base2.cfg

   [buildout]
   parts = base2

If you have that config, you should end up with parts = base2 foo (as it is now), this is because the last config in the extends chain will set the parts value. So base2.cfg is overriding the value of base1.cfg.

I don't see why you would need the "magic" behavior as described in the pull request.

[mgedmin]

Can't you get what you want by saying parts += base2 in base2.cfg?

Personally, I find such implicit concatenation unintuitive. What happens if both base1.cfg and base2.cfg say newest = false? Will your code produce newest = false false?

[benoitbryon]

@claytron

I feel the current behaviour is "magic" too ;)
In your example, you don't tell base2.cfg to extend base1.cfg.
If you want to get "parts = base2 foo" where base2 overrides base1, then do you really need multiple inheritance since you can do:

.. code-block:: cfg
   :filename: buildout.cfg

   [buildout]
   extends = base2.cfg
   parts += foo

.. code-block:: cfg
   :filename: base1.cfg

   [buildout]
   parts = base1

.. code-block:: cfg
   :filename: base2.cfg

   [buildout]
   extends = base1.cfg
   parts = base2

My use case is to use the root buildout.cfg to deploy several components, which can collaborate, but run independantly from each other.

.. code-block:: cfg
   :filename: buildout.cfg

   [buildout]
   extends =
       documentation.cfg
       frontend.cfg
       backend.cfg

.. code-block:: cfg
   :filename: documentation.cfg

   [buildout]
   parts = sphinx

   [sphinx]
   recipe = z3c.recipe.scripts
   eggs = Sphinx

.. code-block:: cfg
   :filename: frontend.cfg

   [buildout]
   parts = frontend

   [frontend]
   recipe = z3c.recipe.scripts
   eggs = someapp

.. code-block:: cfg
   :filename: backend.cfg

   [buildout]
   parts = backend

   [backend]
   recipe = z3c.recipe.scripts
   eggs = anotherapp

I could get the result I expect by moving the "parts" directive in the root buildout.cfg, i.e.: parts = sphinx frontend backend... but the more I have bases (recursively), and the more bases declare parts, then the longer would be the root buildout... The feature proposal avoids that.

I can't get the result I expect by running buildout with several root configuration files (using the -c option), because I'd like to use the strategy recursively (higher-level components "extend" lower-level components...).

That said, I understand that's a significant change.

As told in https://bugs.launchpad.net/zc.buildout/+bug/1060236, my question is also: what is the use case for current implementation of multiple inheritance? I mean, why do people manage options override in multiple inheritance? And is there a way to "combine" several buildouts?

[benoitbryon]

@mgedmin

Can't you get what you want by saying parts += base2 in base2.cfg?

No. As told at https://bugs.launchpad.net/zc.buildout/+bug/1060236 :

• += doesn't give the expected result. This may be a bug. But since it currently doesn't work as expected, I really wonder whether people use this feature or not. I suppose the bug would have been reported before if it was an important feature.
• I'd like to write configuration files as if it was "root", i.e. use "=", and then, optionally, extend these files. Else, we have to use += almost everywhere, just in case the file is used as a base one day.

Personally, I find such implicit concatenation unintuitive. What happens if both base1.cfg and base2.cfg say newest = false? Will your code produce newest = false false?

In the proposed implementation, there is a de-duplication. I mean you'll get "newest = false".

Then, you are right, there is a problem if you have base1.cfg set "newest=true" and base2.cfg set "newest=false". You will get "newest = true false"...
But there, the clash would produce an error! Imho, that's the case where explicit (manual) overriding is important, and maybe mandatory. I mean, in case of a clash, I'll appreciate to be aware of the clash, then to fix it explicitely.

With current implementation, I defined "parts" in several bases. It generated clashes and implicit overrides. It took me time to detect the implicit overrides. It took me time to understand mutliple inheritance behaviour. I currently don't know an alternative to resolve my use case. That's why I created the ticket on Launchpad and this pull-request:

• get information about the multiple inheritance use-case(s)
• improve documentation about multiple inheritance
• fix the += current implementation if it's a feature of multiple inheritance
• or change implementation (add a feature) if that's not currently a significant feature

[lelit]

Thanks for working on this issue, as I had the very same troubles in a similar situation. I started looking at it a few months ago but didn't go too far due to focus change.

I agree we should either alter the implementation as you suggest, or improve the documentation on the current behavior possibly explaining how to handle such a case.

[benoitbryon]

Here is another use-case example. I feel it's closer to real life:

.. code-block:: cfg
   :filename: buildout.cfg

   [buildout]
   extends =
       versions.cfg
       frontend.cfg
       backend.cfg

.. code-block:: cfg
   :filename: versions.cfg

   [buildout]
   extensions = buildout-versions
   versions = versions

   [versions]
   # Placeholder for version pinning.

.. code-block:: cfg
   :filename: frontend.cfg

   [buildout]
   parts = frontend

   [frontend]
   recipe = z3c.recipe.scripts
   eggs = myfrontend Django

   [versions]
   Django = 1.4.1

.. code-block:: cfg
   :filename: backend.cfg

   [buildout]
   parts = backend

   [backend]
   recipe = z3c.recipe.scripts
   eggs = mybackend Django

   [versions]
   Django = 1.3.3

This example shows a potential use case for multiple inheritance and highlights 2 potential issues:

• How to make "parts" a combination of the "parts" in bases? This pull-request proposes that the example above results with [buildout] parts = frontend backend.
• Should Django's version pinned in frontend.cfg be overridden by backend.cfg silently? This pull-request proposes the example above results with [versions] Django = 1.4.1 1.3.3, which should generate an error. Human action seems required here.

As far as I understand multiple inheritance usage currently:

• bases ought to implement distinct features, preferably features you often use. Versions management, buildout options such as cache/newest, mr.developer, ... I mean, I don't see the use-case where we set "[buildout] newest = ..." in several bases. I feel only one base should deal with "newest" option, or we should override it explicitely in the root buildout configuration file.
• in most cases, bases use distinct sections and directives. No clashes, no issues.
• except some directives in [buildout] section and maybe other directives in "shared" sections such as [versions]. In that cases, I see 2 sub-cases:
  • the option accepts multiple values, like the "[buildout] parts" does. In that case, in current implementation the last value overrides others. This pull-request proposes to combine the values.
  • the option doesn't accept multiple values, like the "[versions] Django" in the example above. In that case, in current implementation, the last value overrides others and no error is raised. This pull-request proposes to combine the values, which should generate an error later: there is a clash, let's fix it manually. Notice that the error is raised by the recipe or extension which handles the value: if it can't handle multiple values, it should check whether the value contains newlines or not (or maybe whether the value has been computed from different configuration files, using buildout's internal notes, as used by buildout annotate).

Are there other usages for multiple inheritance?

[benoitbryon]

Updated the pull-request branch so that it remains fast-forward.

[reinout]

I'm solving it in a different way. It requires no buildout changes. It is nicely explicit, though that also might translate into "more work". It would look like this in your case:

[buildout]
extends =
documentation.cfg
frontend.cfg
backend.cfg
parts =
foo
${buildout:documentation-parts}
${buildout:frontend-parts}
${buildout:backend-parts}

And the doc/front/backend configs would look like this:

[buildout]
documentation-parts =
   sphinx
   whatever

[sphinx]
....

Such an extra documentation-parts attribute in the [buildout] section is fine. Would this approach work for you?

[benoitbryon]

Would this approach work for you?

Yes, it works.
But, I still don't find the buildout inheritance mechanism comprehensive.
That said, I understand the solution proposed in this pull-request is not the most interesting candidate, and surely won't be accepted.

I wrote an article to summarize solutions I know: http://tech.novapost.fr/buildout-multiple-extends-en.html but I forgot to mention it in this thread.
@reinout, the solution your are proposing is the same as http://tech.novapost.fr/buildout-multiple-extends-en.html#multiple-components-in-a-buildout isn't it?

What still puzzles me in buildout inheritance mechanism is that bases inherit each other. With current implementation, "A extends B and C" is handled exactly the same as "C extends B, and A extends C".
Explanation lives in this code:

buildout/src/zc/buildout/buildout.py

Line 1466 in 9e36ccb

_update(eresult, _open(base, fname, seen, dl_options, override,

(which is still in buildout 2.x) where:

• eresult is "options from first base"
• then, for "next base" in "other bases", eresult is updated with "options from next base"
• where _update() uses "+=" operator
• which actually makes base[1] extend base[0], ..., base[-1] extends base[-2]

IMHO, should be something like this:

• base_options = [options foreach base] # bases are parsed independantly of others
• eresult = merge_bases(base_options) # don't use += operator to merge bases

That said, I guess this is not in the scope of this pull request.
So, perhaps I'd better close this thread and open another about "bases shouldn't extend each other".

[benoitbryon]

That said, I guess this is not in the scope of this pull request.
So, perhaps I'd better close this thread and open another about "bases shouldn't extend each other".

Or, maybe I could update the implementation proposed in this pull-request.

[djay]

I suspect the real problem here is not so much multiple inheritance but +=.
If you have parts+= without having parts= in a base then it gets ignored. But then you have multiple inheritance you don't want to have parts= in multiple bases.

I think the solution is to make += work the same as = if you don't inherit from something that includes a =. That is intuitive to me.

[djay]

I think the problem lies in this code https://github.com/buildout/buildout/blob/master/src/zc/buildout/buildout.py#L1654

+= only gets looked at if two sections are being merged so a += in the base is ignored. In addition if you merge a += into a += the current merge only looks for key=, not key+=.

I think if we add the following line at the top of _update_section:

for key in list(s1.keys()):
   if key.endswith('+'):
        v = s1[key]
        key = k.rstrip(' +')
        s1[key] = v

[benoitbryon]

A note about my personal status on this ticket...

Short version: I am no longer planning to work on this story, so feel free to take over it.

Long version: I stopped using multiple inheritance feature of zc.buildout. I used templates (jinja2) to generate a single buildout.cfg file. The single file was a bit long, but very explicit. I think it was more readable. And template engines have features like "extend" and "include" when necessary. My team was used to template engines and not to buildout inheritance, so they were more comfortable with template engines... I also tried to limit buildout usage to simple cases, i.e. do not have very long configuration files... Later, I stopped using buildout for similar reasons: my coworkers use other tools.