New Build System Status

[Sebastian Huber]

On 04/12/2019 05:11, Chris Johns wrote:
> On 3/12/19 5:16 pm, Sebastian Huber wrote:
>> On 02/12/2019 23:24, Chris Johns wrote:
>>> On 2/12/19 5:42 pm, Sebastian Huber wrote:
>>>> On 01/12/2019 23:53, Chris Johns wrote:
>>>>> On 27/11/19 11:26 pm, Sebastian Huber wrote:
>>>>>> On 27/11/2019 13:17, Hesham Almatary wrote:
>>>>>>> On Wed, 27 Nov 2019 at 11:59, Sebastian Huber
>>>>>>> <sebastian.huber at embedded-brains.de>  wrote:
>>>>>>>> On 27/11/2019 12:49, Hesham Almatary wrote:
>>>>>>>>> Hi Sebastian,
>>>>>>>>>
>>>>>>>>> Thanks for that great effort. I'd aim to use this build system for my
>>>>>>>>> RISC-V development.
>>>>>>>>>
>>>>>>>>> I followed the user manual trying to build RISC-V targets and RTEMS
>>>>>>>>> (aaf7f8b84) and here are a few comments:
>>>>>>>>>
>>>>>>>>> * .waf bsp_defaults doesn't give an error when mistyping the BSP name,
>>>>>>>>> but just outputs an empty .ini file.
>>>>>>>> If there is no matching BSP, then you get nothing. I think this is the
>>>>>>>> right thing to do.
>>>>> If the user does not enter the correct BSP name we should provide an relevant
>>>>> error message on stderr and return a non-zero error code. I believe we should
>>>>> not silently move past an error without a clear indication there is a problem.
>>>> We are talking here about the "./waf bsp_defaults" command. I think it is
>>>> perfectly fine to output nothing if there is nothing to output. This is the
>>>> normal UNIX style. What happens if you grep for something which doesn't exits?
>>>> Anyway, I changed the command like this:
>>>>
>>>> $ ./waf bsp_defaults --rtems-specs=spec/build/cpukit
>>>> # The build specification contains no BSPs
>>>>
>>>> $ ./waf bsp_defaults --rtems-bsps=foobar,perfection
>>>> # No BSP matches with the specified patterns: 'foobar', 'perfection'
>>> Unix commands often cannot change because of the things users have built up
>>> around them. It does not mean what is provided is the right way. What we are
>>> discussing is going to be around a long time and we need to examine the detail
>>> so we can avoid breaking users with changes.
>>>
>>> In the case of grep it's default if no file names are provided is to read
>>> string. I do not think it relates.
>>>
>>> As I said before it makes scripting and tooling around the command problematic
>>> and complicated.
>>
>> Could you please give an example, why the no output if there is nothing to
>> output makes scripting and tooling complicated? If you search for the empty set
>> you get it.
> 
> 1. I see this being equivalent to a compiler or linker creating it's output file
> with 0 length on any error and you need to check the length to know there has
> been an error.
> 
> 2. In a script `set -e` catches errors, this would be broken and you need to add
> extra "stuff" to hack around this and also means if you actually knew things
> worked this way. Most people would assume an error and I think this is reasonable.
> 
> 3. In python you need to have code to handle a non-zero exit code from a
> subprocess call and in this single case you need add more code to check the
> output length being 0 then convert that to "you have an invalid BSP name".
> 
> I would like to see an exit code that is non-zero for all errors and an invalid
> BSP name is an error.

For me this is not an error, however, I changed these condition into a 
ctx.fatal():

$ ./waf bsp_list --rtems-bsps=foobar
No BSP matches with the specified patterns: 'foobar'

$ ./waf bsp_list --rtems-specs=spec/build/cpukit
The build specification contains no BSPs

$ ./waf bsp_defaults --rtems-specs=spec/build/cpukit
The build specification contains no BSPs

$ ./waf bsp_defaults --rtems-bsps=foobar
No BSP matches with the specified patterns: 'foobar'

With respect to

def is_in_white_list(variant, white_list):
     if not white_list:
         return True
     for pattern in white_list:
         if re.search(pattern, variant):
             return True
     return False

should this be changed like this

def is_in_white_list(variant, white_list):
     if not white_list:
         return True
     for pattern in white_list:
         if re.search("^" + pattern + "$", variant):
             return True
     return False

to avoid issues like this

$ ./waf bsp_list --rtems-bsps=riscv/rv64imac
riscv/rv64imac
riscv/rv64imac_medany

?

[...]
>>> The issue I am attempting to address is present no matter what solution you
>>> provide. I am wondering if in time we need to add some sort of diff and conflict
>>> resolution tool so a user can see how they stand against the defaults in any
>>> release we make? We cannot solve the problem however we can provide a standard
>>> way our users can.
>>
>> Adding a diff command would be easy, but I don't think we should overburden the
>> new build system with features before it is in wide spread use.
> 
> Yes this is a good point however there is a balance we need to find. We also
> need to establish an interface, a command line API if you prefer, we can
> document and maintain over a long period of time. The issue with outside things
> is a subtle dependence that grows around things that maybe internal and if this
> becomes important we may not able change something. I am not saying in this case
> it is true, I do not know. If an option is provided and a standard output format
>   generate we have an API and what we do internally does not matter.
> 
>> You can already do it with the existing command:
>>
>> ./waf bsp_defaults --rtems-bsps=somebsp > defaults.ini
>> diff -u myconfig.ini defaults.ini
> 
> Sure. I suggest we formally document this. Doing so serves the purpose of
> indicating there is an issue here users need to be mindful of and it provides a
> simple command for them to use.

Ok, I will update the documentation in the user manual.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.