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 directive: remove otk.meta and use target.* instead #53

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented May 7, 2024

This is (very much) a RFC/brainstorm commit. Right now the use of otk.meta is a bit unclear, there is an open bug about this (c.f. #2).

But it seems to me that meta is really about writing json data from an omnifest for a specific tool (in this case "composer"). Which makes me think that we have support for this already with out target directive. So otk -t osbuild-composer would generate the needed meta information.

This is (very much) a RFC/brainstorm commit. Right now the use
of `otk.meta` is a bit unclear, there is an open bug about this
(c.f. osbuild#2).

But it seems to me that `meta` is really about writing json data
from an omnifest for a specific tool (in this case "composer").
Which makes me think that we have support for this already with
out `target` directive. So `otk -t osbuild-composer` would generate
the needed meta information.
@mvo5 mvo5 requested a review from ondrejbudai May 7, 2024 08:52
@supakeen
Copy link
Member

supakeen commented May 7, 2024

I don't like this one, let's see what others think.

Currently whatever lives under otk.target.osbuild.<name> is translated directly into the manifest and if we do this we'll have to start ignoring keys that are invalid in that schema. The target is also not osbuild-composer, we're generating an osbuild manifest.

The otk.meta keys are that; additional information for tools that aren't the target to allow them to make assumptions about what targets exist and what to do with them.

@mvo5
Copy link
Contributor Author

mvo5 commented May 7, 2024

[..]

Currently whatever lives under otk.target.osbuild.<name> is translated directly into the manifest and if we do this we'll have to start ignoring keys that are invalid in that schema. The target is also not osbuild-composer, we're generating an osbuild manifest.

Maybe I'm misunderstanding but why would we start ignoring keys? The osbuild manifest would still be written via otk.target.osbuild.* and everything there needs to be valid. In additioon there is extra metadata that is not part of osbuild and only meaningful for osbuild-composer so for that extra meta-data the target is osbuild-composer as osbuild does not know about it or need it or can consume it. What am I missing or misrepresenting here?

The otk.meta keys are that; additional information for tools that aren't the target to allow them to make assumptions about what targets exist and what to do with them.

But the otk.meta keys are only meaningful for a specific consumer, otk.meta.*osbuild-composer*.boot_mode is targeted to osbuild-composer - I would love to understand what is different between target and meta that we need them as distinct concepts (and if we add them as distinct concepts, why they are different). I.e. it seems to me by just commiting this change we could still generated the metadata for osbuild-composer by just passing -t osbuild-composer to the otk file?

I may be wrong here of course, my knowledge about the metadata is from reading a bit in the composer source, I have not seen an actual file with an example :/

@mvo5 mvo5 changed the title directive: remove otk.meta and use target.* instead RFC directive: remove otk.meta and use target.* instead May 8, 2024
Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we all agreed on doing this through targets instead; if you could fix the conflict I'll ack :)

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