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

Add way to easily persist CLI configuration #1993

Open
DaanDeMeyer opened this issue Oct 19, 2023 · 2 comments
Open

Add way to easily persist CLI configuration #1993

DaanDeMeyer opened this issue Oct 19, 2023 · 2 comments
Labels

Comments

@DaanDeMeyer
Copy link
Contributor

It'd be very useful if we could "remember" configuration passed via CLI between running multiple commands.

iam-TJ pushed a commit to iam-TJ/mkosi that referenced this issue Feb 10, 2024
Report the valid command-line configuration options.

Closes: systemd#1993
iam-TJ pushed a commit to iam-TJ/mkosi that referenced this issue Feb 10, 2024
Report the valid command-line configuration options.

Closes: systemd#1993
iam-TJ pushed a commit to iam-TJ/mkosi that referenced this issue Feb 10, 2024
Report the valid command-line configuration options.

Closes: systemd#1993
@DaanDeMeyer
Copy link
Contributor Author

Daan De Meyer, [10/19/23 9:51 AM]
So this just came to mind

Daan De Meyer, [10/19/23 9:52 AM]
What about adding a —persist option to write the final configuration as JSON to mkosi.json in the cwd

Daan De Meyer, [10/19/23 9:53 AM]
And then until —persist is specified again, whenever mkosi runs again, we use the config from mkosi.json merged with CLI arguments

Daan De Meyer, [10/19/23 9:56 AM]
Wait forget about persisting the final configuration

Daan De Meyer, [10/19/23 9:56 AM]
We'd persist the CLI configuration

Daan De Meyer, [10/19/23 9:58 AM]
What this solves

Daan De Meyer, [10/19/23 9:58 AM]
Is that currently, when I share mkosi one liners

Daan De Meyer, [10/19/23 9:58 AM]
They have to keep the full CLI at hand

Daan De Meyer, [10/19/23 9:58 AM]
You can't build the image

Daan De Meyer, [10/19/23 9:59 AM]
And then run it with mkosi qemu

Daan De Meyer, [10/19/23 9:59 AM]
Because mkosi qemu needs the full config as well

Daan De Meyer, [10/19/23 9:59 AM]
But if we had a —persist

J B, [10/19/23 9:59 AM]
I'm good with that

Daan De Meyer, [10/19/23 9:59 AM]
You'd only need the full CLI once

Daan De Meyer, [10/19/23 10:00 AM]
I wonder if this should be a verb or a CLI option

J B, [10/19/23 10:00 AM]
Why not both?

J B, [10/19/23 10:01 AM]
Nah, scratch that, just a verb

Daan De Meyer, [10/19/23 10:02 AM]
The thing is you might want to either nuke or extend the existing mkosi.json

J B, [10/19/23 10:03 AM]
Nuking could be done via mkosi persist --force and extending would just be doing CLI arguments, no?

Daan De Meyer, [10/19/23 10:03 AM]
Yeah I was thinking we'd reuse the existing —force argument

J B, [10/19/23 10:04 AM]
and if you'd want to extend the json config via config files we could add a hydrate verb to write out a config from JSON

Daan De Meyer, [10/19/23 10:04 AM]
Do we have a better name available than "persist"?

Daan De Meyer, [10/19/23 10:05 AM]
"serialize" is also kinda icky

J B, [10/19/23 10:05 AM]
dehydrate? :)

J B, [10/19/23 10:05 AM]
"Just add water!" ;)

Daan De Meyer, [10/19/23 10:07 AM]
The only annoying thing with a verb is I can't have that in my one liner

Daan De Meyer, [10/19/23 10:08 AM]
But probably a good thing

J B, [10/19/23 10:09 AM]
Before I said "scratch that" I was thinkng that one could do both

J B, [10/19/23 10:09 AM]
i.e. also have a --persist option for all the verbs that can trigger a build

J B, [10/19/23 10:10 AM]
but I think it would be cleaner to not have that, since then we will never serialize a config with persist: True

Daan De Meyer, [10/19/23 10:10 AM]
Yeah but I wonder if that could lead to misuse

J B, [10/19/23 10:10 AM]
That too

Daan De Meyer, [10/19/23 10:10 AM]
I'd guess --persist would be equal to -f persist

Daan De Meyer, [10/19/23 10:11 AM]
A separate command is probably better

Daan De Meyer, [10/19/23 10:11 AM]
Although

Daan De Meyer, [10/19/23 10:13 AM]
Let's do a separate verb

Daan De Meyer, [10/19/23 10:13 AM]
I can always do mkosi <config> persist && mkosi -f qemu

J B, [10/19/23 10:14 AM]
Yep

J B, [10/19/23 10:15 AM]
Also has the upside that the arg in the MkosiArgs is something we generally need to ignore anyway

Daan De Meyer, [10/19/23 10:16 AM]
Hmm so for this to work I'd need to serialize the CLI argparse namespace as json though

Daan De Meyer, [10/19/23 10:16 AM]
Or a full MkosiConfig object but without any defaults

J B, [10/19/23 10:17 AM]
I think we'd just serialize the full MkosiConfig, no?

Daan De Meyer, [10/19/23 10:17 AM]
The thing is we need to merge it with the config again when we read it

Daan De Meyer, [10/19/23 10:17 AM]
We don't want to serialize the config object

Daan De Meyer, [10/19/23 10:17 AM]
Because then people can't modify the config later

J B, [10/19/23 10:18 AM]
:D

J B, [10/19/23 10:18 AM]
I feel like we've had this conversation before

J B, [10/19/23 10:19 AM]
I don't think people should modify the config anymore. Once you persist, that should be an immutable artifact and even if you serialise defaults, that's good because than the person you're handing this to will get what you had.

Daan De Meyer, [10/19/23 10:20 AM]
I agree with that use case

Daan De Meyer, [10/19/23 10:20 AM]
But this is about the hacking use case

Daan De Meyer, [10/19/23 10:20 AM]
As in you run mkosi -d debian -t directory

Daan De Meyer, [10/19/23 10:20 AM]
Followed by mkosi qemu

Daan De Meyer, [10/19/23 10:20 AM]
And the mkosi qemu should use the debian image that was just built

Daan De Meyer, [10/19/23 10:21 AM]
But when I do that, I still want the full option to modify and mkosi config files and have those changes get applied as well

Daan De Meyer, [10/19/23 10:22 AM]
I don't want to modify a config file and then run mkosi persist to have it get picked up

J B, [10/19/23 10:22 AM]
I'm not sure I follow, let me try to rephrase that so you can tell me whether I got what you're saying

Daan De Meyer, [10/19/23 10:24 AM]
Let me give an example, if I'm hacking on something in systemd, I might run mkosi -d debian -t directory -f to build a debian directory image that I later want to boot in qemu

Daan De Meyer, [10/19/23 10:24 AM]
So the build is done, and then I run mkosi qemu to boot it

Daan De Meyer, [10/19/23 10:24 AM]
Except that won't boot the debian image

Daan De Meyer, [10/19/23 10:24 AM]
It will use any existing fedora image or build a new fedora image if there isn't one

J B, [10/19/23 10:25 AM]
ah, now I get it

J B, [10/19/23 10:26 AM]
but then I think a different solution might actually be better

J B, [10/19/23 10:27 AM]
that actually sounds a bit like we should resurrect writing the manifest on every build and persist the config in there

J B, [10/19/23 10:28 AM]
and by default we should operate on the image whose manifest we have there

Daan De Meyer, [10/19/23 10:29 AM]
The thing is the manifest is a per image thing

Daan De Meyer, [10/19/23 10:29 AM]
And ideally located in the output directory

Daan De Meyer, [10/19/23 10:29 AM]
Whereas this cannot be located in the output directory

J B, [10/19/23 10:29 AM]
totally harebrained: if we put the manifest next to the image and give it a corresponding name, we could have a symlink to that, giving the old mkosi.default semantics

J B, [10/19/23 10:29 AM]
(although I wouldn't reuse the name)

Daan De Meyer, [10/19/23 10:29 AM]
I'm not sure, this is fundamentally a cross image thing

Daan De Meyer, [10/19/23 10:30 AM]
Whereas manifests are per image

Daan De Meyer, [10/19/23 10:30 AM]
Think when you have multiple presets

J B, [10/19/23 10:32 AM]
How would you imagine that with multiple presets?

Daan De Meyer, [10/19/23 10:33 AM]
You'd either need to iterate on all the preset manifests to modify them with mkosi persist

Daan De Meyer, [10/19/23 10:33 AM]
Or have some kind of multi image manifest

Daan De Meyer, [10/19/23 10:33 AM]
Either way it seems rather messy

Daan De Meyer, [10/19/23 10:33 AM]
Whereas CLI config is inherently preset independent

Daan De Meyer, [10/19/23 10:34 AM]
So you'd just read the mkosi.json file first and then read the rest of the config as usual

J B, [10/19/23 10:35 AM]
I'm not convinced we should mix json and non-json config

J B, [10/19/23 10:35 AM]
that seems like a whole can of worms to me

Daan De Meyer, [10/19/23 10:36 AM]
Why would it be, we read the CLI JSON as an argparse namespace

Daan De Meyer, [10/19/23 10:36 AM]
And everything else stays the same

Daan De Meyer, [10/19/23 10:39 AM]
Maybe all of this is overkill

J B, [10/19/23 10:44 AM]
So my issue is that I think the tower is getting a bit high

J B, [10/19/23 10:45 AM]
I think that your suggestion is: mkosi.json -> config files -> CLI args

J B, [10/19/23 10:46 AM]
Whereas I'd like to keep it more flat: config files -> CLI args or mkosi.json -> CLI args

J B, [10/19/23 10:47 AM]
But I'm actually somewhat sure we're not talking about the same contents for mkosi.json

Daan De Meyer, [10/19/23 10:48 AM]
In my vision, mkosi.json would be populated with the JSON representation of the argparse namespace that we get after calling parse_arguments() on the argparser object

J B, [10/19/23 10:49 AM]
I would have just dropped MkosiArgs and MkosiConfig in there

Daan De Meyer, [10/19/23 10:49 AM]
Right but then you can't parse config files anymore if you're reading a mkosi.json file

Daan De Meyer, [10/19/23 10:49 AM]
So any changes in the config files will not be reflected

Daan De Meyer, [10/19/23 10:49 AM]
Which is bad for usability

Daan De Meyer, [10/19/23 10:50 AM]
Whereas if you only read the CLI namespace from mkosi.json, you can parse the config files

Daan De Meyer, [10/19/23 10:50 AM]
Because you'd be getting duplicates of many settings this way

Daan De Meyer, [10/19/23 10:50 AM]
Every list setting basically

J B, [10/19/23 10:51 AM]
I'm not sure. It avoids split-brain between the stuff on disk and the CLI invocation, which I think should be somewhat ephemeral

J B, [10/19/23 10:52 AM]
But I think we'e having multiple issues here

Daan De Meyer, [10/19/23 10:52 AM]
The entire thing I'm trying to fix here is to make the CLI stuff not ephemeral

Daan De Meyer, [10/19/23 10:54 AM]
For most of the settings it doesn't actually matter

Daan De Meyer, [10/19/23 10:54 AM]
But there are a few that I would like to be able to remember

Daan De Meyer, [10/19/23 10:54 AM]
The output format

Daan De Meyer, [10/19/23 10:54 AM]
And the distribution

Daan De Meyer, [10/19/23 10:54 AM]
Since those drastically affect the following call to mkosi qemu

J B, [10/19/23 10:55 AM]
yes

J B, [10/19/23 10:56 AM]
but wouldn't that be solved by writing out a manifest, persisting MkosiArgs and MkosiConfig there and having something point to the most recent manifest we built?

Daan De Meyer, [10/19/23 10:57 AM]
No because you might need multiple images

Daan De Meyer, [10/19/23 10:57 AM]
e.g. tools tree

Daan De Meyer, [10/19/23 10:58 AM]
Also, using some settings from MkosiConfig in mkosi qemu but not other because they might have been specified on the CLI seems very icky

J B, [10/19/23 10:59 AM]
True, but wouldn't this imply that for reproducibility we need to put all configs for the images we depend on in the manifest?

Daan De Meyer, [10/19/23 10:59 AM]
Currently the manifest is per image

Daan De Meyer, [10/19/23 10:59 AM]
We might need to change it to be cross image

J B, [10/19/23 10:59 AM]
I think we should do that anyway

Daan De Meyer, [10/19/23 11:00 AM]
But I still wouldn't tie these together so much, because you still want to be able to modify settings when invoking qemu

J B, [10/19/23 11:00 AM]
building multiple images would then mean that later manifests include all earlier ones, but i don't think that's an issue

J B, [10/19/23 11:03 AM]
When loading the config from the manifest, we could replace arbitrary keys form the CLI, so that wouldn't be tied to much?

Daan De Meyer, [10/19/23 11:08 AM]
Ehh I still don't like it, both CLI and updated config should be used

Daan De Meyer, [10/19/23 11:09 AM]
So maybe like this

Daan De Meyer, [10/19/23 11:10 AM]
We add —persist which when used with the build verb writes the CLI arguments as JSON, so we can remember them

Daan De Meyer, [10/19/23 11:10 AM]
And then when we run any other verb, we read the JSON again and use it

Daan De Meyer, [10/19/23 11:10 AM]
But we don't allow extending or stuff like that

Daan De Meyer, [10/19/23 11:10 AM]
And overwrite the JSON every time we do a new build

Daan De Meyer, [10/19/23 11:11 AM]
You can see the JSON as the symlink that allows us to find all the rest

J B, [10/19/23 11:12 AM]
I think I'm fine with that. It sounds sufficiently close to what I had in mind during this discussion.

Daan De Meyer, [10/19/23 11:13 AM]
Maybe we need to narrow down "any other verb"

Daan De Meyer, [10/19/23 11:13 AM]
I think it makes sense to use the JSON for qemu boot and ssh

Daan De Meyer, [10/19/23 11:13 AM]
Not sure about summary

J B, [10/19/23 11:13 AM]
Depending on what gets persisted in the file I'd maybe change the name to something like mkosi.cli.json to maybe keep the mkosi.json moniker for something else

Daan De Meyer, [10/19/23 11:14 AM]
Yes I was thinking the same

J B, [10/19/23 11:14 AM]
the build verbs in essence?

Daan De Meyer, [10/19/23 11:14 AM]
Right but summary kinda could use it as well

J B, [10/19/23 11:14 AM]
mhm, true, yeah

Daan De Meyer, [10/19/23 11:18 AM]
Opened an issue, this is going to be a bit more work than I thought so won't be able to get to it immediately

@keszybz
Copy link
Member

keszybz commented Feb 18, 2024

I like the conclusion of this chat.

What about the following workflow:

  • When executing any verb, --session=<name> can be specified.
  • -S is the same as --session=default.
  • (For documentation and help, this parameter is silently ignored and has no effect.)
  • When a verb is called with --session and configuration options, those options are added to any previous options for that session. So those options accumulate and accumulate. For example, you can say -p a, and then -p b, and you effectively and up with both a and b installed.
  • This means that if you do mkosi --session -p systemd -d cherokeelinux summary and then mkosi --session build and mkosi --session qemu, it all operates on the same cherokeelinux image with systemd installed.
  • This session config is saved to a single file somewhere, see discussion below.
  • We need a way to reset, I guess mkosi reset-session [<name>] or mkosi reset-session --all. Removing the file gives the same result.
  • The fact that we're using a session should be made visible in output, e.g. at the top, something like "Running session , inherited parameters: -d cherokeelinux -p systemd".

(Some options are not persisted, for example --force and --debug* and --json. In general stuff that is command-line only. But the exact list would need to be figured out.)

Maybe there should be just one session possible? Then the option of specifying <name> goes away and we can simplify the implementation. But I think the ability to have more than one session could be very useful, for example if one is trying to develop for multiple distros. By having a shortcut -S that works with one session makes this painless.

About the location of saving of the session state: it certainly shouldn't be per-output, because we want to have multiple outputs with almost the same config. OTOH, it shouldn't be global per-user, because sessions should not be shared between different unrelated mkosi projects. Something in between is needed. I could see tying it to the source directory, but source directory doesn't have to exist (it's totally OK to say mkosi -p fedora -r 38 -p systemd as the whole config), or it may not be writeable (e.g. mkosi-initrd). But I think the output directory would be OK. Generally it must exist and be writeable, and I think people generally create it once and let it persist. So maybe just <output_dir>/.mkosi-session.<name>.json is the location to use? (Also, it's normal to mix mkosi invocations with sudo, so it's not convenient to tie this any global user directory.)

I think this would work nicely for the way I use mkosi from the commandline. When working with a few output formats, this would effectively make the output format sticky, and require specifying a different output format once to switch, which seems reasonable.

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

No branches or pull requests

2 participants