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

build: Call 'systemd-tmpfiles --create' when installing #965

Merged

Conversation

debarshiray
Copy link
Member

It's only necessary to call 'systemd-tmpfiles --create' when building
and installing from source, and not when using a downstream package,
because:

  • When 'meson install' is called as part of building the package,
    that's not when the temporary files need to be created. They need
    to be created when the package is later downloaded and installed
    by the user.

  • Downstream tools can sometimes handle it automatically. eg., on
    Fedora, the systemd RPM installs a trigger that tells RPM to call
    'systemd-tmpfiles --create' automatically when a tmpfiles.d snippet
    is installed.

#955

@softwarefactory-project-zuul
Copy link

Build failed.

@debarshiray
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@debarshiray
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@HarryMichal
Copy link
Member

Did you try running this locally? On my machine (when ran inside of a toolbx) this often results in:

Running custom install script '/usr/bin/systemd-tmpfiles --create'
--- stdout ---

--- stderr ---
fchownat() of /var/lib/systemd/coredump failed: Read-only file system
fchownat() of /tmp failed: Operation not permitted
fchownat() of /run/media failed: Operation not permitted
Setting access ACL "u::rwx,g::r-x,g:adm:r-x,g:wheel:r-x,g:4294967295:r-x,g:4294967295:r-x,m::r-x,o::r-x" on /var/log/journal failed: Read-only file system
Failed to re-open '/var/log/journal': Operation not permitted
fchownat() of /var/log/journal failed: Read-only file system
Setting access ACL "u::rwx,g::r-x,g:adm:r-x,g:wheel:r-x,g:4294967295:r-x,g:4294967295:r-x,m::r-x,o::r-x" on /var/log/journal/64ea8f1e4b534013843813476c178ab7 failed: Read-only file system
Failed to re-open '/var/log/journal/64ea8f1e4b534013843813476c178ab7': Operation not permitted
fchownat() of /var/log/journal/64ea8f1e4b534013843813476c178ab7 failed: Read-only file system
fchownat() of /dev/snd/seq failed: Operation not permitted
fchownat() of /dev/snd/timer failed: Operation not permitted
fchownat() of /dev/loop-control failed: Operation not permitted
fchownat() of /dev/kvm failed: Operation not permitted
fchownat() of /dev/vhost-net failed: Operation not permitted
fchownat() of /dev/vhost-vsock failed: Operation not permitted
Setting access ACL "u::rw-,g::r-x,g:adm:r--,g:wheel:r--,g:4294967295:r--,g:4294967295:r--,m::r--,o::---" on /var/log/journal/64ea8f1e4b534013843813476c178ab7/system.journal failed: Read-only file system
fchownat() of /var/log/journal/64ea8f1e4b534013843813476c178ab7/system.journal failed: Read-only file system

FAILED: install script '/usr/bin/systemd-tmpfiles --create' exit code 73, stopped
FAILED: meson-install
/usr/bin/meson install --no-rebuild
ninja: build stopped: subcommand failed.

The goal of this step is to create /run/host and not to create other tmpfiles. A way to solve this is to create only this file. I did it with this snipper:

meson.add_install_script(
  systemd_tmpfiles,
  '--create', '--prefix=/run/host', tmpfiles_toolbox_conf,
  skip_if_destdir: true,
)

@HarryMichal HarryMichal added 2. Build System Issue is related to the build system 3. Enhancement Improvement to an existing feature labels Dec 18, 2021
@debarshiray
Copy link
Member Author

Did you try running this locally? On my machine (when ran inside of a toolbx)
this often results in:

I only tried the build with DESTDIR locally on a Fedora Workstation host. I didn't try it without DESTDIR inside a Toolbox container. Thanks for testing it out.

By the way, this makes me think that we need a test that tries to build Toolbox from source inside a Toolbox container.

@HarryMichal HarryMichal added this to the Release 0.1.0 milestone Jan 7, 2022
@HarryMichal
Copy link
Member

By the way, this makes me think that we need a test that tries to build Toolbox from source inside a Toolbox container.

Do we? I'm wondering why is that required. What does it test? That Toolbx is buildable? That a software can be built?

If we were to add this test, it should not exist in the system tests but in a different group of tests.

@HarryMichal HarryMichal linked an issue Jan 8, 2022 that may be closed by this pull request
@debarshiray
Copy link
Member Author

By the way, this makes me think that we need a test that tries to build
Toolbox from source inside a Toolbox container.

Do we? I'm wondering why is that required.

It's required because our test suite clearly failed to catch that this pull request breaks the build inside a Toolbx container. :)

What does it test?

It should test that Toolbx can be built from source inside a Toolbx container. Toolbx is necessary for hacking on Toolbx, so we shouldn't be pushing changes that break the build.

@HarryMichal
Copy link
Member

It's required because our test suite clearly failed to catch that this pull request breaks the build inside a Toolbx container. :)

Technically speaking Toolbx can be built even without this fix being applied. It can not be run though due to missing /run/host symlink. I'm playing with words a bit but the problem does not lie in the build but the executable environment and that one is usually set up during installation (also this PR fixes the problem on installation). The fact we can run Toolbx without installing it is coincidental convenience.

It should test that Toolbx can be built from source inside a Toolbx container.

Despite what I've written above, I agree the test can be written but I still believe such a test should exist outside of the current system test test suite.

Toolbx is necessary for hacking on Toolbx

This generally applies only to Fedora Silverblue.

@debarshiray
Copy link
Member Author

The goal of this step is to create /run/host and not to create other
tmpfiles.

In the past, toolbox create wouldn't work without /run/media. All those paths in data/tmpfiles.d/toolbox.conf are important for one reason or another.

A way to solve this is to create only this file. I did it with this
snipper:

meson.add_install_script(
  systemd_tmpfiles,
  '--create', '--prefix=/run/host', tmpfiles_toolbox_conf,
  skip_if_destdir: true,
)

Does this snippet actually work? Or is it meant to be pseudo-code? :)

The build failed inside a Toolbx container because systemd doesn't quite work inside the container. I didn't check exactly which part of systemd-tmpfiles runs into problems, but the files we want to create are meant to live on the host, so it doesn't really matter.

@debarshiray
Copy link
Member Author

It's required because our test suite clearly failed to catch that this
pull request breaks the build inside a Toolbx container. :)

Technically speaking Toolbx can be built even without this fix being
applied. It can not be run though due to missing /run/host symlink.
I'm playing with words a bit but the problem does not lie in the build
but the executable environment and that one is usually set up during
installation (also this PR fixes the problem on installation).

Eh? :)

This pull request is trying to fix #955 which is about the built binary toolbox failing to execute. Generally speaking, there's no point in building a binary that doesn't work. This is also why when projects have tests, failing tests are equated to broken builds.

Since we are trying to fix #955, we might as well ensure that we do so without breaking the build. :)

It should test that Toolbx can be built from source inside a Toolbx container.

Despite what I've written above, I agree the test can be written but I still
believe such a test should exist outside of the current system test test suite.

It doesn't matter so much exactly what we call the test, as long as it tests the right thing and it is called from ninja test. :)

Toolbx is necessary for hacking on Toolbx

This generally applies only to Fedora Silverblue.

... and Fedora Silverblue is the primary reason for Toolbx to exist.

It will be a major problem if it becomes impossible to hack on Toolbx on a Silverblue system.

@HarryMichal
Copy link
Member

In the past, toolbox create wouldn't work without /run/media. All those paths in data/tmpfiles.d/toolbox.conf are important for one reason or another.

This is true but /run/media is needed in a later part of the execution process of Toolbx (exactly when creating + starting Toolbx containers). /run/host is needed to run the tool at all.

The reason for the suggestion is not to have a failing statement as part of Toolbx build. That way we'll never get a successful build.

Does this snippet actually work? Or is it meant to be pseudo-code? :)

Yes, it works :).

Comparison of the systemd-tmpfiles invocation inside of a Toolbx container:

; systemd-tmpfiles --create
Failed to open directory 'cryptsetup': Permission denied
Failed to open directory 'portables': Permission denied
Failed to open directory 'sudo': Permission denied
Failed to open directory 'ts': Permission denied
Failed to open directory 'svnserve': Permission denied
fchownat() of /var/lib/systemd/coredump failed: Read-only file system
Failed to open directory 'private': Permission denied
Failed to open directory 'private': Permission denied
Failed to open directory 'private': Permission denied
fchownat() of /tmp failed: Operation not permitted
Setting access ACL "u::rwx,g::r-x,g:adm:r-x,g:wheel:r-x,g:4294967295:r-x,g:4294967295:r-x,m::r-x,o::r-x" on /var/log/journal failed: Read-only file system
Failed to re-open '/var/log/journal': Operation not permitted
fchownat() of /var/log/journal failed: Read-only file system
Setting access ACL "u::rwx,g::r-x,g:adm:r-x,g:wheel:r-x,g:4294967295:r-x,g:4294967295:r-x,m::r-x,o::r-x" on /var/log/journal/64ea8f1e4b534013843813476c178ab7 failed: Read-only file system
Failed to re-open '/var/log/journal/64ea8f1e4b534013843813476c178ab7': Operation not permitted
fchownat() of /var/log/journal/64ea8f1e4b534013843813476c178ab7 failed: Read-only file system
fchownat() of /dev/snd/seq failed: Operation not permitted
fchownat() of /dev/snd/timer failed: Operation not permitted
fchownat() of /dev/loop-control failed: Operation not permitted
fchownat() of /dev/kvm failed: Operation not permitted
fchownat() of /dev/vhost-net failed: Operation not permitted
fchownat() of /dev/vhost-vsock failed: Operation not permitted
Setting access ACL "u::rw-,g::r-x,g:adm:r--,g:wheel:r--,g:4294967295:r--,g:4294967295:r--,m::r--,o::---" on /var/log/journal/64ea8f1e4b534013843813476c178ab7/system.journal failed: Read-only file system
fchownat() of /var/log/journal/64ea8f1e4b534013843813476c178ab7/system.journal failed: Read-only file system
Setting default ACL "u::rwx,g::rwx,g:tss:rwx,m::rwx,o::r-x" on /var/lib/tpm2-tss/system/keystore failed: Operation not permitted
Setting default ACL "u::rwx,g::rwx,g:tss:rwx,m::rwx,o::r-x" on /run/tpm2-tss/eventlog failed: Operation not permitted

; echo $status
73

; systemd-tmpfiles --create --prefix=/run/host /var/home/marty/Documents/projects/toolbox/data/tmpfiles.d/toolbox.conf

; echo $status
0

The build failed inside a Toolbx container because systemd doesn't quite work inside the container.

systemd-tmpfiles should work despite systemd being the active init, right?

I didn't check exactly which part of systemd-tmpfiles runs into problems, but the files we want to create are meant to live on the host, so it doesn't really matter.

/run/host needs to be everywhere. Toolbx takes care of creating it in its containers.

@debarshiray debarshiray force-pushed the wip/rishi/call-systemd-tmpfiles branch from 1cc8083 to 73a33e2 Compare January 10, 2022 19:35
@debarshiray
Copy link
Member Author

debarshiray commented Jan 10, 2022

In the past, toolbox create
wouldn't work without /run/media.
All those paths in data/tmpfiles.d/toolbox.conf are important for one
reason or another.

This is true but /run/media is needed in a later part of the execution process
of Toolbx (exactly when creating + starting Toolbx containers). /run/host is
needed to run the tool at all.

Ideally, we don't want to differentiate between breakages. We want our build to generate a fully working binary, as far as reasonably and practically possible.

Also, replicating a different subset of the files in data/tmpfiles.d/toolbox.conf in different parts of the build will be a maintenance nightmare. We should try to keep things consolidated in one place as far as possible.

The reason for the suggestion is not to have a failing statement as part of
Toolbx build. That way we'll never get a successful build.

Umm... I didn't understand.

Does this snippet actually work? Or is it meant to be pseudo-code? :)

Yes, it works :).

I asked because this snippet didn't look valid:

meson.add_install_script(
  systemd_tmpfiles,
  '--create', '--prefix=/run/host', tmpfiles_toolbox_conf,
  skip_if_destdir: true,
)

... and the Meson logs didn't exactly match what I was seeing. eg.,

Running custom install script '/usr/bin/systemd-tmpfiles --create'

I suppose you had defined systemd_tmpfiles elsewhere.

The build failed inside a Toolbx container because systemd doesn't quite
work inside the container.

systemd-tmpfiles should work despite systemd being the active init, right?

systemd-tmpfiles is part of systemd. We don't promise that all systemd features will work inside Toolbx containers, so it's not surprising that systemd-tmpfiles doesn't work either. In theory, we could probably make it work, but it might also not be accepted by systemd upstream.

I didn't check exactly which part of systemd-tmpfiles runs into problems, but
the files we want to create are meant to live on the host, so it doesn't really matter.

/run/host needs to be everywhere. Toolbx takes care of creating it in its containers.

We are talking about systemd-tmpfiles --create here. Those files are host-specific.

@softwarefactory-project-zuul
Copy link

Build succeeded.

@debarshiray debarshiray force-pushed the wip/rishi/call-systemd-tmpfiles branch from 73a33e2 to cc26c42 Compare January 10, 2022 20:28
@softwarefactory-project-zuul
Copy link

Build failed.

It's only necessary to call 'systemd-tmpfiles --create' when building
and installing from source on the host operating system.

It's not needed when using a pre-built binary downstream package,
because:

  * When 'meson install' is called as part of building the package,
    that's not when the temporary files need to be created. They need
    to be created when the binary package is later downloaded and
    installed by the user.

  * Downstream tools can sometimes handle it automatically. eg., on
    Fedora, the systemd RPM installs a trigger that tells RPM to call
    'systemd-tmpfiles --create' automatically when a tmpfiles.d snippet
    is installed.

It's also not needed when installing inside a toolbox container because
the files that 'systemd-tmpfiles --create' is supposed to create are
meant to be on the host.

Downstream distributors set the DESTDIR environment variable when
building their packages. Therefore, it's used to detect when a
downstream package is being built.

Unfortunately, environment variables are messy and, generally, Meson
doesn't support accessing them inside its scripts [1]. Therefore, this
adds a spurious build-time dependency on systemd for downstream
distributors. However, that's probably not a big problem because all
supported downstream operating systems are already expected to use
systemd for the tmpfiles.d(5) snippets to work.

[1] mesonbuild/meson#9

containers#955
@HarryMichal
Copy link
Member

Ideally, we don't want to differentiate between breakages. We want our build to generate a fully working binary, as far as reasonably and practically possible.

Fair enough.

Also, replicating a different subset of the files in data/tmpfiles.d/toolbox.conf in different parts of the build will be a maintenance nightmare. We should try to keep things consolidated in one place as far as possible.

Fair enough.

Umm... I didn't understand.

Reason why I'm suggesting to run systemd-tmpfiles on a subset instead of running it with default settings.

I asked because this snippet didn't look valid:

meson.add_install_script(
  systemd_tmpfiles,
  '--create', '--prefix=/run/host', tmpfiles_toolbox_conf,
  skip_if_destdir: true,
)

I suppose you had defined systemd_tmpfiles elsewhere.

Of course systemd_tmpfiles has to be declared first. This snippet should do the trick:

systemd_tmpfiles = find_program('systemd-tmpfiles')

And tmpfiles_toolbox_conf has to also exist, first. Ideally this call is done in data/tmpfiles.d/meson.build where the variable holds a list of files with the config file present inside.

systemd-tmpfiles is part of systemd. We don't promise that all systemd features will work inside Toolbx containers, so it's not surprising that systemd-tmpfiles doesn't work either. In theory, we could probably make it work, but it might also not be accepted by systemd upstream.

I don't understand what do you mean by "systemd-tmpfiles doesn't work". It works but in the container exist files/directories that the tool can not have privileges to work with because they've been bind-mounted from the host. There's also a difference between running the tool in system or user mode (--user option).

/run/host needs to be everywhere. Toolbx takes care of creating it in its containers.

We are talking about systemd-tmpfiles --create here. Those files are host-specific.

Of course. Still, the point about /run/host stands.

@debarshiray
Copy link
Member Author

Umm... I didn't understand.

Reason why I'm suggesting to run systemd-tmpfiles on a subset instead
of running it with default settings.

I still don't understand. Why would we run a subset of the tmpfiles.d snippet?

It already works without a subset. We detect that we are inside a container and skip the systemd-tmpfiles --create call.

systemd-tmpfiles is part of systemd. We don't promise that all systemd
features will work inside Toolbx containers, so it's not surprising that
systemd-tmpfiles doesn't work either. In theory, we could probably make it
work, but it might also not be accepted by systemd upstream.

I don't understand what do you mean by "systemd-tmpfiles doesn't work".
It works but in the container exist files/directories that the tool can not have
privileges to work with because they've been bind-mounted from the host.

Ah, you are right that it's failing because the locations are owned by nobody:nobody.

Regardless, why systemd-tmpfiles --create is failing inside containers doesn't matter that much, because we don't need to run it inside containers anyway.

@debarshiray debarshiray force-pushed the wip/rishi/call-systemd-tmpfiles branch from cc26c42 to be2ba6d Compare January 10, 2022 21:31
@softwarefactory-project-zuul
Copy link

Build succeeded.

@debarshiray debarshiray merged commit be2ba6d into containers:main Jan 10, 2022
@debarshiray debarshiray deleted the wip/rishi/call-systemd-tmpfiles branch January 10, 2022 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. Build System Issue is related to the build system 3. Enhancement Improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'systemd-tmpfiles --create' is not called when installing
3 participants