[v2] cve-check: print warning if file is missing

Submitted by Armin Kuster on July 27, 2020, 2:52 a.m. | Patch ID: 174786

Details

Message ID 20200727025231.24418-1-akuster808@gmail.com
State New
Headers show

Commit Message

Armin Kuster July 27, 2020, 2:52 a.m.
If the "tmp/cve_check" file was not create as in the case for -c populate_sdk, just print a
warning instead of dumping a trace back

---
v2] lets include the missing part of the patch

Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 meta/classes/cve-check.bbclass | 4 ++++
 1 file changed, 4 insertions(+)

Patch hide | download patch | download mbox

diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
index 0889e7544aa..b16da9890af 100644
--- a/meta/classes/cve-check.bbclass
+++ b/meta/classes/cve-check.bbclass
@@ -55,6 +55,10 @@  python cve_save_summary_handler () {
 
     cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
 
+    if not os.path.isfile(cve_tmp_file):
+        bb.warn("File Not found: %s" % cve_tmp_file)
+        return
+
     cve_summary_name = d.getVar("CVE_CHECK_SUMMARY_FILE_NAME")
     cvelogpath = d.getVar("CVE_CHECK_SUMMARY_DIR")
     bb.utils.mkdirhier(cvelogpath)

Comments

Richard Purdie July 27, 2020, 9:39 a.m.
On Sun, 2020-07-26 at 19:52 -0700, akuster wrote:
> If the "tmp/cve_check" file was not create as in the case for -c
> populate_sdk, just print a
> warning instead of dumping a trace back
> 
> ---
> v2] lets include the missing part of the patch
> 
> Signed-off-by: Armin Kuster <akuster808@gmail.com>
> ---
>  meta/classes/cve-check.bbclass | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-
> check.bbclass
> index 0889e7544aa..b16da9890af 100644
> --- a/meta/classes/cve-check.bbclass
> +++ b/meta/classes/cve-check.bbclass
> @@ -55,6 +55,10 @@ python cve_save_summary_handler () {
>  
>      cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
>  
> +    if not os.path.isfile(cve_tmp_file):
> +        bb.warn("File Not found: %s" % cve_tmp_file)
> +        return
> +

A warning is something the user really needs to fix or take some action
about. Is this a warning?

Cheers,

Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#140976): https://lists.openembedded.org/g/openembedded-core/message/140976
Mute This Topic: https://lists.openembedded.org/mt/75815509/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Armin Kuster July 27, 2020, 1:57 p.m.
On 7/27/20 2:39 AM, Richard Purdie wrote:
> On Sun, 2020-07-26 at 19:52 -0700, akuster wrote:
>> If the "tmp/cve_check" file was not create as in the case for -c
>> populate_sdk, just print a
>> warning instead of dumping a trace back
>>
>> ---
>> v2] lets include the missing part of the patch
>>
>> Signed-off-by: Armin Kuster <akuster808@gmail.com>
>> ---
>>  meta/classes/cve-check.bbclass | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-
>> check.bbclass
>> index 0889e7544aa..b16da9890af 100644
>> --- a/meta/classes/cve-check.bbclass
>> +++ b/meta/classes/cve-check.bbclass
>> @@ -55,6 +55,10 @@ python cve_save_summary_handler () {
>>  
>>      cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
>>  
>> +    if not os.path.isfile(cve_tmp_file):
>> +        bb.warn("File Not found: %s" % cve_tmp_file)
>> +        return
>> +
> A warning is something the user really needs to fix or take some action
> about. Is this a warning?
Its actually an error in certail conditions. Do we want this to be
silent or fail on 'populate_sdk'?
>
> Cheers,
>
> Richard
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#140982): https://lists.openembedded.org/g/openembedded-core/message/140982
Mute This Topic: https://lists.openembedded.org/mt/75815509/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Richard Purdie July 27, 2020, 2:47 p.m.
On Mon, 2020-07-27 at 06:57 -0700, akuster808 wrote:
> 
> On 7/27/20 2:39 AM, Richard Purdie wrote:
> > On Sun, 2020-07-26 at 19:52 -0700, akuster wrote:
> > > If the "tmp/cve_check" file was not create as in the case for -c
> > > populate_sdk, just print a
> > > warning instead of dumping a trace back
> > > 
> > > ---
> > > v2] lets include the missing part of the patch
> > > 
> > > Signed-off-by: Armin Kuster <akuster808@gmail.com>
> > > ---
> > >  meta/classes/cve-check.bbclass | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-
> > > check.bbclass
> > > index 0889e7544aa..b16da9890af 100644
> > > --- a/meta/classes/cve-check.bbclass
> > > +++ b/meta/classes/cve-check.bbclass
> > > @@ -55,6 +55,10 @@ python cve_save_summary_handler () {
> > >  
> > >      cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
> > >  
> > > +    if not os.path.isfile(cve_tmp_file):
> > > +        bb.warn("File Not found: %s" % cve_tmp_file)
> > > +        return
> > > +
> > A warning is something the user really needs to fix or take some
> > action
> > about. Is this a warning?
> Its actually an error in certail conditions. Do we want this to be
> silent or fail on 'populate_sdk'?

What are the "certain conditions"?

Should it not detect those conditions and error if they occur?

Showing a warning and asking the user to guess is not a good user
experience. I have no idea whether I should worry about the warning
above or not for example, so I doubt anyone else does either.

Cheers,

Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#140984): https://lists.openembedded.org/g/openembedded-core/message/140984
Mute This Topic: https://lists.openembedded.org/mt/75815509/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Armin Kuster July 27, 2020, 4:05 p.m.
On 7/27/20 7:47 AM, Richard Purdie wrote:
> On Mon, 2020-07-27 at 06:57 -0700, akuster808 wrote:
>> On 7/27/20 2:39 AM, Richard Purdie wrote:
>>> On Sun, 2020-07-26 at 19:52 -0700, akuster wrote:
>>>> If the "tmp/cve_check" file was not create as in the case for -c
>>>> populate_sdk, just print a
>>>> warning instead of dumping a trace back
>>>>
>>>> ---
>>>> v2] lets include the missing part of the patch
>>>>
>>>> Signed-off-by: Armin Kuster <akuster808@gmail.com>
>>>> ---
>>>>  meta/classes/cve-check.bbclass | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-
>>>> check.bbclass
>>>> index 0889e7544aa..b16da9890af 100644
>>>> --- a/meta/classes/cve-check.bbclass
>>>> +++ b/meta/classes/cve-check.bbclass
>>>> @@ -55,6 +55,10 @@ python cve_save_summary_handler () {
>>>>  
>>>>      cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
>>>>  
>>>> +    if not os.path.isfile(cve_tmp_file):
>>>> +        bb.warn("File Not found: %s" % cve_tmp_file)
>>>> +        return
>>>> +
>>> A warning is something the user really needs to fix or take some
>>> action
>>> about. Is this a warning?
>> Its actually an error in certail conditions. Do we want this to be
>> silent or fail on 'populate_sdk'?
> What are the "certain conditions"?

populate_sdk

>
> Should it not detect those conditions and error if they occur?
if INHERIT += "cve-check' is included and -c populate_sdk, the file is
not generated.

>
> Showing a warning and asking the user to guess is not a good user
> experience. I have no idea whether I should worry about the warning
> above or not for example, so I doubt anyone else does either.

I am fine with not sending any warning.

- armin
>
> Cheers,
>
> Richard
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#141004): https://lists.openembedded.org/g/openembedded-core/message/141004
Mute This Topic: https://lists.openembedded.org/mt/75815509/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Richard Purdie July 27, 2020, 4:13 p.m.
On Mon, 2020-07-27 at 09:05 -0700, akuster808 wrote:
> 
> On 7/27/20 7:47 AM, Richard Purdie wrote:
> > On Mon, 2020-07-27 at 06:57 -0700, akuster808 wrote:
> > > On 7/27/20 2:39 AM, Richard Purdie wrote:
> > > > On Sun, 2020-07-26 at 19:52 -0700, akuster wrote:
> > > > > If the "tmp/cve_check" file was not create as in the case for
> > > > > -c
> > > > > populate_sdk, just print a
> > > > > warning instead of dumping a trace back
> > > > > 
> > > > > ---
> > > > > v2] lets include the missing part of the patch
> > > > > 
> > > > > Signed-off-by: Armin Kuster <akuster808@gmail.com>
> > > > > ---
> > > > >  meta/classes/cve-check.bbclass | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/meta/classes/cve-check.bbclass
> > > > > b/meta/classes/cve-
> > > > > check.bbclass
> > > > > index 0889e7544aa..b16da9890af 100644
> > > > > --- a/meta/classes/cve-check.bbclass
> > > > > +++ b/meta/classes/cve-check.bbclass
> > > > > @@ -55,6 +55,10 @@ python cve_save_summary_handler () {
> > > > >  
> > > > >      cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
> > > > >  
> > > > > +    if not os.path.isfile(cve_tmp_file):
> > > > > +        bb.warn("File Not found: %s" % cve_tmp_file)
> > > > > +        return
> > > > > +
> > > > A warning is something the user really needs to fix or take
> > > > some
> > > > action
> > > > about. Is this a warning?
> > > Its actually an error in certail conditions. Do we want this to
> > > be
> > > silent or fail on 'populate_sdk'?
> > What are the "certain conditions"?
> 
> populate_sdk
> 
> > Should it not detect those conditions and error if they occur?
> if INHERIT += "cve-check' is included and -c populate_sdk, the file
> is
> not generated.
> 
> > Showing a warning and asking the user to guess is not a good user
> > experience. I have no idea whether I should worry about the warning
> > above or not for example, so I doubt anyone else does either.
> 
> I am fine with not sending any warning.

That sounds like the correct solution to me in this case or maybe a
bb.note() which won't be shown on the console but probably will be in
the logs.

Cheers,

Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#141006): https://lists.openembedded.org/g/openembedded-core/message/141006
Mute This Topic: https://lists.openembedded.org/mt/75815509/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-