Re: [PATCH 1/3] bitbake.conf: make libc over-ride lower priority than _forcevariable

From: Andre McCurdy <armccurdy@gmail.com>
To: Khem Raj <raj.khem@gmail.com>
Cc: OE Core mailing list <openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH 1/3] bitbake.conf: make libc over-ride lower priority than _forcevariable
Date: Thu, 10 May 2018 15:33:19 -0700	[thread overview]
Message-ID: <CAJ86T=VJ6OL+EABTwV9QJumdYu_CvxHOnOGKYuLonpXUyy-dig@mail.gmail.com> (raw)
In-Reply-To: <874d828c-9387-3a1c-3a4c-381da0a0e270@gmail.com>

On Wed, May 9, 2018 at 10:41 PM, Khem Raj <raj.khem@gmail.com> wrote:
> On 5/9/18 8:14 PM, Andre McCurdy wrote:
>>
>> Fix long standing quirk, _forcevariable is documented as being the
>> highest priority over-ride.
>>
>> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
>> ---
>>   meta/conf/bitbake.conf                        | 3 ++-
>>   meta/conf/distro/include/tclibc-baremetal.inc | 2 --
>>   meta/conf/distro/include/tclibc-glibc.inc     | 2 --
>>   meta/conf/distro/include/tclibc-musl.inc      | 2 --
>>   4 files changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
>> index a21b728..a772cab 100644
>> --- a/meta/conf/bitbake.conf
>> +++ b/meta/conf/bitbake.conf
>> @@ -735,7 +735,8 @@ DISTRO_NAME ??= "OpenEmbedded"
>>   #
>>   # This works for  functions as well, they are really just environment
>> variables.
>>   # Default OVERRIDES to make compilation fail fast in case of build
>> system misconfiguration.
>> -OVERRIDES =
>> "${TARGET_OS}:${TRANSLATED_TARGET_ARCH}:build-${BUILD_OS}:pn-${PN}:${MACHINEOVERRIDES}:${DISTROOVERRIDES}:${CLASSOVERRIDE}:forcevariable"
>> +OVERRIDES =
>> "${TARGET_OS}:${TRANSLATED_TARGET_ARCH}:build-${BUILD_OS}:pn-${PN}:${MACHINEOVERRIDES}:${DISTROOVERRIDES}:${CLASSOVERRIDE}${LIBCOVERRIDE}:forcevariable"
>> +LIBCOVERRIDE ?= ""
>
>
> Overall this looks an improvement
>
>>   CLASSOVERRIDE ?= "class-target"
>>   DISTROOVERRIDES ?= "${@d.getVar('DISTRO') or ''}"
>>   MACHINEOVERRIDES ?= "${MACHINE}"
>> diff --git a/meta/conf/distro/include/tclibc-baremetal.inc
>> b/meta/conf/distro/include/tclibc-baremetal.inc
>> index 8d570f0..1bf44c8 100644
>> --- a/meta/conf/distro/include/tclibc-baremetal.inc
>> +++ b/meta/conf/distro/include/tclibc-baremetal.inc
>> @@ -4,9 +4,7 @@
>>     LIBCEXTENSION = ""
>>   -# Add baremetal libc overrides to the overrides.
>>   LIBCOVERRIDE = ":libc-baremetal"
>> -OVERRIDES .= "${LIBCOVERRIDE}"
>>     ASSUME_PROVIDED += "virtual/libc virtual/libiconv virtual/crypt"
>>   diff --git a/meta/conf/distro/include/tclibc-glibc.inc
>> b/meta/conf/distro/include/tclibc-glibc.inc
>> index ad8000f..8d10764 100644
>> --- a/meta/conf/distro/include/tclibc-glibc.inc
>> +++ b/meta/conf/distro/include/tclibc-glibc.inc
>> @@ -4,9 +4,7 @@
>>     LIBCEXTENSION = "${@['', '-gnu'][(d.getVar('ABIEXTENSION') or '') !=
>> '']}"
>>   -# Add glibc overrides to the overrides for glibc.
>>   LIBCOVERRIDE = ":libc-glibc"
>> -OVERRIDES .= "${LIBCOVERRIDE}"
>
>
> having .= appear was letting user know how it was being lined up in
> overrides, now it might raise question in new readers mind as to why ':' is
> prepended

Yes, it's a shame LIBCOVERRIDE was defined with a leading : to begin
with, but now it's done that way I'm not sure it's worth redefining
(there could be classes or recipe in other layers which reference
LIBCOVERRIDE and expect the leading : to be there).

  http://git.openembedded.org/openembedded-core/commit/?id=6e2b53f47da0e97271fb51b281d24da1e1d549cc

> so ideally it would be good to define LIBCOVERRIDE = "libc-<libc>" but that
> might add useless : in case of native recipes
>
> Maybe bitbake should just eat consecutive separators without consequence but
> that seems less than ideal too.

Bitbake already does eat consecutive separators (it needs to for
native recipes, since native recipes set MACHINEOVERRIDES to "").

>>     PREFERRED_PROVIDER_virtual/libiconv ?= "glibc"
>>   PREFERRED_PROVIDER_virtual/nativesdk-libiconv ?= "nativesdk-glibc"
>> diff --git a/meta/conf/distro/include/tclibc-musl.inc
>> b/meta/conf/distro/include/tclibc-musl.inc
>> index 54e1acf..052ae0e 100644
>> --- a/meta/conf/distro/include/tclibc-musl.inc
>> +++ b/meta/conf/distro/include/tclibc-musl.inc
>> @@ -4,9 +4,7 @@
>>     LIBCEXTENSION = "-musl"
>>   -# Add musl libc overrides to the overrides.
>>   LIBCOVERRIDE = ":libc-musl"
>> -OVERRIDES .= "${LIBCOVERRIDE}"
>>     PREFERRED_PROVIDER_virtual/libc ?= "musl"
>>   PREFERRED_PROVIDER_virtual/libiconv ?= "musl"
>>
>