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: Default values for otk.define and default values of ${} #17

Closed
supakeen opened this issue Apr 18, 2024 · 5 comments
Closed

RFC: Default values for otk.define and default values of ${} #17

supakeen opened this issue Apr 18, 2024 · 5 comments

Comments

@supakeen
Copy link
Member

supakeen commented Apr 18, 2024

It is common for current users of osbuild-mpp to need defaults for variables; see for example the CentOS Automotive default definitions and their selection.

This is used specifically in the definition part of the omnifest. Initially we had defined an otk.argument directive specifically for those things that are passed from outside: #7 which might cover some of the usecases here.

We currently forbid redefining an otk.define'd name with a new value, I propose we keep doing so as it leads to weird state on when things are available but get overwritten by things deeper in the tree.

I propose the following syntax for default variable values:

otk.default:
  name: "foo"
  list:
    - 1
    - 2

otk.define:
  name: "bar"

Would lead to any lookup of ${name} to be resolved to "bar" and a lookup of ${list} to be resolved to the default value of [1,2].

The other issue is usage, sometimes you want to take some action based on a value not existing. I propose this option:

Reintroduce otk.variable syntax. This is consistent with other directives that take arguments. The default is only used if list is not defined. Not when it is defined to something Falseyish.

otk.variable:
  name: list
  default:
    - 1
    - 2

Alternatively we could explore a system where an undefined variable is not an error but instead becomes falseyish, in that case it could work together with a future otk.op.if or otk.op.or.

@mvo5
Copy link
Contributor

mvo5 commented Apr 18, 2024

Is there a concrete (ideally small) real world example how this is used by automotive? Any link or file or pasted example would be most welcome :)

@supakeen
Copy link
Member Author

supakeen commented Apr 18, 2024

Not quite small but there are links to use by automotive in the OP. Here they are again:

  1. https://gitlab.com/CentOS/automotive/sample-images/-/blob/main/osbuild-manifests/include/defaults.ipp.yml
  2. https://gitlab.com/CentOS/automotive/sample-images/-/blob/main/osbuild-manifests/include/computed-vars.ipp.yml

Which are then used in the pipelines.

@mvo5
Copy link
Contributor

mvo5 commented Apr 18, 2024

Fwiw, I would like to explore the idea of nesting allowing to overwrite previously defined vars. We could provide a "otk --warn=overwriten" or "otk --warn-override=myvar". Then we don't need new syntax, the user would just do:

default.yaml
otk.variable:
 foo: "my-default"

fedora-39-amd64.yaml
include default.yaml

otk.variable:
 foo: "my-fedora-foo"

and going deeper the same. But given that I have not played with the examples yet maybe it's a bad idea (I just wonder if we can keep the syntax/constructs minimal and variable shadowing is something that programers are familiar with so it's not violating the principle of least surprise)

@supakeen
Copy link
Member Author

Right, that could live in the linter or logs.

And I assume that defines from the command line always win

@supakeen
Copy link
Member Author

No longer necessary after a long chat yesterday and the approval of #18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants