Patchwork [v2] util-linux: Use PACKAGECONFIG to control pam and system config options

login
register
mail settings
Submitter Koen Kooi
Date July 25, 2013, 1:02 p.m.
Message ID <4F2BF532-D3A4-431E-8905-E4F33FBF80D5@dominion.thruhere.net>
Download mbox | patch
Permalink /patch/54485/
State New
Headers show

Comments

Koen Kooi - July 25, 2013, 1:02 p.m.
Op 25 jul. 2013, om 14:58 heeft Koen Kooi <koen@dominion.thruhere.net> het volgende geschreven:

> 
> Op 10 jul. 2013, om 18:26 heeft Enrico Scholz <enrico.scholz@sigma-chemnitz.de> het volgende geschreven:
> 
>> Saul Wold <sgw-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
>> 
>>> The PACKAGECONFIG will ensure consistent enabling and disabling of the pam and systemd related
>>> options for configure and the correct dependencies
>>> 
>>> v2: fixed PACKAGECONFIG line continuation grammar
>>>   added _class-target for PACKAGECONFIG to work on target only
>>> ...
>>> +PACKAGECONFIG_class-target ??= "${@base_contains('DISTRO_FEATURES', 'pam', 'pam', '', d)} \
>>> +                                ${@base_contains('DISTRO_FEATURES', 'systemd', 'systemd', '', d)} "
>> 
>> This does not work here. d.getVar('PACKAGECONFIG', True) evaluates to 'None'
>> in the anonymous python function in base.bbclass so that non-systemd options
>> are selected in systemd distributions and packaging fails in a sanity check.
>> 
>> Using the less weak '?=' operator makes thing work as expected.
> 
> And a different version of this patch got merged:
> 
> 	https://github.com/openembedded/oe-core/commit/7cde7c639c53724327d981cbc0db5e123607de1c
> 
> Which has the following bug:
> 
> PACKAGECONFIG_class-target ??= "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)} \
> [..]
> PACKAGECONFIG[pam] = "--enable-su --enable-runuser,--disable-su --disable-runuser, pam,"
> 
> It sets 'libpam' as PACKAGECONFIG option, but the option is actually named 'pam'. The patches posted to this list don't seem to have this bug.

Here's what I used with dylan + backports:

commit 2f318d0f9627e1872732db1ecafaed9caeb68ebf
Author: Koen Kooi <koen@dominion.thruhere.net>
Date:   Thu Jul 25 14:58:27 2013 +0200

    util-linux: fix PACKAGECONFIG options
    
    The ??= operator is too weak and it's setting a non-existent PACKAGECONFIG option ('libpam' instead of 'pam').
    
    Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
Saul Wold - July 25, 2013, 2:17 p.m.
On 07/25/2013 06:02 AM, Koen Kooi wrote:
>
>
>
> Op 25 jul. 2013, om 14:58 heeft Koen Kooi <koen@dominion.thruhere.net> het volgende geschreven:
>
>>
>> Op 10 jul. 2013, om 18:26 heeft Enrico Scholz <enrico.scholz@sigma-chemnitz.de> het volgende geschreven:
>>
>>> Saul Wold <sgw-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
>>>
>>>> The PACKAGECONFIG will ensure consistent enabling and disabling of the pam and systemd related
>>>> options for configure and the correct dependencies
>>>>
>>>> v2: fixed PACKAGECONFIG line continuation grammar
>>>>    added _class-target for PACKAGECONFIG to work on target only
>>>> ...
>>>> +PACKAGECONFIG_class-target ??= "${@base_contains('DISTRO_FEATURES', 'pam', 'pam', '', d)} \
>>>> +                                ${@base_contains('DISTRO_FEATURES', 'systemd', 'systemd', '', d)} "
>>>
>>> This does not work here. d.getVar('PACKAGECONFIG', True) evaluates to 'None'
>>> in the anonymous python function in base.bbclass so that non-systemd options
>>> are selected in systemd distributions and packaging fails in a sanity check.
>>>
>>> Using the less weak '?=' operator makes thing work as expected.
>>
>> And a different version of this patch got merged:
>>
>> 	https://github.com/openembedded/oe-core/commit/7cde7c639c53724327d981cbc0db5e123607de1c
>>
>> Which has the following bug:
>>
>> PACKAGECONFIG_class-target ??= "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)} \
>> [..]
>> PACKAGECONFIG[pam] = "--enable-su --enable-runuser,--disable-su --disable-runuser, pam,"
>>
>> It sets 'libpam' as PACKAGECONFIG option, but the option is actually named 'pam'. The patches posted to this list don't seem to have this bug.
>
> Here's what I used with dylan + backports:
>
> commit 2f318d0f9627e1872732db1ecafaed9caeb68ebf
> Author: Koen Kooi <koen@dominion.thruhere.net>
> Date:   Thu Jul 25 14:58:27 2013 +0200
>
>      util-linux: fix PACKAGECONFIG options
>
>      The ??= operator is too weak and it's setting a non-existent PACKAGECONFIG option ('libpam' instead of 'pam').
>
>      Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
>

Koen,

Good catch, can you send this as a proper patch to the OE-Core list also.

Thanks
	Sau!

> diff --git a/meta/recipes-core/util-linux/util-linux.inc b/meta/recipes-core/util-linux/util-linux.inc
> index d373cec..3d1198a 100644
> --- a/meta/recipes-core/util-linux/util-linux.inc
> +++ b/meta/recipes-core/util-linux/util-linux.inc
> @@ -47,9 +47,9 @@ EXTRA_OECONF = "--libdir=${base_libdir} --disable-use-tty-group \
>                   --enable-libuuid --enable-libblkid --enable-fsck --without-udev \
>                  usrsbin_execdir='${sbindir}' \
>   "
> -PACKAGECONFIG_class-target ??= "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)} \
> +PACKAGECONFIG_class-target ?= "${@base_contains('DISTRO_FEATURES', 'pam', 'pam', '', d)} \
>                                   ${@base_contains('DISTRO_FEATURES', 'systemd', 'systemd', '', d)} "
> -PACKAGECONFIG[pam] = "--enable-su --enable-runuser,--disable-su --disable-runuser, pam,"
> +PACKAGECONFIG[pam] = "--enable-su --enable-runuser,--disable-su --disable-runuser, libpam,"
>
>   # Respect the systemd feature for uuidd
>   PACKAGECONFIG[systemd] = "--enable-socket-activation --with-systemdsystemunitdir=${systemd_unitdir}/system/, --disable-socket-activation --without-systemdsystemunitdir"
>
>
>
>
Enrico Scholz - July 25, 2013, 3:58 p.m.
Koen Kooi <koen-QLwJDigV5abLmq1fohREcCpxlwaOVQ5f@public.gmane.org>
writes:

>     The ??= operator is too weak
> ...
> -PACKAGECONFIG_class-target ??= "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)} \
> +PACKAGECONFIG_class-target ?= "${@base_contains('DISTRO_FEATURES', 'pam', 'pam', '', d)} \
>                                  ${@base_contains('DISTRO_FEATURES', 'systemd', 'systemd', '', d)} "

good to see that I am not the only one hitting this problem.  But does
there exist some explanation for this behavior?  Lots of other packages
have

| PACKAGECONFIG ??=

and it seems to work there.  It might be related to the '_class-target'
override but that's a nasty, unpredictable bug causing silent breakage.

Something like

| localconfig = ""
| localconfig_class-target = "${@base_contains('DISTRO_FEATURES', 'pam', 'pam', '', d)}...
|
| PACKAGECONFIG ??= "${localconfig}"

and forbidding '??' in override-constructs might make things clearer.


Enrico

Patch

diff --git a/meta/recipes-core/util-linux/util-linux.inc b/meta/recipes-core/util-linux/util-linux.inc
index d373cec..3d1198a 100644
--- a/meta/recipes-core/util-linux/util-linux.inc
+++ b/meta/recipes-core/util-linux/util-linux.inc
@@ -47,9 +47,9 @@  EXTRA_OECONF = "--libdir=${base_libdir} --disable-use-tty-group \
                 --enable-libuuid --enable-libblkid --enable-fsck --without-udev \
                usrsbin_execdir='${sbindir}' \
 "
-PACKAGECONFIG_class-target ??= "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)} \
+PACKAGECONFIG_class-target ?= "${@base_contains('DISTRO_FEATURES', 'pam', 'pam', '', d)} \
                                 ${@base_contains('DISTRO_FEATURES', 'systemd', 'systemd', '', d)} "
-PACKAGECONFIG[pam] = "--enable-su --enable-runuser,--disable-su --disable-runuser, pam,"
+PACKAGECONFIG[pam] = "--enable-su --enable-runuser,--disable-su --disable-runuser, libpam,"
 
 # Respect the systemd feature for uuidd
 PACKAGECONFIG[systemd] = "--enable-socket-activation --with-systemdsystemunitdir=${systemd_unitdir}/system/, --disable-socket-activation --without-systemdsystemunitdir"