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

Support the mkosi build type #1640

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Vogtinator
Copy link
Member

Unlike other recipe files, OBS detects this by matching the prefix rather than the suffix. As this would cause conflicts with e.g. mkosi.spec the build type detection had to be changed a bit.

IMO the mkosi.* glob is really inconvenient. It would be great if OBS could switch to *.mkosi or maybe *.mkosi.conf or similar. @bluca @mlschroe

@pep8speaks
Copy link

pep8speaks commented Oct 10, 2024

Hello @Vogtinator! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 788:121: E501 line too long (125 > 120 characters)

Comment last updated at 2024-11-05 15:15:58 UTC

@bluca
Copy link
Contributor

bluca commented Oct 10, 2024

The problem is that the upstream recipes are mkosi.*, so not sure how to square this circle unfortunately

@Vogtinator
Copy link
Member Author

The problem is that the upstream recipes are mkosi.*, so not sure how to square this circle unfortunately

The way I read the man page is that a mkosi.conf file will be used implicitly, but that's the only rule. You can pass any filename as config file, it doesn't need to start with mkosi..

We could also match only mkosi.conf and not accept any other file name, that would also avoid conflicts.

@Vogtinator
Copy link
Member Author

... now I understand what you mean - it's about preexisting recipe files that should be recognized by OBS as-is, without filename changes. I don't think that's solvable, as there can technically be a mkosi.kiwi or similar.

The PR as-is works, it's just a bit annoying with automatic recipe detection not working if there are multiple mkosi.* matches.

@bluca
Copy link
Contributor

bluca commented Oct 10, 2024

Yeah you'd need to write OBS-specific recipes. That might be fine, as an entry point, as the bulk of the recipes could be in auxiliary files which should just work - but please do test it, like having mkosi.conf.d/ together with a <special name> top level recipe. Also unless you are aware of people using this in production already, it might be ok to do backward incompat changes, if there are some benefits to be gained.

@bluca
Copy link
Contributor

bluca commented Oct 10, 2024

For myself, having better integration and being able to sign mkosi artifacts and build extensions based on pre-existing base images (dependencies) is way way more important than compatibility, so if this is necessary to get more development going, I have no objections on my side to changing the recipe names as needed.

@Vogtinator
Copy link
Member Author

Yeah you'd need to write OBS-specific recipes. That might be fine, as an entry point, as the bulk of the recipes could be in auxiliary files which should just work - but please do test it, like having mkosi.conf.d/ together with a <special name> top level recipe.

OBS doesn't really support subdirectories in sources, only with .obscpio archives which can't be inspected for e.g. dep resolution... It's possible that this changes now with git scmsync backed packages and projects, but I suppose plain osc packages should work still.

@dmach
Copy link
Contributor

dmach commented Oct 11, 2024

@Vogtinator I started working on a change in build recipe handling this week.
It's probably going to take me a while to finish it so let's merge this pull request first.

Using mkosi.conf sounds good to me. I prefer this over mkosi.*.
If multibuild support is needed, then we can extend it with mkosi.<flavor>.conf - which would be consistent with existing Containerfile.<flavor> and Dockerfile.<flavor>.

@bluca
Copy link
Contributor

bluca commented Oct 11, 2024

btw on the topic of OBS and mkosi, any chance this could get a quick look please? openSUSE/open-build-service#15823
This is needed for building images for multiple architectures

@Vogtinator
Copy link
Member Author

Using mkosi.conf sounds good to me. I prefer this over mkosi.*.

Ok, so for now I'll replace mkosi.* with mkosi.conf for recipe auto detection.

Should I revert the then unneeded refactoring for if -> elif?

Unlike other recipe files, OBS detects this by matching the prefix rather
than the suffix. As this would cause conflicts with e.g. mkosi.spec the
build type detection had to be changed a bit.
@Vogtinator
Copy link
Member Author

The darker linter complains about build.py - should I apply the rather large reformatting?

@dmach
Copy link
Contributor

dmach commented Nov 5, 2024

The darker linter complains about build.py - should I apply the rather large reformatting?

No, keep it as is, it's fine.

@Vogtinator
Copy link
Member Author

Using mkosi.conf sounds good to me. I prefer this over mkosi.*.

Ok, so for now I'll replace mkosi.* with mkosi.conf for recipe auto detection.

On second thought - we should keep the behaviour in sync with OBS and ATM that does mkosi.*. If we want to change the behaviour it should be done in OBS first (which is not that easy...)

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.

4 participants