Re: [gentoo-dev] [RFC] systemd.eclass: Patch for new function systemd_get_udevdir()

[Michał Górny]

Ok, a few comments from me.

On Mon, 06 Aug 2012 13:00:08 +0300
Samuli Suominen <ssuominen@g.o> wrote:

> --- systemd.eclass	2012-08-06 12:49:20.532528219 +0300
> +++ /tmp/systemd.eclass	2012-08-06 12:57:28.333542735 +0300

First of all, I'm not really convinced systemd.eclass is a good place
for this.

The main reason for introducing a separate systemd.eclass was to have
a single place to control installing systemd unit files. The rule is
quite simple here: you install systemd unit files, you inherit
the eclass.

Most importantly, this allows us to easily find out which packages
install such files and perform global operations on them. For example,
if a particular user had systemd locations in INSTALL_MASK and changed
his mind, he can easily update his system by rebuilding all packages
inheriting systemd.eclass.

If all packages installing udev rules start inheriting it, the above
will no longer be correct. Also, the opposite way -- rebuilding
packages installing udev rules -- won't be that easy.

That's not my call but I believe that putting those functions in
separate udev.eclass could be the best course of action, for the reason
stated above -- we can easily find out what installs them, and rebuild
it all at once.

> @@ -25,6 +25,8 @@
>  # }
>  # @CODE
>
> +inherit toolchain-funcs
> +
>  case ${EAPI:-0} in
>  	0|1|2|3|4) ;;
>  	*) die "${ECLASS}.eclass API in EAPI ${EAPI} not yet
> established." @@ -34,6 +36,31 @@
>  DEPEND="!<sys-apps/systemd-29-r4
>  	!=sys-apps/systemd-37-r1"
>
> +# @FUNCTION: _systemd_get_udevdir
> +# @INTERNAL
> +# @DESCRIPTION:
> +# Get unprefixed udevdir.
> +_systemd_get_udevdir() {
> +	if $($(tc-getPKG_CONFIG) --exists udev); then
> +		echo -n "$($(tc-getPKG_CONFIG) --variable=udevdir
> udev)"

Secondly, I believe you shouldn't do such a thing *without* depending
on udev. Even though there is fallback here, there is another problem:
you are introducing a *potential inconsistency* between built packages,
depending on contents of udev.pc. Thus, I believe the package should
depend on the package providing it so that the dependency tree is
complete.

Also, I'm not so sure if this will work correctly for Prefix.

> +	else
> +		echo -n /lib/udev
> +	fi
> +}

And finally, I believe we shouldn't even be making the install location
variable. Right now, the Gentoo location for udev rules is /lib/udev,
and I believe William will agree with me on this. We hadn't decided on
moving them, and thus all udev and systemd versions in the tree *will*
support /lib/udev (I still need to commit patched systemd).

If we decide to move rules to /usr/lib/udev, I believe we will move
them all at once. And then all users will have to use a newer
(or patched) udev supporting the new location (or both). In order to
enforce that, the eclass would have to block old udev versions (like
the systemd.eclass blocks old systemd versions).

Making the install location dynamic is just asking for trouble.
Consider the following: user installs new udev, rebuilds package, then
downgrades udev. Package rules no longer work. That's what we would
like to avoid.

> +
> +# @FUNCTION: systemd_get_udevdir
> +# @DESCRIPTION:
> +# Output the path for the udev directory (not including ${D}).
> +# This function always succeeds, even if udev is not installed.
> +# Dependencies need to include sys-fs/udev or otherwise
> +# the fallback return value is /lib/udev.
> +systemd_get_udevdir() {
> +	has "${EAPI:-0}" 0 1 2 && ! use prefix && EPREFIX=
> +	debug-print-function ${FUNCNAME} "${@}"
> +
> +	echo -n "${EPREFIX}$(_systemd_get_udevdir)"
> +}
> +
>  # @FUNCTION: _systemd_get_unitdir
>  # @INTERNAL
>  # @DESCRIPTION:

As a final note, please note that this is mostly my opinion as systemd
maintainer. I believe William has a final word on udev itself.

-- 
Best regards,
Michał Górny