Skip to content

Revert "Put build history into the output directory" - #4373

Merged
behrmann merged 1 commit into
systemd:mainfrom
bluca:revert
Jun 25, 2026
Merged

Revert "Put build history into the output directory"#4373
behrmann merged 1 commit into
systemd:mainfrom
bluca:revert

Conversation

@bluca

@bluca bluca commented Jun 24, 2026

Copy link
Copy Markdown
Member

Breaks the systemd CI, and also seems to make passing --output-directory mandatory on the command line if it is defined in the config, which is just horrible UX:

‣ /home/runner/work/systemd/systemd/mkosi/mkosi.local.conf: Setting OutputDirectory specified by specifier '%O' in %O/ovmf_vars_shim.fd is not yet set, ignoring
‣ /home/runner/work/systemd/systemd/mkosi/mkosi.local.conf: Setting OutputDirectory specified by specifier '%O' in %O/ovmf_vars_shim.fd is not yet set, ignoring
‣ Firmware variables file /ovmf_vars_shim.fd does not exist

https://github.com/systemd/systemd/actions/runs/28126107457/job/83290221148?pr=42739

This reverts commit da49fe9.

Breaks the systemd CI, and also seems to make passing --output-directory
mandatory on the command line if it is defined in the config, which is
just horrible UX:

‣ /home/runner/work/systemd/systemd/mkosi/mkosi.local.conf: Setting OutputDirectory specified by specifier '%O' in %O/ovmf_vars_shim.fd is not yet set, ignoring
‣ /home/runner/work/systemd/systemd/mkosi/mkosi.local.conf: Setting OutputDirectory specified by specifier '%O' in %O/ovmf_vars_shim.fd is not yet set, ignoring
‣ Firmware variables file /ovmf_vars_shim.fd does not exist

https://github.com/systemd/systemd/actions/runs/28126107457/job/83290221148?pr=42739

This reverts commit da49fe9.
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Claude review of PR #4373 (4ede625)

No issues found. This is a clean revert of commit da49fe9 which broke the systemd CI by introducing a circular dependency: config files using the %O specifier for OutputDirectory could not be parsed before OutputDirectory was resolved, effectively making --output-directory mandatory on the command line. The revert correctly restores the prior behavior.

Workflow run

@martinpitt

Copy link
Copy Markdown
Contributor

Thanks @bluca -- I'll put that on my todo list then, check how systemd uses that, and have another go at that.

@behrmann
behrmann merged commit 582eade into systemd:main Jun 25, 2026
49 of 54 checks passed
@behrmann

Copy link
Copy Markdown
Contributor

Unfortunate.

@bluca
bluca deleted the revert branch June 25, 2026 08:02
@martinpitt martinpitt mentioned this pull request Jun 25, 2026
7 tasks
@martinpitt

Copy link
Copy Markdown
Contributor

@bluca I'd like to reproduce this on my fork, but want to understand it first. systemd pins mkosi by SHA. The second-last time happened in systemd/systemd@936a790 on June 4, which didn't include any of #4360. The next commit was systemd/systemd@45f561b yesterday, which included both #4360 and this revert. So how did landing 4360 spontaneously break systemd CI? Thanks in advance!

@martinpitt

Copy link
Copy Markdown
Contributor

@bluca unping -- of course that happened in a PR, specifically systemd/systemd#42739 . Sorry for the noise! 🤗

@martinpitt

Copy link
Copy Markdown
Contributor

Got it -- sadly this is already quite fragile, and the change made that latent bug explode. mkosi.local.conf is parsed before mkosi.conf, so a %O in mkosi.local.conf cannot see the OutputDirectory= defined in mkosi.conf. It only resolves because a prior build's history pre-populates output_dir into context.cli before config parsing. On a clean systemd checkout this is visible even without the problematic commit (i.e. on current main). reproducing what CI configures:

❱❱❱ cat mkosi/mkosi.local.conf 
[Distribution]
Distribution=fedora

[Content]
Packages=shim-signed
ShimBootloader=signed

[Runtime]
Firmware=uefi-secure-boot
FirmwareVariables=%O/ovmf_vars_shim.fd

❱❱❱ mkosi summary
‣ /home/martin/upstream/systemd/mkosi/mkosi.local.conf: Setting OutputDirectory specified by specifier '%O' in %O/ovmf_vars_shim.fd is not yet set, ignoring
Firmware Variables: /ovmf_vars_shim.fd

⬆️ see the wrong path there.

The subsequent mkosi build would then "seed" --output-directory into the build history file, so that %O resolves it for the following mkosi vm/boot.

systemd's meson.build target passes --output-dir <builddir>/mkosi.output on the CLI, while every consume step (mkosi summary, the shim/VM runs) passes no -O and relies on OutputDirectory=../build/mkosi.output in mkosi.conf. That's pretty close to what the original commit advertised:

As a consequence, mkosi vm (and the other verbs that consume a previous build) now require -O/--output-directory when the build used one. This is a behaviour change, but unbreaks having more than one output dir.

... except I didn't consider mixing OutputDirectory= in the config and using -O on the CLI. AFAICS, they really have to coincide, otherwise calling mkosi in systemd sometimes with -O and sometimes without would just result in a mess.

It would be nice to only do one or the other in systemd to avoid the redundancy, but I think both are useful: the config option for human developers running mkosi vm outside of meson after a build; and the meson CLI option for making sure the images go into the configured build tree. Now, this relies on the build tree literally being build, as otherwise OutputDirectory=../build/mkosi.output would disagree with the -O option. (That's the redundancy).

But I think we can fix mkosi for this specific case.

@martinpitt

Copy link
Copy Markdown
Contributor

I sent #4376 with a new version of this. This time with unit tests, and confirmed to work against both mkosi/barrage and also systemd's own CI (ran on my fork).

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

Development

Successfully merging this pull request may close these issues.

3 participants