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

[Samuli Suominen <ssuominen@gentoo.org>]

On 08/06/2012 01:20 PM, Michał Górny wrote:
> Ok, a few comments from me.
>
> On Mon, 06 Aug 2012 13:00:08 +0300
> Samuli Suominen <ssuominen@gentoo.org> 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.


OK, we can make it udev.eclass and then we can add the virtual/pkgconfig 
and sys-fs/udev dependency of it.


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

OK, I can agree with udev.eclass as I stated before already.

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

I'm sure that is easily checked and we will get feedback quickly.

>
>> +	else
>> +		echo -n /lib/udev
>> +	fi
>> +}
>
> And finally, I believe we shouldn't even be making the install location
> variable.

It's the upstream way to determine the path and is used treewide by 
upstream configure scripts etc. for various packages already.

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

The udevdir is whatever udev.pc determines it to be, and currently in 
~arch it's set to /usr/lib/udev
Therefore the Gentoo udevdir is also /usr/lib/udev for ~arch

As in, packages in tree are using it already, independent of getting 
this helper function to tree or not.

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

Downgrading is never a good idea when you don't know what you are doing.

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

Summarizing:

- Will call this udev.eclass and add the dependencies for udev and 
pkg-config

- Gentoo's udevdir is already set to /usr/lib/udev by 
 >=sys-fs/udev-187-r1 in Portage, and is widely queried by upstream 
configure scripts
We should leave it like it is now, and get this helper function ASAP in 
tree to get things consistent again

- Samuli