[PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Jonathan Liu]

If ASSUME_PROVIDES contains libsdl-native, we need to add sdl-config
to HOSTTOOLS to allow access to the host sdl-config.

Signed-off-by: Jonathan Liu <net147@gmail.com>
---
 meta/conf/bitbake.conf | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 8e4f4bbb56..3ad905c917 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -471,6 +471,9 @@ HOSTTOOLS += " \
 # Tools needed to run testimage runtime image testing
 HOSTTOOLS += "ip ping ps scp ssh stty"
 
+# Link to sdl-config if using host SDL
+HOSTTOOLS += "${@bb.utils.contains('ASSUME_PROVIDES', 'libsdl-native', 'sdl-config', '', d)}"
+
 # Link to these if present
 HOSTTOOLS_NONFATAL += "aws ccache gcc-ar gpg ld.bfd ld.gold nc sftp socat sudo"
 
-- 
2.12.2

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Patrick Ohly]

On Thu, 2017-06-01 at 22:15 +1000, Jonathan Liu wrote:
> If ASSUME_PROVIDES contains libsdl-native, we need to add sdl-config
> to HOSTTOOLS to allow access to the host sdl-config.
> 
> Signed-off-by: Jonathan Liu <net147@gmail.com>
> ---
>  meta/conf/bitbake.conf | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index 8e4f4bbb56..3ad905c917 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -471,6 +471,9 @@ HOSTTOOLS += " \
>  # Tools needed to run testimage runtime image testing
>  HOSTTOOLS += "ip ping ps scp ssh stty"
>  
> +# Link to sdl-config if using host SDL
> +HOSTTOOLS += "${@bb.utils.contains('ASSUME_PROVIDES', 'libsdl-native', 'sdl-config', '', d)}"
> +

Why are you checking ASSUME_PROVIDES? The variable is called
ASSUME_PROVIDED.

Even if you had checked the right variable, is that really necessary?
I'm building qemu with ASSUME_PROVIDED += "libsdl-native" just fine on
Debian Jessie, without sdl-config in HOSTTOOLS.

Sorry for the late reply, going through my backlog... I see that this
has been merged. Probably needs to be reverted or fixed.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Jonathan Liu]

Hi Patrick,

Something is really strange. HOSTTOOLS doesn't appear to be working
anymore in pyro branch
If I do "bitbake -c devshell qemu-native", I can run the following
commands successfully which should have been filtered out by
HOSTTOOLS:

$ which sdl-config
/usr/bin/sdl-config
$ which ncdu
/usr/bin/ncdu
$ which whoami
/usr/bin/whoami

For some reason /usr/bin is contained in PATH. Previously it was filtered out.

Regards,
Jonathan

On 27 June 2017 at 19:05, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Thu, 2017-06-01 at 22:15 +1000, Jonathan Liu wrote:
>> If ASSUME_PROVIDES contains libsdl-native, we need to add sdl-config
>> to HOSTTOOLS to allow access to the host sdl-config.
>>
>> Signed-off-by: Jonathan Liu <net147@gmail.com>
>> ---
>>  meta/conf/bitbake.conf | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
>> index 8e4f4bbb56..3ad905c917 100644
>> --- a/meta/conf/bitbake.conf
>> +++ b/meta/conf/bitbake.conf
>> @@ -471,6 +471,9 @@ HOSTTOOLS += " \
>>  # Tools needed to run testimage runtime image testing
>>  HOSTTOOLS += "ip ping ps scp ssh stty"
>>
>> +# Link to sdl-config if using host SDL
>> +HOSTTOOLS += "${@bb.utils.contains('ASSUME_PROVIDES', 'libsdl-native', 'sdl-config', '', d)}"
>> +
>
> Why are you checking ASSUME_PROVIDES? The variable is called
> ASSUME_PROVIDED.
>
> Even if you had checked the right variable, is that really necessary?
> I'm building qemu with ASSUME_PROVIDED += "libsdl-native" just fine on
> Debian Jessie, without sdl-config in HOSTTOOLS.
>
> Sorry for the late reply, going through my backlog... I see that this
> has been merged. Probably needs to be reverted or fixed.
>
> --
> Best Regards, Patrick Ohly
>
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
>
>
>

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Patrick Ohly]

On Tue, 2017-06-27 at 19:53 +1000, Jonathan Liu wrote:
> Hi Patrick,
> 
> Something is really strange. HOSTTOOLS doesn't appear to be working
> anymore in pyro branch
> If I do "bitbake -c devshell qemu-native", I can run the following
> commands successfully which should have been filtered out by
> HOSTTOOLS:
> 
> $ which sdl-config
> /usr/bin/sdl-config
> $ which ncdu
> /usr/bin/ncdu
> $ which whoami
> /usr/bin/whoami
> 
> For some reason /usr/bin is contained in PATH. Previously it was filtered out.

I think that's done intentionally for devshell (and only for devshell)
because developers expect to have access to all host tools.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

On 27 June 2017 at 10:05, Patrick Ohly <patrick.ohly@intel.com> wrote:

> Sorry for the late reply, going through my backlog... I see that this
> has been merged. Probably needs to be reverted or fixed.
>

The typo fix is in ross/mut already.

Ross

[-- Attachment #2: Type: text/html, Size: 665 bytes --]

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Patrick Ohly]

On Tue, 2017-06-27 at 11:05 +0200, Patrick Ohly wrote:
> Even if you had checked the right variable, is that really necessary?
> I'm building qemu with ASSUME_PROVIDED += "libsdl-native" just fine on
> Debian Jessie, without sdl-config in HOSTTOOLS.

I'm still interested in learning what problem it fixes.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Jonathan Liu]

Hi Patrick,

The original problem was that bitbake would print out the error:
"libsdl-native is set to be ASSUME_PROVIDED but sdl-config can't be
found in PATH. Please either install it, or configure qemu not to
require sdl.", if "libsdl-native" was in ASSUME_PROVIDED even if the
host has sdl-config in its PATH.

This occurred really early for a clean build and bitbake would bail
out. The sanity check is in meta/classes/sanity.bbclass.

Regards,
Jonathan

On 27 June 2017 at 20:21, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Tue, 2017-06-27 at 11:05 +0200, Patrick Ohly wrote:
>> Even if you had checked the right variable, is that really necessary?
>> I'm building qemu with ASSUME_PROVIDED += "libsdl-native" just fine on
>> Debian Jessie, without sdl-config in HOSTTOOLS.
>
> I'm still interested in learning what problem it fixes.
>
> --
> Best Regards, Patrick Ohly
>
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
>
>
>

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Patrick Ohly]

On Tue, 2017-06-27 at 20:24 +1000, Jonathan Liu wrote:
> Hi Patrick,
> 
> The original problem was that bitbake would print out the error:
> "libsdl-native is set to be ASSUME_PROVIDED but sdl-config can't be
> found in PATH. Please either install it, or configure qemu not to
> require sdl.", if "libsdl-native" was in ASSUME_PROVIDED even if the
> host has sdl-config in its PATH.
> 
> This occurred really early for a clean build and bitbake would bail
> out. The sanity check is in meta/classes/sanity.bbclass.

I've not hit that problem, probably because the sanity check was not run
again when I changed ASSUME_PROVIDED. I can reproduce it in a clean
build directory without conf/sanity_info.

I think extending HOSTTOOLS merely to satisfy sanity.bbclass is the
wrong solution to the problem. It makes sdl-config available to all
recipes, which is unnecessary and potentially introduces back host
contamination.

It is unnecessary because the qemu recipe has special code that enables
the use of the host SDL when told to do so via ASSUME_PROVIDED.

Can you come up with a better solution, probably by patching
sanity.bbclass?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Jonathan Liu]

Hi Patrick,

On 27 June 2017 at 20:38, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Tue, 2017-06-27 at 20:24 +1000, Jonathan Liu wrote:
>> Hi Patrick,
>>
>> The original problem was that bitbake would print out the error:
>> "libsdl-native is set to be ASSUME_PROVIDED but sdl-config can't be
>> found in PATH. Please either install it, or configure qemu not to
>> require sdl.", if "libsdl-native" was in ASSUME_PROVIDED even if the
>> host has sdl-config in its PATH.
>>
>> This occurred really early for a clean build and bitbake would bail
>> out. The sanity check is in meta/classes/sanity.bbclass.
>
> I've not hit that problem, probably because the sanity check was not run
> again when I changed ASSUME_PROVIDED. I can reproduce it in a clean
> build directory without conf/sanity_info.
>
> I think extending HOSTTOOLS merely to satisfy sanity.bbclass is the
> wrong solution to the problem. It makes sdl-config available to all
> recipes, which is unnecessary and potentially introduces back host
> contamination.
>
> It is unnecessary because the qemu recipe has special code that enables
> the use of the host SDL when told to do so via ASSUME_PROVIDED.
>
> Can you come up with a better solution, probably by patching
> sanity.bbclass?

I can't think of any at this stage. Feel free to post a patch if you
come up with something better.

>
> --
> Best Regards, Patrick Ohly
>
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
>
>
>

Regards,
Jonathan

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Richard Purdie]

On Tue, 2017-06-27 at 11:05 +0200, Patrick Ohly wrote:
> On Thu, 2017-06-01 at 22:15 +1000, Jonathan Liu wrote:
> > 
> > If ASSUME_PROVIDES contains libsdl-native, we need to add sdl-
> > config
> > to HOSTTOOLS to allow access to the host sdl-config.
> > 
> > Signed-off-by: Jonathan Liu <net147@gmail.com>
> > ---
> >  meta/conf/bitbake.conf | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > index 8e4f4bbb56..3ad905c917 100644
> > --- a/meta/conf/bitbake.conf
> > +++ b/meta/conf/bitbake.conf
> > @@ -471,6 +471,9 @@ HOSTTOOLS += " \
> >  # Tools needed to run testimage runtime image testing
> >  HOSTTOOLS += "ip ping ps scp ssh stty"
> >  
> > +# Link to sdl-config if using host SDL
> > +HOSTTOOLS += "${@bb.utils.contains('ASSUME_PROVIDES', 'libsdl-
> > native', 'sdl-config', '', d)}"
> > +
> Why are you checking ASSUME_PROVIDES? The variable is called
> ASSUME_PROVIDED.
> 
> Even if you had checked the right variable, is that really necessary?
> I'm building qemu with ASSUME_PROVIDED += "libsdl-native" just fine
> on Debian Jessie, without sdl-config in HOSTTOOLS.
> 
> Sorry for the late reply, going through my backlog... I see that this
> has been merged. Probably needs to be reverted or fixed.

I've reverted this since I can't see how it can actually work or help
anything, unless someone has set "ASSUME_PROVIDES" to test this (which
is a variable that doesn't exist or is used anywhere).

Cheers,

Richard

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Jonathan Liu]

Hi Richard,

On 27 June 2017 at 22:54, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Tue, 2017-06-27 at 11:05 +0200, Patrick Ohly wrote:
>> On Thu, 2017-06-01 at 22:15 +1000, Jonathan Liu wrote:
>> >
>> > If ASSUME_PROVIDES contains libsdl-native, we need to add sdl-
>> > config
>> > to HOSTTOOLS to allow access to the host sdl-config.
>> >
>> > Signed-off-by: Jonathan Liu <net147@gmail.com>
>> > ---
>> >  meta/conf/bitbake.conf | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
>> > index 8e4f4bbb56..3ad905c917 100644
>> > --- a/meta/conf/bitbake.conf
>> > +++ b/meta/conf/bitbake.conf
>> > @@ -471,6 +471,9 @@ HOSTTOOLS += " \
>> >  # Tools needed to run testimage runtime image testing
>> >  HOSTTOOLS += "ip ping ps scp ssh stty"
>> >
>> > +# Link to sdl-config if using host SDL
>> > +HOSTTOOLS += "${@bb.utils.contains('ASSUME_PROVIDES', 'libsdl-
>> > native', 'sdl-config', '', d)}"
>> > +
>> Why are you checking ASSUME_PROVIDES? The variable is called
>> ASSUME_PROVIDED.
>>
>> Even if you had checked the right variable, is that really necessary?
>> I'm building qemu with ASSUME_PROVIDED += "libsdl-native" just fine
>> on Debian Jessie, without sdl-config in HOSTTOOLS.
>>
>> Sorry for the late reply, going through my backlog... I see that this
>> has been merged. Probably needs to be reverted or fixed.
>
> I've reverted this since I can't see how it can actually work or help
> anything, unless someone has set "ASSUME_PROVIDES" to test this (which
> is a variable that doesn't exist or is used anywhere).
>
> Cheers,
>
> Richard

It is a typo. The fix is in ross/mut -
http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=ross/mut&id=7605eb1e507d4ebc0c5b8d98a358be7c55e4ddd2.

Regards,
Jonathan

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Jonathan Liu]

Hi Richard,

On 27 June 2017 at 22:56, Jonathan Liu <net147@gmail.com> wrote:
> Hi Richard,
>
> On 27 June 2017 at 22:54, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Tue, 2017-06-27 at 11:05 +0200, Patrick Ohly wrote:
>>> On Thu, 2017-06-01 at 22:15 +1000, Jonathan Liu wrote:
>>> >
>>> > If ASSUME_PROVIDES contains libsdl-native, we need to add sdl-
>>> > config
>>> > to HOSTTOOLS to allow access to the host sdl-config.
>>> >
>>> > Signed-off-by: Jonathan Liu <net147@gmail.com>
>>> > ---
>>> >  meta/conf/bitbake.conf | 3 +++
>>> >  1 file changed, 3 insertions(+)
>>> >
>>> > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
>>> > index 8e4f4bbb56..3ad905c917 100644
>>> > --- a/meta/conf/bitbake.conf
>>> > +++ b/meta/conf/bitbake.conf
>>> > @@ -471,6 +471,9 @@ HOSTTOOLS += " \
>>> >  # Tools needed to run testimage runtime image testing
>>> >  HOSTTOOLS += "ip ping ps scp ssh stty"
>>> >
>>> > +# Link to sdl-config if using host SDL
>>> > +HOSTTOOLS += "${@bb.utils.contains('ASSUME_PROVIDES', 'libsdl-
>>> > native', 'sdl-config', '', d)}"
>>> > +
>>> Why are you checking ASSUME_PROVIDES? The variable is called
>>> ASSUME_PROVIDED.
>>>
>>> Even if you had checked the right variable, is that really necessary?
>>> I'm building qemu with ASSUME_PROVIDED += "libsdl-native" just fine
>>> on Debian Jessie, without sdl-config in HOSTTOOLS.
>>>
>>> Sorry for the late reply, going through my backlog... I see that this
>>> has been merged. Probably needs to be reverted or fixed.
>>
>> I've reverted this since I can't see how it can actually work or help
>> anything, unless someone has set "ASSUME_PROVIDES" to test this (which
>> is a variable that doesn't exist or is used anywhere).
>>
>> Cheers,
>>
>> Richard
>
> It is a typo. The fix is in ross/mut -
> http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=ross/mut&id=7605eb1e507d4ebc0c5b8d98a358be7c55e4ddd2.

Also it was indeed tested after the fix but the sanity check was
cached in build/conf/sanity_info and I didn't think to clear it.
So the sanity check wasn't re-run after I deleted bitbake.lock,
buildhistory, cache, sstate-cache, tmp and rebuilt.

Why is state data written into a "conf" folder anyway? Shouldn't this
go into another folder like "cache"? Users don't generally think to
wipe files in conf folder when they want to do a clean rebuild.

Regards,
Jonathan

Re: [PATCH v2] bitbake.conf: Add sdl-config to HOSTTOOLS if using host SDL

[Patrick Ohly]

On Tue, 2017-06-27 at 21:11 +1000, Jonathan Liu wrote:
> Hi Patrick,
> 
> On 27 June 2017 at 20:38, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > On Tue, 2017-06-27 at 20:24 +1000, Jonathan Liu wrote:
> >> Hi Patrick,
> >>
> >> The original problem was that bitbake would print out the error:
> >> "libsdl-native is set to be ASSUME_PROVIDED but sdl-config can't be
> >> found in PATH. Please either install it, or configure qemu not to
> >> require sdl.", if "libsdl-native" was in ASSUME_PROVIDED even if the
> >> host has sdl-config in its PATH.
> >>
> >> This occurred really early for a clean build and bitbake would bail
> >> out. The sanity check is in meta/classes/sanity.bbclass.
> >
> > I've not hit that problem, probably because the sanity check was not run
> > again when I changed ASSUME_PROVIDED. I can reproduce it in a clean
> > build directory without conf/sanity_info.
> >
> > I think extending HOSTTOOLS merely to satisfy sanity.bbclass is the
> > wrong solution to the problem. It makes sdl-config available to all
> > recipes, which is unnecessary and potentially introduces back host
> > contamination.
> >
> > It is unnecessary because the qemu recipe has special code that enables
> > the use of the host SDL when told to do so via ASSUME_PROVIDED.
> >
> > Can you come up with a better solution, probably by patching
> > sanity.bbclass?
> 
> I can't think of any at this stage.

Here's what qemu.inc does:

do_configure_prepend_class-native() {
        # Append build host pkg-config paths for native target since the host may provide sdl
        BHOST_PKGCONFIG_PATH=$(PATH=/usr/bin:/bin pkg-config --variable pc_path pkg-config || echo "")
        if [ ! -z "$BHOST_PKGCONFIG_PATH" ]; then
                export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:$BHOST_PKGCONFIG_PATH
        fi

insanity.bbclass could use the host pkg-config to ensure that sdl.pc is
installed.

> Feel free to post a patch if you come up with something better.

Sorry, I don't have time for that. I've filed
https://bugzilla.yoctoproject.org/show_bug.cgi?id=11725 so that we don't
forget about it.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.