[PATCH 1/1] RSB: Mitigate too short error reports

Chris Johns chrisj at rtems.org
Fri Jan 20 00:20:48 UTC 2023 


On 20/1/2023 3:13 am, Frank Kühndel wrote:
> Hi Joel,
> 
> On 1/19/23 15:08, Joel Sherrill wrote:
>> Subject:
>> Re: [PATCH 1/1] RSB: Mitigate too short error reports
>> From:
>> Joel Sherrill <joel at rtems.org>
>> Date:
>> 1/19/23, 15:08
>>
>> To:
>> Frank Kühndel <frank.kuehndel at embedded-brains.de>
>> CC:
>> Chris Johns <chrisj at rtems.org>, devel at rtems.org
>>
>>
>> On Thu, Jan 19, 2023 at 6:47 AM Frank Kühndel <
>> frank.kuehndel at embedded-brains.de> wrote:
>>
>>> Hello Chris,
>>> Hello Joel,
>>>
>>> On 1/16/23 18:27, Joel Sherrill wrote:
>>>> Subject:
>>>> Re: [PATCH 1/1] RSB: Mitigate too short error reports
>>>> From:
>>>> Joel Sherrill<joel at rtems.org>
>>>> Date:
>>>> 1/16/23, 18:27
>>>>
>>>> To:
>>>> Frank Kühndel<frank.kuehndel at embedded-brains.de>
>>>> CC:
>>>> Chris Johns<chrisj at rtems.org>,devel at rtems.org
>>>>
>>>>
>>>> On Mon, Jan 16, 2023 at 8:46 AM Frank Kühndel <
>>>> frank.kuehndel at embedded-brains.de> wrote:
>>>>
>>>>> Hi Chris,
>>>>>
>>>>> On 1/16/23 01:02, Chris Johns wrote:
>>>>>> Subject:
>>>>>> Re: [PATCH 1/1] RSB: Mitigate too short error reports
>>>>>> From:
>>>>>> Chris Johns<chrisj at rtems.org>
>>>>>> Date:
>>>>>> 1/16/23, 01:02
>>>>>>
>>>>>> To:
>>>>>> Frank Kühndel<frank.kuehndel at embedded-brains.de>,devel at rtems.org
>>>>>>
>>>>>>
>>>>>> On 22/12/2022 9:09 pm, Frank Kühndel wrote:
>>>>>>> On 12/21/22 00:06, Chris Johns wrote:
>>>>>>>> On 21/12/2022 3:44 am, Frank Kuehndel wrote:
>>>>>>>>> From: Frank Kühndel<frank.kuehndel at embedded-brains.de>
>>>>>>>>>
>>>>>>>>> Close #4642
>>>>>>>>> ---
>>>>>>>>>      source-builder/sb/ereport.py | 4 ++++
>>>>>>>>>      1 file changed, 4 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/source-builder/sb/ereport.py
>>>>> b/source-builder/sb/ereport.py
>>>>>>>>> index d8fb5f6..d391917 100755
>>>>>>>>> --- a/source-builder/sb/ereport.py
>>>>>>>>> +++ b/source-builder/sb/ereport.py
>>>>>>>>> @@ -55,6 +55,10 @@ def generate(name, opts, header = None, footer =
>>>>> None):
>>>>>>>>>                  with open(name, 'w') as l:
>>>>>>>>>                      l.write(os.linesep.join(r))
>>>>>>>>>                  log.notice('  See error report: %s' % (name))
>>>>>>>>> +            log.notice('  (Hint: The first error may be in front of
>>>>> a '
>>>>>>>>> +                'line containing\n'
>>>>>>>>> +                '  "Error 1" [GNU make] and may be only in the
>>> whole
>>>>> log '
>>>>>>>> Is this too specific to GNU make? What ifs a package uses cmake or
>>>>> something
>>>>>>>> else?
>>>>>>> As the text indicates, this is specific to GNU make. For those using
>>>>> something
>>>>>>> else reading this text will still hint that the first error may not be
>>>>> in the
>>>>>>> end of the report "and may be only in the whole log".
>>>>>>>
>>>>>>> Another weak point in this text is that by far not all errors
>>>>> originating from
>>>>>>> "make". Yet, the true trouble is the original "See error report: %s"
>>>>> where it
>>>>>>> can sometimes happen that the error is not in this "error report" at
>>>>> all.
>>>>>>> I found it difficult to find a wording which is short, clear and
>>>>> helpful to the
>>>>>>> reader and at the same time not confusing. I am not perfectly happy
>>>>> with the
>>>>>>> notice above. I just found it a reasonable compromise.
>>>>>>>
>>>>>>> If you prefer more generic texts - such as the examples below - I will
>>>>> send a
>>>>>>> new patch with the suggested test.
>>>>>>>
>>>>>>>        "Hint: The first error may be far way from the end of the
>>>>>>>        report and in cases can only be found in the whole build log."
>>>>>>>
>>>>>>>        "Hint: The error is most likely in the error report otherwise
>>>>>>>        see the whole build log [--log option]."
>>>>>>>
>>>>>>> If you find any such change might cause more confusion than it might
>>> be
>>>>> helpful,
>>>>>>> I think it better to close this bug than to try to fix it.
>>>>>>>
>>>>>> I think all you have written is valid and I have found the wording
>>>>> difficult.
>>>>>> There will never be a robust error message scanner or a simple full
>>>>> proof way to
>>>>>> find errors. The parallel builds makes tracking the errors difficult
>>> and
>>>>> the
>>>>>> point of error and end of the build a long distance apart.
>>>> I usually search the logs for "rror:" and that's the first time something
>>>> is reported
>>>> whether by make or gcc or whatever. It may not be the root cause but it
>>> gets
>>>> me to the first report.
>>>>
>>>> Cutting any of these long reports down is always going to be possible to
>>>> cut out the real issue. It's ok because it it's more than just an odd
>>> setup
>>>> issue on the host, someone will have to build locally to reproduce the
>>>> issue.
>>>> And then they will get the full output.
>>>>
>>>>
>>>>>> As a result I question the value of the report and wonder if it should
>>> be
>>>>>> removed. The report adds overhead to the build as the logging process
>>>>> needs to
>>>>>> maintain a buffer of lines that is always updating. Your attention and
>>>>> interest
>>>>>> around this feature highlights how problematic it is so maybe it is
>>>>> simpler and
>>>>>> better to remove it and we leave users to find the error in the log
>>> file.
>>>>>> I am happy to accept the report has not worked as a feature, remove it
>>>>> and in
>>>>>> the process we recover some overheads in the logging area of the RSB?
>>>>>>
>>>>> I am not against the error report and I do not say it is a useless
>>>>> feature. It is just that I found the message '  See error report: %s'
>>>>> confusing in those cases where the report does not contain the error
>>>>> at all because it is too short (the error report consists simply of the
>>>>> last 400 lines of the build log).
>>>>>
>>>>> To answer your question, I believe there is always a build log - no
>>>>> matter whether the `--log` option is used or not. In this case, removing
>>>>> the error report and pointing to the build log in case of error (for
>>>>> example like '  See build log: %s') would certainly solve all my
>>> concerns.
>>>> But on the build@ reports, it is nice to have something. Many times it
>>>> is possible to diagnose the issue. Just in the past fifteen minutes,
>>> there
>>>> was one which having the log made it clear that CentOS 7 and other older
>>>> distributions need to use a newer GCC. Having the info in the build@
>>>> message was more than enough to diagnose that.
>>>>
>>>>> On the other hand, implementing the error report took time and was
>>>>> certainly done with good reason. I do not feel like I should be the one
>>>>> deciding to remove it. Changing the message or simply closing
>>>>> https://devel.rtems.org/ticket/4642   would also be perfectly valid for
>>> me.
>>>> Changing the message to encourage --log or whatever guidance for
>>>> debug. I don't even mind it pointing to a URL with guidance on debugging
>>>> RSB build issues.
>>> When I understand Joel right, he would keep the error report and prefers
>>> to change the message.
>>
>> In a perfect world, there would also be improvements on what's reported
>> but we know there is no perfect solution.
>>
>>
>>> This brings the discussion back to decide on an
>>> appropriate text. To make progress, I suggests
>>>
>>>     '  See error report: %s\n'
>>>     '  Note: In some cases the error appears only in\n'
>>>     '  the complete build log (see --log option)'

Please use os.linesep and not `\n'. I tend to:

 os.linesep.join(['  See error report: %s',
                  '  Note: In some cases the error appears only in',
                  '  the complete build log (see --log option)'])

>>>
>>> If your are OK with this text, I am happy to prepare a patch. I also
>>> welcome any other suggestion. If we cannot find any solution, I prefer
>>> to close my ticket #4642 without any action taken.
>>>
>> I'm happy with text like that. Is there a ticket to improve the message
>> or should #4642 have a comment and the message improved?
>>
> I am aiming at closing my ticket. I can add a comment to #4642, referring to
> this mailing list discussion and summarizing its result. Then a will post a
> patch which should close that ticket, provided the patch gets accepted.

Frank, please consider a patch this with change approved. And I am sorry this
has dragged on so long so thank you for your patience and persistence.

Chris

> 
> Greetings
> Frank
> 
>> --joel
>>
>>
>>> Greetings
>>> fk
>>>
>>>> --joel
>>>>
>>>>
>>>>> Greetings ... and a happy new year to you
>>>>> Frank
>>>>>
>>>>> -- 
>>>>> embedded brains GmbH
>>>>> Herr Frank KÜHNDEL
>>>>> Dornierstr. 4
>>>>> 82178 Puchheim
>>>>> Germany
>>>>> email:frank.kuehndel at embedded-brains.de
>>>>> phone:  +49-89-18 94 741 - 23
>>>>> mobile: +49-176-15 22 06 - 11
>>>>> fax:    +49-89-18 94 741 - 08
>>>>>
>>>>> Registergericht: Amtsgericht München
>>>>> Registernummer: HRB 157899
>>>>> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
>>>>> Unsere Datenschutzerklärung finden Sie hier:
>>>>> https://embedded-brains.de/datenschutzerklaerung/
>>>>>
>>>>> _______________________________________________
>>>>> devel mailing list
>>>>> devel at rtems.org
>>>>> http://lists.rtems.org/mailman/listinfo/devel
>>
>

More information about the devel mailing list