PR#1017: Tmpfiles.d: The _tmpfilesdir directory shouldn't be unowned - packaging-committee

packaging-committee

#1017 Tmpfiles.d: The _tmpfilesdir directory shouldn't be unowned

Closed a year ago by rishi. Opened 3 years ago by rishi.

rishi/packaging-committee wip/rishi/systemd-tmpfiles.d-require-systemd into master

Tmpfiles.d: The _tmpfilesdir directory shouldn't be unowned

Debarshi Ray • 3 years ago

d19ea9e

guidelines/modules/ROOT/pages/Tmpfiles.d.adoc

file modified

+5 -2

		`@@ -4,7 +4,7 @@`

		`== tmpfiles.d configuration`

		- Asking the tmpfiles.d mechanism to create directories for you just involves dropping a file into `+%{_tmpfilesdir}+`. You will need a build dependency on systemd-rpm-macros in order to make use of this macro.
		+ Asking the tmpfiles.d mechanism to create directories for you just involves dropping a file into `+%{_tmpfilesdir}+`. You will need a build dependency on systemd-rpm-macros in order to make use of this macro, and a runtime dependency on systemd to provide the `+%{_tmpfilesdir}+` directory itself.

		For example, if the package needs a few directories to be created in `+/run+` in order for it to run, the packager needs to create a file named `+%{name}.conf+` that is installed as `+%{_tmpfilesdir}/%{name}.conf+`. The file has one or more lines of the following format:

		`@@ -31,12 +31,15 @@`

		`== Example spec file`

		- In the spec file, the packager needs to install the tmpfiles.d conf file into the `+%{_tmpfilesdir}+` directory and also make sure the directory is included in the rpm.
		+ In the spec file, the packager needs to install the tmpfiles.d conf file into the `+%{_tmpfilesdir}+` directory and also make sure that the rpm depends on the owner of the directory.

		`....`
		`# For the _tmpfilesdir macro.`
		`BuildRequires: systemd-rpm-macros`

		`+ # To own _tmpfilesdir directory itself.`
		`+ Requires: systemd`
		`+`
		`# tmpfiles.d configuration for the /run directory`
		`Source1: %{name}-tmpfiles.conf`
		`[...]`

rishi commented 3 years ago

If a package is dropping a file into _tmpfilesdir, then installing the
package shouldn't lead to an unowned _tmpfilesdir directory. The
package should either depend on something that already owns that
directory (in this case systemd), or own the directory itself (in case
it wants to limit its dependency chain).

In fact, the text before the example spec file says:
"packager needs to ... make sure the directory is included in the
rpm."

That's the second option above.

On a stock Fedora 32 Silverblue system and a stock Fedora 31
Workstation, _tmpfilesdir is only owned by the systemd package.
Therefore, it might be better to have the documentation lean towards
the first alternative of depending on systemd. Ultimately, in reality,
systemd isn't such an onerous addition because all variants of Fedora
use it as part of the base OS, and by using the tmpfiles.d mechanism
the package is implicitly indicating a desire to use systemd.

rebased onto d19ea9e

3 years ago

ngompa commented 3 years ago

We could just add it to filesystem, no?

carlwgeorge commented 3 years ago

I agree about adding it to filesystem. For the container use case, it would be unfortunate for if systemd was pulled in as a dependency just because a package includes a potentially optional configuration file.

In fact, we should add other similar systemd directories to filesystem for the same reason. Checking a fedora:32 container image, I see the following candidates:

[carl@tassadar:~]$ podman run -it --rm fedora:32
[root@6b9524f53510 /]# rpm -qf /usr/lib/{sysctl.d,systemd{,/user,/system},tmpfiles.d,udev{,/rules.d}}
file /usr/lib/sysctl.d is not owned by any package
file /usr/lib/systemd is not owned by any package
file /usr/lib/systemd/user is not owned by any package
file /usr/lib/systemd/system is not owned by any package
file /usr/lib/tmpfiles.d is not owned by any package
file /usr/lib/udev is not owned by any package
file /usr/lib/udev/rules.d is not owned by any package

rishi commented 3 years ago

We could just add it to filesystem, no?

Or that, yes.

rishi commented 2 years ago

Oops! Looks like I dropped the ball on this.

I agree that adding %{_tmpfilesdir} to filesystem makes sense to avoid making the dependency chains too rigid, and I will look into that.

However, I realized that there might be cases where a package really does have a hard dependency on its tmpfiles.d snippet. eg., toolbox which isn't meant to work inside containers, and the binary won't even get linked at run-time if the tmpfiles.d snippet was ignored.

Should the guidelines should say something about such cases, because I am now wondering if I should be using %{?systemd_requires} or Requires: systemd? Maybe I shouldn't even be adding the dependency, because all variants of Fedora use systemd as part of the base OS?

rishi commented 2 years ago

Pull request against filesystem:
https://pagure.io/filesystem/pull-request/6

rishi commented 2 years ago

Also filed a bug against filesystem:
https://bugzilla.redhat.com/show_bug.cgi?id=2068460

mosvald commented 2 years ago

Thanks for the patch!

tibbs commented 2 years ago

Since the filesystem patch has been merged, what should become of this PR? Obviously the dependency won't be necessary in the usual case.

Regarding the previous question about some special packages like toolbox, generally we don't go out of the way to cover every conceivable situation. Is the situation described sufficiently common that it will cause issues for more than a couple of packages? Certainly things which really need systemd should depend on systemd; that's not any different than any other dependency.

But that isn't to say that systemd isn't special, and if the guidelines need clarification then we should probably discuss what needs to be done. But that should happen in a separate ticket

rishi commented a year ago

Since the filesystem patch has been merged, what should become of this PR?

It can be closed.

My remaining question is whether I should bother adding a runtime dependency on systemd to the toolbox RPM, and if I do add one should it be %{?systemd_requires} or Requires: systemd?