`charm build` with defined options overwrites instead of merging layer.yaml

[marcoceppi]

When supplying options in layer.yaml

includes:
  - 'layer:basic'
options:
  basic:
    packages:
      - tree
      - sl

and

includes:
  - layer:tester
options:
  basic:
    packages:
      - cowsay

the second layer wins with only cowsay being selected. Expected output would be ['tree', 'sl', 'cowsay']

[johnsca]

This was actually intentional, because otherwise there would be no way to override or remove list values, since lists would only be additive. (You can override dict values by specifying the same key again, so deep merging dicts doesn't have the same issue.) However, particularly with the dash list YAML syntax, it's unexpected (I've seen it surprise people several times now, including myself) and there isn't a good way to handle this use-case.

One way to handle this might be to support a custom schema setting that would indicate that a list value should be merged instead of overridden. The plus for this approach is that the schema seems a natural place to define this. The minuses are that it applies to the option for every layer (i.e., you couldn't merge by default but also support a layer flat-out overriding the list), that there's no way to remove an option from a merged list, and that it only works for layer opts. The first and second aren't issues for the package opt, because you really wouldn't want to remove a package that a lower layer depends on, but it might be a reasonable thing to do for some other list value. For the third, layer options are the most obvious and common use-case, but there might be others, e.g. the list of tags in metadata.yaml.

Another approach might be to have a special (e.g.) $layer-merge$ value that, if found in a list, would merge in the values from the previous layer. This would work for any YAML file and can be controlled on a per-layer basis, but has the potential for whatever special key we choose to conflict with a real value, and it means that the $layer-merge$ value must always be specified or all the previous merging will be dropped.

[marcoceppi]

I didn't realize the use case for removing values. If you ask me tags in metadata should be additive (I thought it was, actually). I can see now the problem and why we are where we are. I think creating a new type might be a worthy approach in the schema, combined-array is what I would go for or maybe adding a key to the array type which defines behavior.

packages:
  type: array
  description: Oh, you know
  behavior: (merge | replace)

I'll take a stab at this in the reactive library for a new type in the schema as I have a usecase for additive behavior.

[johnsca]

So, that's basically what I was suggesting with the first approach. And it would work pretty well for layer options, but doesn't address the issue with tags, which I agree should be additive which is why I called it out. Another area where this comes up which I think was the reason the override behavior was chosen is for config.yaml options and default list values. There are cases where we would want to add an additional value to a default list of values, and there are other cases where we might want to override the list completely or even remove a value.

Another point to consider is that there is support in layer.yaml for deleting keys from other layer's YAML files. This is to handle the case of a layer wanting to remove an interface, config option, etc. provided by a base layer, since the dict portions of YAML files are entirely additive, like we're considering having list values be.

Ideally, I'd like to have a single solution that covers all of the YAML files in a common, consistent way. I'd also like the modifications be as close to the values as possible; I'm a little unhappy with layer.yaml having values that modify other values in config.yaml.

[stub42]

@johnsca Do you have a use case for when the override behaviour in lists is desirable? This might make it clear what the default behaviour should be (or if override should be supported at all). My naive assumption would be that options designed to be overridden would default to an empty list.

Perhaps a list of software sources, where the layer provides defaults and consumers of the layer can tune it? Nah, the layer just needs to do 'if not config['sources']: sources = ['ppa:foo/bar']' and provide the default that way.

[johnsca]

@bcsaller can you provide a specific use-case example?

@stub42 while we could manage the default in the code like that, it makes the schema less self-documenting. It also fails if you have more than two layers, with the middle layer adding a value and the third layer wanting to replace one or more.

[bcsaller]

Maybe software deps. If for example you want to change the default list of
software + versions to a newer one, you don't want the lists combined by
default

On Mon, Jan 4, 2016 at 9:09 AM Cory Johns notifications@github.com wrote:

@bcsaller https://github.com/bcsaller can you provide a specific
use-case example?

@stub42 https://github.com/stub42 while we could manage the default in
the code like that, it makes the schema less self-documenting. It also
fails if you have more than two layers, with the middle layer adding a
value and the third layer wanting to replace one or more.

—
Reply to this email directly or view it on GitHub
#85 (comment).

[stub42]

@bcsaller Software dependencies is an interesting example. In this case, it is the charm using the layer that needs to decide if a list should be combined or overridden, rather than the layer insisting one way or the other. Does marco's solution allow the charm to decide the behaviour at build time, rather than it being hard coded in the layer?

[johnsca]

@stub42 It does not. Either the $merge$ magic value approach or extending the DSL in layer.yaml would, but I'm not really happy with either of those ideas.