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

[Michał Górny]

On Tue, 15 Nov 2016 13:42:00 -0600
William Hubbs <williamh@g.o> 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@g.o>
> # William Hubbs <williamh@g.o>
> # @AUTHOR:
> # Mike Gilbert <floppym@g.o>
> # William Hubbs <williamh@g.o>
> # @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.

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.

> 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).

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.

>
...
> 	# @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/>