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

Improve parsing of local mkosi config #3572

Merged
merged 1 commit into from
Mar 3, 2025
Merged

Conversation

septatrix
Copy link
Contributor

Clarify that mkosi.local shall be a directory with a mkosi.conf file inside of it.

@septatrix septatrix marked this pull request as draft March 2, 2025 20:21
@septatrix
Copy link
Contributor Author

I think the cleaner fix is to also unconditionally parse mkosi.local/ iff it is a dir to allow stuff like mkosi.local/mkosi.conf.d/... without requiring a mkosi.conf file for detection

@septatrix septatrix marked this pull request as ready for review March 2, 2025 20:53
@septatrix septatrix changed the title Clarify parsing of mkosi.local Improve parsing of local mkosi config Mar 2, 2025
This aligns more with what the users expects
and allows working around some limitations of the config system
(esp. regarding the interplay of `Include=` and `Profiles=`).
@DaanDeMeyer DaanDeMeyer merged commit 0a9fcb2 into systemd:main Mar 3, 2025
35 checks passed
@septatrix
Copy link
Contributor Author

To expand on the "and allows working around some limitations of the config system", the problem is when one wants to locally include something which uses matches or profiles that are set by mkosi.conf.

# File: mkosi.conf
[Distribution]
Distribution=arch

# File: mkosi.local/mkosi.conf.d/20-include-tools.conf
[Include]
Include=mkosi-tools

This can now be worked around by putting the distro configuration at mkosi.local/mkosi.conf.d/10-default-distro.conf and ensuring that this file is tracked by git. Not a very pretty solution but it enables a few things that are otherwise not possible. Every other solution (e.g. postpone parsing of configs with [Matches]) I could think of had other disadvantages or just moves that problem to other scenarios

Comment on lines +330 to +331
This file or directory should be in `.gitignore` (or equivalent)
and is intended for local configuration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, late to the game, the "this file or directory" makes sense logically, but should probably be an "and", since above we now say that both are parsed. In the last line it should it should also read "are" instead of "is", since both are intended for local configuration.

Copy link
Contributor Author

@septatrix septatrix Mar 4, 2025

Choose a reason for hiding this comment

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

-> #3581

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

Successfully merging this pull request may close these issues.

3 participants