| Message ID | 20231226044457.3699093-1-Qi.Chen@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | devtool/standard: avoid KeyError | expand |
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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
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
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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.