[OE-core][PATCH] devtool/standard: avoid KeyError

[OE-core][PATCH] devtool/standard: avoid KeyError

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

The initial_revs["."] does not have an initial value, resulting
in the following error:

  KeyError: '.'

The problem could be reproduced by running:

  devtool modify -n systemd </path/to/my/local/systemd/repo>

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 scripts/lib/devtool/standard.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 559fd45676..5d9b86ed6a 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -905,6 +905,7 @@ def modify(args, config, basepath, workspace):
                     (stdout, _) = bb.process.run('git rev-list --reverse devtool-base..HEAD', cwd=submodule_path)
                     commits[submodule] = stdout.split()
         else:
+            initial_revs["."] = None
             if os.path.exists(os.path.join(srctree, '.git')):
                 # Check if it's a tree previously extracted by us. This is done
                 # by ensuring that devtool-base and args.branch (devtool) exist.
-- 
2.34.1

Re: [OE-core][PATCH] devtool/standard: avoid KeyError

[ChenQi]

ping

On 12/26/23 12:44, Chen Qi via lists.openembedded.org wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
>
> The initial_revs["."] does not have an initial value, resulting
> in the following error:
>
>    KeyError: '.'
>
> The problem could be reproduced by running:
>
>    devtool modify -n systemd </path/to/my/local/systemd/repo>
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>   scripts/lib/devtool/standard.py | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
> index 559fd45676..5d9b86ed6a 100644
> --- a/scripts/lib/devtool/standard.py
> +++ b/scripts/lib/devtool/standard.py
> @@ -905,6 +905,7 @@ def modify(args, config, basepath, workspace):
>                       (stdout, _) = bb.process.run('git rev-list --reverse devtool-base..HEAD', cwd=submodule_path)
>                       commits[submodule] = stdout.split()
>           else:
> +            initial_revs["."] = None
>               if os.path.exists(os.path.join(srctree, '.git')):
>                   # Check if it's a tree previously extracted by us. This is done
>                   # by ensuring that devtool-base and args.branch (devtool) exist.
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#192906): https://lists.openembedded.org/g/openembedded-core/message/192906
> Mute This Topic: https://lists.openembedded.org/mt/103366660/7304865
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@eng.windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core][PATCH] devtool/standard: avoid KeyError

[Richard Purdie]

On Tue, 2023-12-26 at 12:44 +0800, Chen Qi via lists.openembedded.org
wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> The initial_revs["."] does not have an initial value, resulting
> in the following error:
> 
>   KeyError: '.'
> 
> The problem could be reproduced by running:
> 
>   devtool modify -n systemd </path/to/my/local/systemd/repo>
> 
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  scripts/lib/devtool/standard.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
> index 559fd45676..5d9b86ed6a 100644
> --- a/scripts/lib/devtool/standard.py
> +++ b/scripts/lib/devtool/standard.py
> @@ -905,6 +905,7 @@ def modify(args, config, basepath, workspace):
>                      (stdout, _) = bb.process.run('git rev-list --reverse devtool-base..HEAD', cwd=submodule_path)
>                      commits[submodule] = stdout.split()
>          else:
> +            initial_revs["."] = None
>              if os.path.exists(os.path.join(srctree, '.git')):
>                  # Check if it's a tree previously extracted by us. This is done
>                  # by ensuring that devtool-base and args.branch (devtool) exist.

Why aren't we seeing other reports of this? How is it reproduced?
Should we be adding an extra test to improve coverage?

I'm assuming this isn't related to:

https://bugzilla.yoctoproject.org/show_bug.cgi?id=15318

?

Cheers,

Richard

Re: [OE-core][PATCH] devtool/standard: avoid KeyError

[ChenQi]

On 1/20/24 01:19, Richard Purdie wrote:
> On Tue, 2023-12-26 at 12:44 +0800, Chen Qi via lists.openembedded.org
> wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> The initial_revs["."] does not have an initial value, resulting
>> in the following error:
>>
>>    KeyError: '.'
>>
>> The problem could be reproduced by running:
>>
>>    devtool modify -n systemd </path/to/my/local/systemd/repo>
>>
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>>   scripts/lib/devtool/standard.py | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
>> index 559fd45676..5d9b86ed6a 100644
>> --- a/scripts/lib/devtool/standard.py
>> +++ b/scripts/lib/devtool/standard.py
>> @@ -905,6 +905,7 @@ def modify(args, config, basepath, workspace):
>>                       (stdout, _) = bb.process.run('git rev-list --reverse devtool-base..HEAD', cwd=submodule_path)
>>                       commits[submodule] = stdout.split()
>>           else:
>> +            initial_revs["."] = None
>>               if os.path.exists(os.path.join(srctree, '.git')):
>>                   # Check if it's a tree previously extracted by us. This is done
>>                   # by ensuring that devtool-base and args.branch (devtool) exist.
> Why aren't we seeing other reports of this? How is it reproduced?

I checked the devtool OEQA, there' no test case to cover this.

You can see my reproduce step in my commit message.

When I was trying to add an oeqa to this case just now, I found the 
problem has been fixed by the following commit.

"""

commit 64d5db2f89b4f3712b55127215ae02ce50dd747a
Author: Jamin Lin <jamin_lin@aspeedtech.com>
Date:   Wed Jan 3 18:13:44 2024 +0800

     devtool: modify: fix exception

     Root Cause:
     initial_revs is an empty dictionary and do not have "." key.

     Traceback (most recent call last):
       File "scripts/devtool", line 349, in <module>
         ret = main()
       File "scripts/devtool", line 336, in main
         ret = args.func(args, config, basepath, workspace)
       File "scripts/lib/devtool/standard.py", line 922, in modify
         if not initial_revs["."]:
     KeyError: '.'

     Solution:
     check key exists, then get its value.

     (From OE-Core rev: fb0db5c48abb4d56233a175fdd349d18b972e452)

     Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
     Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
     Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

"""

So my patch can be ignored.

Regards,

Qi

> Should we be adding an extra test to improve coverage?
>
> I'm assuming this isn't related to:
>
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=15318
>
> ?
>
> Cheers,
>
> Richard

Re: [OE-core][PATCH] devtool/standard: avoid KeyError

[ChenQi]

On 1/22/24 13:00, Chen Qi via lists.openembedded.org wrote:
> On 1/20/24 01:19, Richard Purdie wrote:
>> On Tue, 2023-12-26 at 12:44 +0800, Chen Qi via lists.openembedded.org
>> wrote:
>>> From: Chen Qi <Qi.Chen@windriver.com>
>>>
>>> The initial_revs["."] does not have an initial value, resulting
>>> in the following error:
>>>
>>>    KeyError: '.'
>>>
>>> The problem could be reproduced by running:
>>>
>>>    devtool modify -n systemd </path/to/my/local/systemd/repo>
>>>
>>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>>> ---
>>>   scripts/lib/devtool/standard.py | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/scripts/lib/devtool/standard.py 
>>> b/scripts/lib/devtool/standard.py
>>> index 559fd45676..5d9b86ed6a 100644
>>> --- a/scripts/lib/devtool/standard.py
>>> +++ b/scripts/lib/devtool/standard.py
>>> @@ -905,6 +905,7 @@ def modify(args, config, basepath, workspace):
>>>                       (stdout, _) = bb.process.run('git rev-list 
>>> --reverse devtool-base..HEAD', cwd=submodule_path)
>>>                       commits[submodule] = stdout.split()
>>>           else:
>>> +            initial_revs["."] = None
>>>               if os.path.exists(os.path.join(srctree, '.git')):
>>>                   # Check if it's a tree previously extracted by us. 
>>> This is done
>>>                   # by ensuring that devtool-base and args.branch 
>>> (devtool) exist.
>> Why aren't we seeing other reports of this? How is it reproduced?
>
> I checked the devtool OEQA, there' no test case to cover this.
>
> You can see my reproduce step in my commit message.
>
> When I was trying to add an oeqa to this case just now, I found the 
> problem has been fixed by the following commit.
>
> """
>
> commit 64d5db2f89b4f3712b55127215ae02ce50dd747a
> Author: Jamin Lin <jamin_lin@aspeedtech.com>
> Date:   Wed Jan 3 18:13:44 2024 +0800
>
>     devtool: modify: fix exception
>
>     Root Cause:
>     initial_revs is an empty dictionary and do not have "." key.
>
>     Traceback (most recent call last):
>       File "scripts/devtool", line 349, in <module>
>         ret = main()
>       File "scripts/devtool", line 336, in main
>         ret = args.func(args, config, basepath, workspace)
>       File "scripts/lib/devtool/standard.py", line 922, in modify
>         if not initial_revs["."]:
>     KeyError: '.'
>
>     Solution:
>     check key exists, then get its value.
>
>     (From OE-Core rev: fb0db5c48abb4d56233a175fdd349d18b972e452)
>
>     Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>     Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>     Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> """
>
> So my patch can be ignored.
>
> Regards,
>
> Qi

To avoid any regression, I just sent out a patch to add a test case, 
devtool.DevtoolModifyTests.test_devtool_modify_git_no_extract.

Regards,

Qi

>
>> Should we be adding an extra test to improve coverage?
>>
>> I'm assuming this isn't related to:
>>
>> https://bugzilla.yoctoproject.org/show_bug.cgi?id=15318
>>
>> ?
>>
>> Cheers,
>>
>> Richard
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#194112): https://lists.openembedded.org/g/openembedded-core/message/194112
> Mute This Topic: https://lists.openembedded.org/mt/103366660/3618072
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>