Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] otk: error when a otk.include inside a dict override the dict #176

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Jul 10, 2024

We allow the following:

otk.target.osbuild:
  sub:
    c: d
    otk.include: "foo.yaml"

The current semantic is that if the resolved value of foo.yaml is in itself a dict the two are merged and if it's a non-dict value the sub tree is replaced with the scalar value from foo.yaml.

However in the above example this can lead to unexpected results, i.e. depending on how foo.yaml resolves c: d is removed or kept.

This commit make it an error if an otk.include inside a non empty dict returns a scalar. We could consider also making it a warning but I fail to see a use-case right now (that is not contrived) where this would not be unexpected/unwanted behavior.

We allow the following:
```yaml
otk.target.osbuild:
  sub:
    c: d
    otk.include: "foo.yaml"
```
The current semantic is that if the resolved value of `foo.yaml` is in
itself a dict the two are merged and if it's a non-dict value
the `sub` tree is replaced with the scalar value from `foo.yaml`.

However in the above example this can lead to unexpected results,
i.e. depending on how `foo.yaml` resolves `c: d` is removed or
kept.

This commit make it an error if an `otk.include` inside a non
empty dict returns a scalar. We *could* consider also making it
a warning but I fail to see a use-case right now (that is not
contrived) where this would not be unexpected/unwanted behavior.
@mvo5 mvo5 requested a review from achilleas-k July 10, 2024 14:37
@supakeen supakeen added this pull request to the merge queue Jul 10, 2024
Merged via the queue into osbuild:main with commit 152f3cd Jul 10, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants