packages shouldn't be installing things into /var or using keepdir. tmpfiles.d handles this properly.
It looks like we already install a tmpfiles snippet. Probably just need to remove the keepdir.
A couple of issues I want to look into: The keepdir sets the mode to 0750. The tmpfiles snippet sets the mode to 0755. I think we should probably change the tmpfiles snippet to 0750. The tmpfiles snippet is currently only installed/processed when USE=server. We should probably change this to be done unconditionally, since dhclient will also write to /var/lib/dhcp.
(In reply to SpanKY from comment #0) > packages shouldn't be installing things into /var or using keepdir. > tmpfiles.d handles this properly. Why? net-misc/dhcp is not installing random regular files, it's creating a directory, and /var is not volatile; once a directory is created there, it stays there. Who cares how it is initially created? And systemd-tmpfiles runs on every boot, how is trying yo create /var/lib/dhcp over and over again a good idea?
(In reply to Guillermo D. H. from comment #3) > And systemd-tmpfiles runs on every boot, how is trying yo create > /var/lib/dhcp over and over again a good idea? Nothing is going to force it to be deleted on your system.
(In reply to Guillermo D. H. from comment #3) /var is recommended to be maintained across reboots, but it stops at that -- a recommendation, not a requirement. further, daemons & init scripts already do cursory checks on their state to make sure they're setup, and either fix issues automatically, or abort. tmpfiles.d is simply another mechanism to cover that. if a path in /var gets corrupted, accidentally or maliciously, requiring people to re-emerge all packages that install into /var is unreasonable and needlessly expensive. conversely, tmpfiles.d entries are extremely cheap and make the system more robust & self healing, and don't require .keep files cluttering up the system. so there really is no argument for defining the initial state in the ebuild's src_install, and there are plenty of good reasons for doing it in tmpfiles.d, with no downsides. maybe you're confused about what tmpfiles.d do in practice. it is easy to add an entry that says "make sure dir X exists, has permission Y, is owned by Z, and if the dir already exists in that state, do nothing". and as Mike already pointed out, dhcp is already installing a tmpfiles.d entry that already does exactly this.
(In reply to Sam James from comment #4) > Nothing is going to force it to be deleted on your system. So why check on every boot?
(In reply to Guillermo D. H. from comment #6) > (In reply to Sam James from comment #4) > > Nothing is going to force it to be deleted on your system. > > So why check on every boot? See what vapier said. Nothing _forces_ it to be deleted on your system, but tmpfiles means it handles its absence gracefully. Not everyone has the same setup as you.
(In reply to SpanKY from comment #5) > (In reply to Guillermo D. H. from comment #3) > > /var is recommended to be maintained across reboots, but it stops at that -- > a recommendation, not a requirement. True, but Gentoo's setup makes it assuredly nonvolatile. It is not /run or /tmp. > further, daemons & init scripts > already do cursory checks on their state to make sure they're setup, and > either fix issues automatically, or abort. tmpfiles.d is simply another > mechanism to cover that. Useful only if daemons & init scripts don't create the files themselves, and useless double work otherwise. > if a path in /var gets corrupted, accidentally or > maliciously, [...] The same could be said about /usr, yet it is not covered by tmpfiles.d entries. > [...] and don't require .keep files cluttering up the system. sys-apps/baselayout does exactly this, doesn't it? > so there really is no argument for defining the initial state in the > ebuild's src_install, and there are plenty of good reasons for doing it in > tmpfiles.d, with no downsides. "Plenty"? Does a check or failed mkdir() call not consume CPU cycles? Does doing it on every boot consume less CPU cycles than doing it only once? Doesn't this grow O(N) with every useless tmpfiles.d entry? > maybe you're confused about what tmpfiles.d do in practice. Creating files that don't exist at boot time, that someone else needs but refuses to create? Isn't that why it is invoked by the service manager instead of just called, e.g. in ebuilds' pkg_postinst() phase? > and as Mike already pointed out, dhcp is already installing a tmpfiles.d > entry that already does exactly this. Don't ebuilds have the final say on what gets merged?
(In reply to Guillermo D. H. from comment #8) > (In reply to SpanKY from comment #5) > > (In reply to Guillermo D. H. from comment #3) > > > > /var is recommended to be maintained across reboots, but it stops at that -- > > a recommendation, not a requirement. > > True, but Gentoo's setup makes it assuredly nonvolatile. It is not /run or > /tmp. > People build various things on top of Gentoo with their own setups. I don't think this is a meaningful argument to pursue. > > further, daemons & init scripts > > already do cursory checks on their state to make sure they're setup, and > > either fix issues automatically, or abort. tmpfiles.d is simply another > > mechanism to cover that. > > Useful only if daemons & init scripts don't create the files themselves, and > useless double work otherwise. Daemons don't tend to reliably do this, if at all. It's also often the case that e.g. OpenRC initscripts lack the logic to do this properly when it's better done in the daemon or by tmpfiles. And then upstreams tend to prefer tmpfiles because they don't want to get various bugs filed for CVEs involving race conditions/TOCTOUs.
(In reply to Guillermo D. H. from comment #8) > > [...] and don't require .keep files cluttering up the system. > > sys-apps/baselayout does exactly this, doesn't it? I hope to change that soon. See bug 888807 for example.
(In reply to Guillermo D. H. from comment #8) > Useful only if daemons & init scripts don't create the files themselves, and > useless double work otherwise. if they did, we wouldn't need the keepdir nor the tmpfiles.d in the first place. that isn't the argument here (and, afaik, dhcp doesn't initialize the dir itself). > The same could be said about /usr, yet it is not covered by tmpfiles.d > entries. the rootfs and/or /usr can be mounted read-only in a reasonable system. /var cannot be mounted read-only in a reasonable system. > > so there really is no argument for defining the initial state in the > > ebuild's src_install, and there are plenty of good reasons for doing it in > > tmpfiles.d, with no downsides. > > "Plenty"? Does a check or failed mkdir() call not consume CPU cycles? Does > doing it on every boot consume less CPU cycles than doing it only once? > Doesn't this grow O(N) with every useless tmpfiles.d entry? it'd actually be a single stat() call. considering how much shell code still exists in the init system, trying to optimize away a single stat() call for a fs path that is going to be loaded/hot most of the time anyways doesn't seem useful. but even if we managed to cut away the unrelated fat, you know what ? i'm perfectly happy to eat O(N) stat() entries in tmpfiles.d if it means i can reboot my system and worry less about it getting wedged at boot, and be a bit more robust. the value of N is basically guaranteed to be well below 1000 (and prob even fewer than 100) on pretty much every system out there. > > maybe you're confused about what tmpfiles.d do in practice. > > Creating files that don't exist at boot time, that someone else needs but > refuses to create? it does more than create files. it can assert & fix state on files & dirs, e.g. permission bits, ownership settings, SELinux labels, extended attributes (xattrs), POSIX ACLs, and more. > Isn't that why it is invoked by the service manager > instead of just called, e.g. in ebuilds' pkg_postinst() phase? not sure what you mean here. if you're arguing that daemons themselves should have this logic, i don't disagree. although i'll point out that many upstreams opt to use a tmpfiles.d over writing arbitrary C/C++/Rust code because machine-parseable metadata files like tmpfiles.d are much easier to review/understand/assert/change, and tend to have fewer bugs.
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d71420bcf88260155b49c4ff294003189e5cba60 commit d71420bcf88260155b49c4ff294003189e5cba60 Author: Mike Gilbert <floppym@gentoo.org> AuthorDate: 2023-01-20 03:44:24 +0000 Commit: Mike Gilbert <floppym@gentoo.org> CommitDate: 2023-01-20 03:44:24 +0000 net-misc/dhcp: tweak /var/lib/dhcp creation Drop the keepdir from src_install. Update the modes in the tmpfiles.d file, and install it unconditionally. Closes: https://bugs.gentoo.org/891347 Signed-off-by: Mike Gilbert <floppym@gentoo.org> net-misc/dhcp/dhcp-4.4.3_p1-r1.ebuild | 293 ++++++++++++++++++++++++++++++++++ net-misc/dhcp/files/dhcp.tmpfiles | 2 + 2 files changed, 295 insertions(+)
