Patchwork [3/4] default-distrovars: Add empty weak definition for DISTRO variable

login
register
mail settings
Submitter Khem Raj
Date Feb. 24, 2012, 3:33 a.m.
Message ID <bab8960657c873358e301979445c8fa4e81cc15f.1330053959.git.raj.khem@gmail.com>
Download mbox | patch
Permalink /patch/21893/
State New
Headers show

Comments

Khem Raj - Feb. 24, 2012, 3:33 a.m.
Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 meta/conf/distro/include/default-distrovars.inc |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Richard Purdie - Feb. 24, 2012, 4:21 p.m.
On Thu, 2012-02-23 at 19:33 -0800, Khem Raj wrote:
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  meta/conf/distro/include/default-distrovars.inc |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
> index 16b3108..dba204e 100644
> --- a/meta/conf/distro/include/default-distrovars.inc
> +++ b/meta/conf/distro/include/default-distrovars.inc
> @@ -41,6 +41,13 @@ NO32LIBS ??= "1"
>  
>  # Default to emitting logfiles if a build fails.
>  BBINCLUDELOGS ??= "yes"
> +
> +# dummy distro related variables
> +# they should be overridden by real distros
> +# these fallbacks only serve the purpose of
> +# oe-core standalone testability
> +
> +DISTRO ??= ""
>  SDK_VERSION ??= "oe-core.0"
>  DISTRO_VERSION ??= "oe-core.0"

Why do we need this? The commit message tells me what you're doing but
not why. I'm really getting frustrated with one line commit messages
with no details.

I don't think we should need this either, I'm not sure I like the
change. The whole idea was OE-Core could run "distroless".

Cheers,

Richard
Khem Raj - Feb. 24, 2012, 5:05 p.m.
On Fri, Feb 24, 2012 at 8:21 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Thu, 2012-02-23 at 19:33 -0800, Khem Raj wrote:
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> ---
>>  meta/conf/distro/include/default-distrovars.inc |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
>> index 16b3108..dba204e 100644
>> --- a/meta/conf/distro/include/default-distrovars.inc
>> +++ b/meta/conf/distro/include/default-distrovars.inc
>> @@ -41,6 +41,13 @@ NO32LIBS ??= "1"
>>
>>  # Default to emitting logfiles if a build fails.
>>  BBINCLUDELOGS ??= "yes"
>> +
>> +# dummy distro related variables
>> +# they should be overridden by real distros
>> +# these fallbacks only serve the purpose of
>> +# oe-core standalone testability
>> +
>> +DISTRO ??= ""
>>  SDK_VERSION ??= "oe-core.0"
>>  DISTRO_VERSION ??= "oe-core.0"
>
> Why do we need this? The commit message tells me what you're doing but
> not why. I'm really getting frustrated with one line commit messages
> with no details.
>
> I don't think we should need this either, I'm not sure I like the
> change. The whole idea was OE-Core could run "distroless".
>

There is an error case I dont remeber how I obtained that where it
prints bogus value if DISTRO is not
defined at all but and empty DISTRO var is distroless in essence anyway IMO

> Cheers,
>
> Richard
>
>
>
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Saul Wold - March 5, 2012, 9:36 p.m.
On 02/24/2012 09:05 AM, Khem Raj wrote:
> On Fri, Feb 24, 2012 at 8:21 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org>  wrote:
>> On Thu, 2012-02-23 at 19:33 -0800, Khem Raj wrote:
>>> Signed-off-by: Khem Raj<raj.khem@gmail.com>
>>> ---
>>>   meta/conf/distro/include/default-distrovars.inc |    7 +++++++
>>>   1 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
>>> index 16b3108..dba204e 100644
>>> --- a/meta/conf/distro/include/default-distrovars.inc
>>> +++ b/meta/conf/distro/include/default-distrovars.inc
>>> @@ -41,6 +41,13 @@ NO32LIBS ??= "1"
>>>
>>>   # Default to emitting logfiles if a build fails.
>>>   BBINCLUDELOGS ??= "yes"
>>> +
>>> +# dummy distro related variables
>>> +# they should be overridden by real distros
>>> +# these fallbacks only serve the purpose of
>>> +# oe-core standalone testability
>>> +
>>> +DISTRO ??= ""
>>>   SDK_VERSION ??= "oe-core.0"
>>>   DISTRO_VERSION ??= "oe-core.0"
>>
>> Why do we need this? The commit message tells me what you're doing but
>> not why. I'm really getting frustrated with one line commit messages
>> with no details.
>>
>> I don't think we should need this either, I'm not sure I like the
>> change. The whole idea was OE-Core could run "distroless".
>>
>
> There is an error case I dont remeber how I obtained that where it
> prints bogus value if DISTRO is not
> defined at all but and empty DISTRO var is distroless in essence anyway IMO
>
Khem,

Please let us know what the reproducer for this is.  Once we have a 
reproducer we can look into the issue.

Sau!

>> Cheers,
>>
>> Richard
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Khem Raj - March 5, 2012, 11:02 p.m.
On Mon, Mar 5, 2012 at 1:36 PM, Saul Wold <sgw@linux.intel.com> wrote:
> On 02/24/2012 09:05 AM, Khem Raj wrote:
>>
>> On Fri, Feb 24, 2012 at 8:21 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org>  wrote:
>>>
>>> On Thu, 2012-02-23 at 19:33 -0800, Khem Raj wrote:
>>>>
>>>> Signed-off-by: Khem Raj<raj.khem@gmail.com>
>>>> ---
>>>>  meta/conf/distro/include/default-distrovars.inc |    7 +++++++
>>>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/meta/conf/distro/include/default-distrovars.inc
>>>> b/meta/conf/distro/include/default-distrovars.inc
>>>> index 16b3108..dba204e 100644
>>>> --- a/meta/conf/distro/include/default-distrovars.inc
>>>> +++ b/meta/conf/distro/include/default-distrovars.inc
>>>> @@ -41,6 +41,13 @@ NO32LIBS ??= "1"
>>>>
>>>>  # Default to emitting logfiles if a build fails.
>>>>  BBINCLUDELOGS ??= "yes"
>>>> +
>>>> +# dummy distro related variables
>>>> +# they should be overridden by real distros
>>>> +# these fallbacks only serve the purpose of
>>>> +# oe-core standalone testability
>>>> +
>>>> +DISTRO ??= ""
>>>>  SDK_VERSION ??= "oe-core.0"
>>>>  DISTRO_VERSION ??= "oe-core.0"
>>>
>>>
>>> Why do we need this? The commit message tells me what you're doing but
>>> not why. I'm really getting frustrated with one line commit messages
>>> with no details.
>>>
>>> I don't think we should need this either, I'm not sure I like the
>>> change. The whole idea was OE-Core could run "distroless".
>>>
>>
>> There is an error case I dont remeber how I obtained that where it
>> prints bogus value if DISTRO is not
>> defined at all but and empty DISTRO var is distroless in essence anyway
>> IMO
>>
> Khem,
>
> Please let us know what the reproducer for this is.  Once we have a
> reproducer we can look into the issue.

yeah.

>
> Sau!
>
>
>>> Cheers,
>>>
>>> Richard
>>>
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Openembedded-core mailing list
>>> Openembedded-core@lists.openembedded.org
>>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Denys Dmytriyenko - March 22, 2012, 1:23 a.m.
On Mon, Mar 05, 2012 at 01:36:05PM -0800, Saul Wold wrote:
> On 02/24/2012 09:05 AM, Khem Raj wrote:
> >On Fri, Feb 24, 2012 at 8:21 AM, Richard Purdie
> ><richard.purdie@linuxfoundation.org>  wrote:
> >>On Thu, 2012-02-23 at 19:33 -0800, Khem Raj wrote:
> >>>Signed-off-by: Khem Raj<raj.khem@gmail.com>
> >>>---
> >>>  meta/conf/distro/include/default-distrovars.inc |    7 +++++++
> >>>  1 files changed, 7 insertions(+), 0 deletions(-)
> >>>
> >>>diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
> >>>index 16b3108..dba204e 100644
> >>>--- a/meta/conf/distro/include/default-distrovars.inc
> >>>+++ b/meta/conf/distro/include/default-distrovars.inc
> >>>@@ -41,6 +41,13 @@ NO32LIBS ??= "1"
> >>>
> >>>  # Default to emitting logfiles if a build fails.
> >>>  BBINCLUDELOGS ??= "yes"
> >>>+
> >>>+# dummy distro related variables
> >>>+# they should be overridden by real distros
> >>>+# these fallbacks only serve the purpose of
> >>>+# oe-core standalone testability
> >>>+
> >>>+DISTRO ??= ""
> >>>  SDK_VERSION ??= "oe-core.0"
> >>>  DISTRO_VERSION ??= "oe-core.0"
> >>
> >>Why do we need this? The commit message tells me what you're doing but
> >>not why. I'm really getting frustrated with one line commit messages
> >>with no details.
> >>
> >>I don't think we should need this either, I'm not sure I like the
> >>change. The whole idea was OE-Core could run "distroless".
> >>
> >
> >There is an error case I dont remeber how I obtained that where it
> >prints bogus value if DISTRO is not
> >defined at all but and empty DISTRO var is distroless in essence anyway IMO
> >
> Khem,
> 
> Please let us know what the reproducer for this is.  Once we have a
> reproducer we can look into the issue.

Richard, Saul, Khem,

I was able to track this issue down (at least the one I was having). DISTRO is 
used in OVERRIDES unconditionally:

OVERRIDES = "${TARGET_OS}:${TRANSLATED_TARGET_ARCH}:build-${BUILD_OS}:pn-${PN}:${MACHINEOVERRIDES}:${DISTROOVERRIDES}:forcevariable"
DISTROOVERRIDES ?= "${DISTRO}"

If DISTRO is not set, FILESPATH becomes littered with directories like 
files/${DISTRO} etc. It won't bomb until you try to eval it - i.e. 
manipulating FILESPATH directly with .= works fine, but calling e.g. 
base_set_filespath() throws this:

ERROR: Failure expanding variable FILESPATH, expression was ${@blah} which 
triggered exception SyntaxError: EOL while scanning string literal (FILESPATH, 
line 1)

I have a patch that basically does this in conf/bitbake.conf to fix it:

-DISTROOVERRIDES ?= "${DISTRO}"
+DISTROOVERRIDES ?= "${@d.getVar('DISTRO', True) or ''}"

A proper formatted patch to follow...
Khem Raj - March 22, 2012, 4:22 a.m.
On Wed, Mar 21, 2012 at 6:23 PM, Denys Dmytriyenko <denis@denix.org> wrote:
>
> -DISTROOVERRIDES ?= "${DISTRO}"
> +DISTROOVERRIDES ?= "${@d.getVar('DISTRO', True) or ''}"

whats wrong with patch I proposed ?
Khem Raj - March 22, 2012, 4:48 p.m.
On Wed, Mar 21, 2012 at 9:22 PM, Khem Raj <raj.khem@gmail.com> wrote:
> On Wed, Mar 21, 2012 at 6:23 PM, Denys Dmytriyenko <denis@denix.org> wrote:
>>
>> -DISTROOVERRIDES ?= "${DISTRO}"
>> +DISTROOVERRIDES ?= "${@d.getVar('DISTRO', True) or ''}"
>
> whats wrong with patch I proposed ?

Richard

I was in same situation as Denys when I did that patch. I think
this could be a good patch to consider
Richard Purdie - March 22, 2012, 5:52 p.m.
On Thu, 2012-03-22 at 09:48 -0700, Khem Raj wrote:
> On Wed, Mar 21, 2012 at 9:22 PM, Khem Raj <raj.khem@gmail.com> wrote:
> > On Wed, Mar 21, 2012 at 6:23 PM, Denys Dmytriyenko <denis@denix.org> wrote:
> >>
> >> -DISTROOVERRIDES ?= "${DISTRO}"
> >> +DISTROOVERRIDES ?= "${@d.getVar('DISTRO', True) or ''}"
> >
> > whats wrong with patch I proposed ?
> 
> Richard
> 
> I was in same situation as Denys when I did that patch. I think
> this could be a good patch to consider

I agree and its merged! :)

Cheers,

Richard

Patch

diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
index 16b3108..dba204e 100644
--- a/meta/conf/distro/include/default-distrovars.inc
+++ b/meta/conf/distro/include/default-distrovars.inc
@@ -41,6 +41,13 @@  NO32LIBS ??= "1"
 
 # Default to emitting logfiles if a build fails.
 BBINCLUDELOGS ??= "yes"
+
+# dummy distro related variables
+# they should be overridden by real distros
+# these fallbacks only serve the purpose of
+# oe-core standalone testability
+
+DISTRO ??= ""
 SDK_VERSION ??= "oe-core.0"
 DISTRO_VERSION ??= "oe-core.0"