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

Specification for read-only (or const) defines #101

Open
achilleas-k opened this issue May 29, 2024 · 4 comments
Open

Specification for read-only (or const) defines #101

achilleas-k opened this issue May 29, 2024 · 4 comments

Comments

@achilleas-k
Copy link
Member

While discussing the partition table generator with @mvo5, we considered the idea of having a way to signal that certain defines shouldn't be modified after they're set (even producing an error if they are). This is especially important for large, complicated data structures generated by externals where there are interdependencies between values (like the partition table).

Consider the following scenario:

  • The partition table external generates a fully-described PT structure (with UUIDs, starts and offets, etc) and stores it in ${filesystems}
  • A truncate stage is generated as:
otk.target.osbuild:
pipeline:
 disk:
  - type: org.osbuild.truncate
    options:
      filename: "whatever.disk"
      size: ${filesystems.partition_table.size}
  • The user decides to change the size of the partition table but due to a poor understanding of the mechanics of externals and defines, does the following:
otk.define.defaults:
  filesystems:
    otk.external.gen_partition_table:
      ...
      size: "10 GiB"
      ...
  
  ...
  # potentially in a different file
  filesystems.partition_table.size: "20 GiB"

otk.target.osbuild:
pipeline:
 disk:
  - type: org.osbuild.truncate
    options:
      filename: whatever.disk
      size: ${filesystems.partition_table.size}

If this is allowed, it'll throw off the the partition table consistency and partition positions and alignments. Changing other values could also make the whole thing unbuildable (which might be better or worse depending on POV).

It would be nice to have a way to protect values inside data structures generated by externals in some way, perhaps with a keyword convention (const?). So in the example above, the generator adds the const (or whatever) keyword to the data structure it produces, making the property ${fileystems.const...} and anything under const can't be modified after it's set the first time and doing:

otk.define.stuff:
  filesystems.const.partition_table.size: "5 KiB"

produces an error.

@mvo5
Copy link
Contributor

mvo5 commented May 29, 2024

Fwiw, I feel we don't need to enforce this programmatically initially, having a strong hint to the user that some defines should not be modified will probably be already very helpful (so picking a good name "const", "protected", "readonly" ....) is helpful :)

@supakeen
Copy link
Member

supakeen commented May 29, 2024

Is the current idea of warning on overwrite enough?

@supakeen
Copy link
Member

(ignore the cruft):

(venv) user@muja otk € otk -w duplicate-definition compile test.yaml
WARNING:otk.context:redefinition of 'a', previous values was 1 and new value is 2
...

@mvo5
Copy link
Contributor

mvo5 commented May 29, 2024

Haivng a way to warn about overrides is nice but it feels slightly orthogonal to me. It's hard to know before we actually have a full set of image definitions in otk but my gut feeling is that there will be quite a few overrides in any normal otk compile run. Plus even with it, it does not help with the discoverability that Achilleas mentioned, that a user with a misconception about how things work easily falls into a trap. But I guess it's fairly trivial to turn the override warning into an actual error for const (or protected, readonly, something`), but let's wait a little bit with the implementation until partitions and externals etc are a bit better understood (but my feeling is that this is going to be a nice pattern).

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

3 participants