Re: [gentoo-dev] new eclass: tmpfiles.eclass for handling tmpfiles.d files

[William Hubbs <williamh@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 4074 bytes --]

On Tue, Nov 15, 2016 at 09:07:20PM +0100, Michał Górny wrote:
> On Tue, 15 Nov 2016 13:42:00 -0600
> William Hubbs <williamh@gentoo.org> wrote:
> 
> > # Copyright 1999-2016 Gentoo Foundation
> > # Distributed under the terms of the GNU General Public License v2
> > # $Id$
> > 
> > # @ECLASS: tmpfiles.eclass
> > # @MAINTAINER:
> > # Gentoo systemd project <systemd@gentoo.org>
> > # William Hubbs <williamh@gentoo.org>
> > # @AUTHOR:
> > # Mike Gilbert <floppym@gentoo.org>
> > # William Hubbs <williamh@gentoo.org>
> > # @BLURB: Functions related to tmpfiles.d files
> > # @DESCRIPTION:
> > # Provides common functionality related to installation an processing
> 
> Typo: 'and'
> 
> > # of tmpfiles snippets.
> 
> Honestly, I would like to see here either a more complete description
> (with example!) on what purpose does this eclass serve and how it's
> supposed to be used.
> 
> It would probably make sense to explain that it can be used to
> *reliably* create temporary directories, i.e. without a need for
> an explicit fallback in ebuild. After all, that's the main goal of
> the whole effort.
 
 Sure, I'll add more info here.

> It may also be worth it to note that full support requires OpenRC or
> systemd. People not using either of the two will only get directories
> created in postinst, and can't use volatile filesystems.

The only thing another init system would have to do is run opentmpfiles
or systemd-tmpfiles at the appropriate point in bootup, so I can mention
that.

> 
> > if [[ -z $tmpfiles_eclass ]]; then
> 
> 1. Don't indent the whole eclass.
> 
> 2. Use capitals for global variables.
> 
> 3. Use ${} with braces.
> 
> > 	tmpfiles_eclass=1
> > 
> > 	case "${EAPI}" in
> > 		5|6) ;;
> > 		*) die "API is undefined for EAPI ${EAPI}" ;;
> > 	esac
> > 
> > 	DEPEND="virtual/tmpfiles"
> 
> You don't need build-time dependency on it. It's not used before pkg_*.
> 
> > 	RDEPEND="virtual/tmpfiles"
> 
> 1. I'm wondering if there's a case for making this optional
> (I'm pretty sure we have USE-conditional installs of tmpfiles, however
> I'm not sure if we care for 100% exact deps).
 
hmm, use-conditional installs of tmpfiles... should we look into fixing
those (wrt our practice of always installing small files), or should we
revisit that whole practice (+1000 for revisiting it).

RDEPEND's can't depend on use flags, so I'm not sure how we would make
this optional.

> 2. I think it would be reasonable to match virtual versions to
> 'versions' of tmpfiles.d support, i.e. to be able to e.g. >=230 when
> the file needs new feature that was introduced in systemd-230. But
> maybe it'd good enough to have additional dependency in ebuild then.
> 

I'm not sure about adding an additional dependency to the ebuild. This
could be handled by bumping the virtual if necessary.

> ...
> > 	# @FUNCTION: tmpfiles_create
> 
> Maybe tmpfiles_process?
> 
> > 	# @USAGE: tmpfiles_create
> 
> I think you actually want to pass filenames here.
> 
> > 	# @DESCRIPTION:
> > 	# Call a tmpfiles implementation to process newly installed tmpfiles.d
> > 	# snippets. Does nothing if no tmpfiles implementation is available.
> 
> I think it should error out when no implementation is available. We
> want to be able to reliably create temporary directories.
> 
> > 	tmpfiles_create() {
> > 		debug-print-function "${FUNCNAME}" "$@"
> > 
> > 		[[ ${EBUILD_PHASE} == postinst ]] || die "${FUNCNAME}: Only valid in pkg_postinst"
> > 		[[ ${#} -gt 0 ]] || die "${FUNCNAME}: Must specify at least one filename"
> > 		[[ ${ROOT} == / ]] || return 0
> > 
> > 		if type systemd-tmpfiles &> /dev/null; then
> > 			systemd-tmpfiles --create "$@"
> > 		elif type opentmptmpfiles &> /dev/null; then
> > 			opentmpfiles --create "$@"
> > 		fi
> > 	}
> > 
> > fi
> 
> I'm wondering if we could go for some cleanup in prerm. But that's
> probably a far-away future goal.
> 
> -- 
> Best regards,
> Michał Górny
> <http://dev.gentoo.org/~mgorny/>



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 163 bytes --]