[PATCH] build: Add "family/" prefix to BSP familiy enable

[Chris Johns]

On 22/7/21 4:44 pm, Sebastian Huber wrote:
> On 22/07/2021 08:37, Chris Johns wrote:
>> On 22/7/21 4:20 pm, Sebastian Huber wrote:
>>> On 22/07/2021 02:39, Chris Johns wrote:
>>>> On 22/7/21 5:22 am, Sebastian Huber wrote:
>>>>> BSP family and BSP variant names may be equal.  This prefix avoids
>>>>> ambiguity in the enabled-by expressions.
>>>>> ---
>>>>>    wscript | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/wscript b/wscript
>>>>> index f27dba6831..b7a0412150 100755
>>>>> --- a/wscript
>>>>> +++ b/wscript
>>>>> @@ -1394,7 +1394,7 @@ def configure_variant(conf, cp, bsp_map, path_list,
>>>>> top_group, variant):
>>>>>        conf.env["ENABLE"] = [
>>>>>            get_compiler(conf, cp, variant),
>>>>>            arch,
>>>>> -        arch_family,
>>>>> +        "family/" + arch_family,
>>>>              "bsps/" + arch_family,
>>>>
>>>> ... as discussed in the other thread? If you are happy with the change as shown
>>>> please push.
>>>
>>> Yes, this is good and matches with our directory layout. I checked it in with
>>> this change.
>>>
>>> We could also merge the default-by-family and default-by-variant lists with this
>>> approach,
>>
>> I am not sure. My initial reaction was "yes" but how would different settings
>> for a BSP and a family be handled? I am assuming a BSP variant setting is able
>> to override a family setting. Is that possible if they are merged?
> 
> Yes, a BSP variant would have higher priority, this is enforced by the search
> order:
> 
>         for default in self.data["default-by-variant"]:
>             if OptionItem._is_variant(default["variants"], variant):
>                 value = default["value"]
>                 break
>         for default in self.data["default-by-family"]:
>             if OptionItem._is_variant(default["families"], family):
>                 value = default["value"]
>                 break

Then I am fine with the merging back to a single default-by-variant.

>>> for example:
>>>
>>> diff --git a/spec/build/bsps/optconsolebaud.yml
>>> b/spec/build/bsps/optconsolebaud.yml
>>> index 4b0869beca..0233fdd61b 100644
>>> --- a/spec/build/bsps/optconsolebaud.yml
>>> +++ b/spec/build/bsps/optconsolebaud.yml
>>> @@ -6,13 +6,10 @@ build-type: option
>>>   copyrights:
>>>   - Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
>>>   default: 115200
>>> -default-by-family:
>>> -- value: 9600
>>> -  families:
>>> -  - powerpc/motorola_powerpc
>>>   default-by-variant:
>>>   - value: 9600
>>>     variants:
>>> +  - bsps/powerpc/motorola_powerpc
>>
>> Oh I think my patch piece may have been wrong. This has two `/` and so three
>> components. We _must_ have unique family names 
> 
> The BSP family names are just names in an architecture directory, so this rule
> would be not enforced by the directory layout.

Sure and that fine.

>> and so the arch part is not
>> really needed. My concern is this type code ...
>>
>> https://git.rtems.org/rtems_waf/tree/rtems.py#n758
>>
>> that breaks. Is this an issue? I think a single `/` in a BSP or family is
>> cleaner.
> 
> Why is this an issue? This BSP family stuff is local to the RTEMS build system.

Currently. It is about the symmetry of the naming and how it would look from
outside. Nothing more.

Chris