Skip to content

mkosi: Move all mkosi configuration into mkosi/ subdirectory#36954

Merged
daandemeyer merged 4 commits intosystemd:mainfrom
daandemeyer:mkosi
Apr 3, 2025
Merged

mkosi: Move all mkosi configuration into mkosi/ subdirectory#36954
daandemeyer merged 4 commits intosystemd:mainfrom
daandemeyer:mkosi

Conversation

@daandemeyer
Copy link
Copy Markdown
Collaborator

Now that mkosi can automatically pick up its main configuration from
a mkosi/ subdirectory if it exists and there is no configuration in the
top level directory, let's make use of it to reduce the amount of clutter
in the top level directory of the repository.

This will also make it easier to install the mkosi configuration files as
part of the testing packages later on.

Copy link
Copy Markdown
Member

@yuwata yuwata left a comment

Choose a reason for hiding this comment

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

Super nice!

Copy link
Copy Markdown
Member

@yuwata yuwata left a comment

Choose a reason for hiding this comment

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

LGTM. But CIs are unhappy.

@yuwata yuwata added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed please-review PR is ready for (re-)review by a maintainer labels Apr 2, 2025
@yuwata yuwata added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Apr 2, 2025
@bluca bluca added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Apr 2, 2025
@yuwata
Copy link
Copy Markdown
Member

yuwata commented Apr 3, 2025

Fedora testing-farm:

+ mkdir systemd
+ rpm2cpio ./systemd-258~devel-1.20250402201735516803.pr36954.2552.gb0b65cc8cd.fc43.src.rpm
+ cpio --to-stdout --extract './*.tar.gz'
+ tar xz --strip-components=1 -C systemd
32401 blocks
+ pushd systemd
/var/ARTIFACTS/work-upstream6lb2aw_4/plans/upstream/discover/default-0/tests/systemd /var/ARTIFACTS/work-upstream6lb2aw_4/plans/upstream/discover/default-0/tests
+ git clone https://github.com/systemd/mkosi
fatal: destination path 'mkosi' already exists and is not an empty directory.
Shared connection to 3.20.227.44 closed.

@yuwata
Copy link
Copy Markdown
Member

yuwata commented Apr 3, 2025

Linter is red.

Also Ubuntu CI needs to be adjusted:

4844s + find /tmp/tmp.OsnkAb923g/apt/sources.list.d -type f -name *.sources
4844s + sed -i \|URIs:.*/tmp/autopkgtest.OkXdhE/binaries|i Enabled: no /tmp/tmp.OsnkAb923g/apt/sources.list.d/autopkgtest-ppa-upstream-systemd-ci-systemd-ci.sources
4844s + sed -i \|URIs:.*/tmp/autopkgtest.OkXdhE/binaries|i Enabled: no /tmp/tmp.OsnkAb923g/apt/sources.list.d/ubuntu.sources
4844s + tee mkosi.local.conf
4844s + git -C /tmp/autopkgtest.OkXdhE/autopkgtest_tmp/mkosi describe
4844s + tr -d v
4844s + systemd-analyze compare-versions 25.3-325-g600d847f lt 25
4844s + tee -a mkosi.local.conf
4844s + sed -i s/Format=btrfs/Format=ext4/ mkosi.repart/10-root.conf
4844s sed: can't read mkosi.repart/10-root.conf: No such file or directory

@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Apr 3, 2025
* b17e5d64a1 Don't try to modify mkosi.repart config if mkosi conf is in subdir
* e2b2ea3776 fmf: Use mkosi/mkosi.local.conf if the mkosi/ directory exists
Now that mkosi can automatically pick up its main configuration from
a mkosi/ subdirectory if it exists and there is no configuration in the
top level directory, let's make use of it  to reduce the amount of clutter
in the top level directory of the repository.

This will also make it easier to install the mkosi configuration files as
part of the testing packages later on.
Instead of always looking up two directories from the
test/integration-tests/meson.build file, let's search in up to 4
parent directories from the given meson project source root. This
allows us to just pass in meson.project_source_root() to
integration-test-wrapper.py instead of having to pass in a fixed
relative offset from the current meson file.

It'll also allow us to install the integration tests and mkosi
configuration in the future without breaking the standalone
integrationt tests functionality;
@yuwata yuwata added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Apr 3, 2025
@daandemeyer daandemeyer merged commit 0bcbb04 into systemd:main Apr 3, 2025
40 of 47 checks passed
@daandemeyer daandemeyer deleted the mkosi branch April 3, 2025 10:18
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Apr 3, 2025
@daandemeyer
Copy link
Copy Markdown
Collaborator Author

@fbuihuu So this gets us a little futher again. It should now be possible to install_subdir() the integration-tests directory and the mkosi directory when install-tests is enabled. mkosi will still want to write a few files to the directory it's invoked from though so I don't think it's quite possible to run the integration tests from /usr/lib/systemd/tests just yet. Can you explain why running them from an unpacked source rpm tarball doesn't work for you?

@fbuihuu
Copy link
Copy Markdown
Contributor

fbuihuu commented Apr 4, 2025

So this gets us a little futher again. It should now be possible to install_subdir() the integration-tests directory and the mkosi directory when install-tests is enabled.

Thank you!

mkosi will still want to write a few files to the directory it's invoked from though

May I ask which ones and why they can't be created in a directory specified in mkosi.conf such as BuildDirectory=?

Can you explain why running them from an unpacked source rpm tarball doesn't work for you?

Initially the SUSE systemd-testsuite sub-package has been introduced to test the systemd binaries of the host where the sub-package is installed and shipping the specific testsuite files via a binary package has 2 main advantages:

  • runtime dependencies of the testsuite are automatically installed on the host when the testsuite sub-package is installed
  • the size of the testsuite subpackage is smaller than the source rpm

@daandemeyer
Copy link
Copy Markdown
Collaborator Author

May I ask which ones and why they can't be created in a directory specified in mkosi.conf such as BuildDirectory=?

They certainly can, this just requires some more work. Currently mkosi.key, mkosi.crt and mkosi.tools, mkosi.tools.manifest are created in a non configurable location. Also, even if these were configurable, you can't pass arbitrary mkosi command line options to meson. I figure we could ship a config dropin to automatically read more configuration from /etc/systemd/tests/mkosi so you could add configuration there.

Initially the SUSE systemd-testsuite sub-package has been introduced to test the systemd binaries of the host where the sub-package is installed and shipping the specific testsuite files via a binary package has 2 main advantages:

So the thing is that even if we make the integration tests installable like this, mkosi almost completely isolates running the integration tests from the host on which the tests are running.

runtime dependencies of the testsuite are automatically installed on the host when the testsuite sub-package is installed

mkosi also takes care of this now so there's no need to encode dependencies required in the testsuite package. In fact it hurts us upstream because the systemd-testsuite is also installed inside the image, where we definitely don't want to pull in qemu and whatnot.

the size of the testsuite subpackage is smaller than the source rpm

I doubt the size difference is big enough to make a noticeable difference.

Please take a look at https://src.fedoraproject.org/rpms/systemd/blob/rawhide/f/plans/run-integration-tests.sh to see how we run the systemd integration tests against the official rpms built by the Fedora build infrastructure these days. Are you sure this can't work for your use case as well?

@fbuihuu
Copy link
Copy Markdown
Contributor

fbuihuu commented Apr 8, 2025

I doubt the size difference is big enough to make a noticeable difference.

It's 28M vs 95M. But yeah this doesn't really make a difference compare to the total size of the mkosi output files (> 5G).

Please take a look at https://src.fedoraproject.org/rpms/systemd/blob/rawhide/f/plans/run-integration-tests.sh to see how we run the systemd integration tests against the official rpms built by the Fedora build infrastructure these days. Are you sure this can't work for your use case as well?

I guess I don't really have the choice especially since it's been embraced by all other distros.

@daandemeyer
Copy link
Copy Markdown
Collaborator Author

I guess I don't really have the choice especially since it's been embraced by all other distros.

I'm open to suggestions for improvements, the current setup definitely isn't perfect, for example I just realized that using the source rpm isn't exactly sufficient since patches aren't applied to the tarball.

Maybe instead of explicitly requiring the systemd testsuite package to be "installed", we can just unpack it to a writable directory and run the tests from there.

@fbuihuu
Copy link
Copy Markdown
Contributor

fbuihuu commented Apr 8, 2025

Maybe instead of explicitly requiring the systemd testsuite package to be "installed", we can just unpack it to a writable directory and run the tests from there.

But I don't see why unpacking it would be better. Even if the files are installed in a read-only directory, they can still be used from a writable directory, it's just a matter of symlinking the relevant directories from the writable place, no?

@daandemeyer
Copy link
Copy Markdown
Collaborator Author

But I don't see why unpacking it would be better. Even if the files are installed in a read-only directory, they can still be used from a writable directory, it's just a matter of symlinking the relevant directories from the writable place, no?

Sure but this is something we have to always keep in mind then when making changes. Also we'd have to specify some canonical location where those symlinks would point to. By just unpacking the systemd-tests rpm to some writable directory we'd avoid having to figure all of that out.

Btw I need this myself now as well so doing some more work in #37046

knelsonmeister pushed a commit to knelsonmeister/systemd that referenced this pull request Apr 23, 2025
knelsonmeister pushed a commit to knelsonmeister/systemd that referenced this pull request Apr 23, 2025
bluca pushed a commit to bluca/systemd-fedora that referenced this pull request Jul 25, 2025
systemd/systemd#36954 will move all the mkosi
configuration in the systemd repository into a mkosi/ subdirectory. This
means we have to put mkosi.local.conf in that subdirectory as well, so check
if the mkosi/ directory exists and put mkosi.local.conf in there if it exists.

The mkosi/ directory will conflict with our checkout of mkosi so we move that
checkout one level up. Additionally, we can't use .. anymore as the package
directory as that only works when mkosi.local.conf is in the top level directory
of the repository so we use an absolute path instead.
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.

4 participants