[OE-core] [PATCH 1/1] base.bbclass: Check BuildStarted for HOSTTOOLS

[Robert Yang]

On 2/14/19 11:43 AM, Peter Kjellerstedt wrote:
>> -----Original Message-----
>> From: openembedded-core-bounces at lists.openembedded.org <openembedded-
>> core-bounces at lists.openembedded.org> On Behalf Of Robert Yang
>> Sent: den 14 februari 2019 04:22
>> To: Peter Kjellerstedt <peter.kjellerstedt at axis.com>; openembedded-
>> core at lists.openembedded.org
>> Subject: Re: [OE-core] [PATCH 1/1] base.bbclass: Check BuildStarted for
>> HOSTTOOLS
>>
>> On 2/14/19 11:05 AM, Peter Kjellerstedt wrote:
>>>> -----Original Message-----
>>>> From: openembedded-core-bounces at lists.openembedded.org
>> <openembedded-
>>>> core-bounces at lists.openembedded.org> On Behalf Of Robert Yang
>>>> Sent: den 14 februari 2019 03:59
>>>> To: openembedded-core at lists.openembedded.org
>>>> Subject: [OE-core] [PATCH 1/1] base.bbclass: Check BuildStarted for
>>>> HOSTTOOLS
>>>>
>>>> There might be no bb.event.ConfigParsed event bitbake server is
>>>> running,
>>>> so check bb.event.BuildStarted to make sure HOSTTOOLS_DIR existed
>> when
>>>> BuildStarted.
>>>>
>>>> Fixed:
>>>> $ export BB_SERVER_TIMEOUT=-1
>>>> $ bitbake quilt-native
>>>> $ rm -fr tmp
>>>> $ bitbake quilt-native
>>>> ERROR: Error running gcc  --version: /bin/sh: gcc: command not found
>>>>
>>>> This error is caused by enable_uninative(), it runs twice
>> (ConfigParsed
>>>> and
>>>> BuildStarted), the error would happen when there is no ConfigParsed
>>>> event
>>>> (no hosttools is created), but BuildStarted. This patch can fix the
>>>> problem.
>>>>
>>>> [YOCTO #13022]
>>>>
>>>> Signed-off-by: Robert Yang <liezhi.yang at windriver.com>
>>>> ---
>>>>    meta/classes/base.bbclass | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
>>>> index f1a3c0e..283cb0f 100644
>>>> --- a/meta/classes/base.bbclass
>>>> +++ b/meta/classes/base.bbclass
>>>> @@ -224,7 +224,11 @@ base_eventhandler[eventmask] =
>>>> "bb.event.ConfigParsed bb.event.MultiConfigParsed
>>>>    python base_eventhandler() {
>>>>        import bb.runqueue
>>>>
>>>> -    if isinstance(e, bb.event.ConfigParsed):
>>>> +    # There might be no bb.event.ConfigParsed event bitbake server
>> is running,
>>>> +    # so check bb.event.BuildStarted to make sure HOSTTOOLS_DIR
>> existed when
>>>> +    # BuildStarted.
>>>> +    if isinstance(e, bb.event.ConfigParsed) or \
>>>> +            (isinstance(e, bb.event.BuildStarted) and not
>> os.path.exists(d.getVar('HOSTTOOLS_DIR'))):
>>>
>>> I don't think you want to do this here since not all of this if
>> statement
>>
>> That can make the code simple and avoid duplicated, the only code which
>> may run twice is "d.setVar('BB_VERSION', bb.__version__)", I don't
>> think it's a problem since it is very light.
> 
> Well, true, but I still don't think you should mix them. Have one case
> for what only needs to be done for bb.event.ConfigParsed, one for only
> bb.event.BuildStarted and one for either.

Yeah, it's not a big problem, I will update it in V2.

> 
>>> should be done for the BuildStarted event. I think it is better to
>> add a
>>> separate if statement just for the HOSTTOOLS part (see below). Also,
>> I don't
>>> think you should be checking if the HOSTTOOLS_DIR exists, because the
>>> contents of ${HOSTTOOLS} and ${HOSTTOOLS_NONFATAL} may have changed,
>> in
>>> which case any new tools need to be added to the directory.
>>
>> You don't have to worry about this since bitbake server can handle it
>> when HOSTTOOLS are changed in conf files, you can try this:
>>
>> $ export BB_SERVER_TIMEOUT=-1
>> $ bitbake quilt-native
>>
>> Add HOSTTOOLS += "passwd" to conf/local.conf, and:
>>
>> $ bitbake quilt-native
>> $ ls tmp/hosttools/ | grep passwd
>> passwd
>>
>> The passwd will be created.
>>
>>>
>>>>            if not d.getVar("NATIVELSBSTRING", False):
>>>>                d.setVar("NATIVELSBSTRING", lsb_distro_identifier(d))
>>>>            d.setVar('BB_VERSION', bb.__version__)
>>>> --
>>>> 2.7.4
>>>
>>> This is my suggestion for how to split the current handling of
>>> bb.event.ConfigParsed (also note that I corrected the comment before
>>> the new if statement):
>>>
>>>       if isinstance(e, bb.event.ConfigParsed):
>>>           if not d.getVar("NATIVELSBSTRING", False):
>>>               d.setVar("NATIVELSBSTRING", lsb_distro_identifier(d))
>>>           d.setVar('BB_VERSION', bb.__version__)
>>>
>>>       # There might be no bb.event.ConfigParsed event if the bitbake server is
>>
>> Thanks for the correction.
>>
>>>       # running, so check bb.event.BuildStarted too to make sure ${HOSTTOOLS_DIR}
>>>       # exists.
>>>       if isinstance(e, bb.event.ConfigParsed) or isinstance(e, bb.event.BuildStarted):
>>
>> I'm afraid that it doesn't work since it would make hosttools generate
>> twice when bitbake sever is not running, which seems not a good idea.
> 
> I don't think that's a problem in practice since the generation of

It may cause a race issue when tools in tmp/hosttools are being used, but 
BuildStarted removes them at the same time.

// Robert


> ${HOSTTOLS_DIR} should be pretty fast if it is already populated, but I
> guess you could use your original if statement to avoid that:
> 
>      if isinstance(e, bb.event.ConfigParsed) or \
>              (isinstance(e, bb.event.BuildStarted) and not os.path.exists(d.getVar('HOSTTOOLS_DIR'))):
> 
>> // Robert
>>
>>>           # Works with the line in layer.conf which changes PATH to point here
>>>           setup_hosttools_dir(d.getVar('HOSTTOOLS_DIR'), 'HOSTTOOLS', d)
>>>           setup_hosttools_dir(d.getVar('HOSTTOOLS_DIR'), 'HOSTTOOLS_NONFATAL', d, fatal=False)
>>>
>>> //Peter
>>>
>>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core at lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>