openSUSE / osc

The Command Line Interface to work with an Open Build Service
http://openbuildservice.org/
GNU General Public License v2.0
170 stars 185 forks source link

Support the mkosi build type #1640

Closed Vogtinator closed 3 weeks ago

Vogtinator commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

Vogtinator commented 1 month ago

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 commented 1 month ago

... 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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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

Vogtinator commented 1 month ago

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?

Vogtinator commented 3 weeks ago

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

dmach commented 3 weeks ago

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

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

Vogtinator commented 3 weeks ago

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...)

dmach commented 3 weeks ago

LGTM, merging. Thanks for the patch!