diff mbox series

[1/1] devtool: Fix the wrong variable in srcuri_entry

Message ID 271614cc517d93437c74e12e4d86d2d3149574ea.1686657637.git.chiachiwu@google.com (mailing list archive)
State New
Headers show
Series [1/1] devtool: Fix the wrong variable in srcuri_entry | expand

Commit Message

Charlie Wu June 13, 2023, 12:56 p.m. UTC
devtool crashes when running "update-recipe" and append changes on the recipe.
"$ devtool update-recipe -a <layer> <recipe>"
Traceback (most recent call last):
...
File "/ovss/ovss_quanta/poky/scripts/lib/devtool/standard.py", line 1636, in srcuri_entry
    return 'file://%s%s' % (basepath, paramstr)
                            ^^^^^^^^
NameError: cannot access free variable 'basepath' where it is not associated with a value in enclosing scope

The input variable 'fname' should have the same meaning as the variable 'basepath'.
Modify the 'fname' to 'basepath' and solve the issue.

Signed-off-by: Charlie Wu <chiachiwu@google.com>
---
 scripts/lib/devtool/standard.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alexander Kanavin June 13, 2023, 1:30 p.m. UTC | #1
Thank you, the correct mailing list for this submission is openembedded-core.

Alex

On Tue, 13 Jun 2023 at 14:56, Charlie Wu via lists.yoctoproject.org
<chiachiwu=google.com@lists.yoctoproject.org> wrote:
>
> devtool crashes when running "update-recipe" and append changes on the recipe.
> "$ devtool update-recipe -a <layer> <recipe>"
> Traceback (most recent call last):
> ...
> File "/ovss/ovss_quanta/poky/scripts/lib/devtool/standard.py", line 1636, in srcuri_entry
>     return 'file://%s%s' % (basepath, paramstr)
>                             ^^^^^^^^
> NameError: cannot access free variable 'basepath' where it is not associated with a value in enclosing scope
>
> The input variable 'fname' should have the same meaning as the variable 'basepath'.
> Modify the 'fname' to 'basepath' and solve the issue.
>
> Signed-off-by: Charlie Wu <chiachiwu@google.com>
> ---
>  scripts/lib/devtool/standard.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
> index 852672810a..86ca77f718 100644
> --- a/scripts/lib/devtool/standard.py
> +++ b/scripts/lib/devtool/standard.py
> @@ -1629,7 +1629,7 @@ def _update_recipe_patch(recipename, workspace, srctree, rd, appendlayerdir, wil
>      else:
>          patchdir_params = {'patchdir': relpatchdir}
>
> -    def srcuri_entry(fname):
> +    def srcuri_entry(basepath):
>          if patchdir_params:
>              paramstr = ';' + ';'.join('%s=%s' % (k,v) for k,v in patchdir_params.items())
>          else:
> --
> 2.41.0.162.gfafddb0af9-goog
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#13083): https://lists.yoctoproject.org/g/poky/message/13083
> Mute This Topic: https://lists.yoctoproject.org/mt/99504639/1686489
> Group Owner: poky+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/poky/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie June 13, 2023, 8:43 p.m. UTC | #2
On Tue, 2023-06-13 at 12:56 +0000, Charlie Wu via
lists.yoctoproject.org wrote:
> devtool crashes when running "update-recipe" and append changes on the recipe.
> "$ devtool update-recipe -a <layer> <recipe>"
> Traceback (most recent call last):
> ...
> File "/ovss/ovss_quanta/poky/scripts/lib/devtool/standard.py", line 1636, in srcuri_entry
>     return 'file://%s%s' % (basepath, paramstr)
>                             ^^^^^^^^
> NameError: cannot access free variable 'basepath' where it is not associated with a value in enclosing scope
> 
> The input variable 'fname' should have the same meaning as the variable 'basepath'.
> Modify the 'fname' to 'basepath' and solve the issue.
> 
> Signed-off-by: Charlie Wu <chiachiwu@google.com>
> ---
>  scripts/lib/devtool/standard.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
> index 852672810a..86ca77f718 100644
> --- a/scripts/lib/devtool/standard.py
> +++ b/scripts/lib/devtool/standard.py
> @@ -1629,7 +1629,7 @@ def _update_recipe_patch(recipename, workspace, srctree, rd, appendlayerdir, wil
>      else:
>          patchdir_params = {'patchdir': relpatchdir}
>  
> -    def srcuri_entry(fname):
> +    def srcuri_entry(basepath):
>          if patchdir_params:
>              paramstr = ';' + ';'.join('%s=%s' % (k,v) for k,v in patchdir_params.items())
>          else:

Thanks for the patch. As Alex mentions, this needs to go to the OE-Core
mailing list. I also wondered if we're missing a test case since the
selftests should catch issues like this?

Could you see if there is something we should add to "oe-selftest -r
devtool" to catch this issue?

Thanks,

Richard
Charlie Wu June 16, 2023, 2 p.m. UTC | #3
Hi Richard,

I set up the environment to run "oe-selftest -r devtool" and all test cases
are passed.
I revisited our use case and checked the code again, I think this might be
related to kmeta
<https://docs.yoctoproject.org/4.0.10/kernel-dev/advanced.html?highlight=kmeta#recipe-space-metadata>
.
Since kmeta is using the folder name, thus the function
"_remove_file_entries" will not remove the .scc and .cfg files here
<https://github.com/openembedded/openembedded-core/blob/master/scripts/lib/devtool/standard.py#L1685>
and
put them into the "remaining" list.
And we call the problematic function "srcuri_entry" when the "remaining"
list is not empty.

I think we do not have recipes using kmeta inside openembedded-core, which
makes it hard to add another unit test for this.
Or we have other ways to make the "remaining" list not empty, but I do not
know how.

I will check the guide
<https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded> and
change to upload the patch to openembedded-core these days.

Thanks,
Charlie


On Wed, Jun 14, 2023 at 4:44 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Tue, 2023-06-13 at 12:56 +0000, Charlie Wu via
> lists.yoctoproject.org wrote:
> > devtool crashes when running "update-recipe" and append changes on the
> recipe.
> > "$ devtool update-recipe -a <layer> <recipe>"
> > Traceback (most recent call last):
> > ...
> > File "/ovss/ovss_quanta/poky/scripts/lib/devtool/standard.py", line
> 1636, in srcuri_entry
> >     return 'file://%s%s' % (basepath, paramstr)
> >                             ^^^^^^^^
> > NameError: cannot access free variable 'basepath' where it is not
> associated with a value in enclosing scope
> >
> > The input variable 'fname' should have the same meaning as the variable
> 'basepath'.
> > Modify the 'fname' to 'basepath' and solve the issue.
> >
> > Signed-off-by: Charlie Wu <chiachiwu@google.com>
> > ---
> >  scripts/lib/devtool/standard.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/lib/devtool/standard.py
> b/scripts/lib/devtool/standard.py
> > index 852672810a..86ca77f718 100644
> > --- a/scripts/lib/devtool/standard.py
> > +++ b/scripts/lib/devtool/standard.py
> > @@ -1629,7 +1629,7 @@ def _update_recipe_patch(recipename, workspace,
> srctree, rd, appendlayerdir, wil
> >      else:
> >          patchdir_params = {'patchdir': relpatchdir}
> >
> > -    def srcuri_entry(fname):
> > +    def srcuri_entry(basepath):
> >          if patchdir_params:
> >              paramstr = ';' + ';'.join('%s=%s' % (k,v) for k,v in
> patchdir_params.items())
> >          else:
>
> Thanks for the patch. As Alex mentions, this needs to go to the OE-Core
> mailing list. I also wondered if we're missing a test case since the
> selftests should catch issues like this?
>
> Could you see if there is something we should add to "oe-selftest -r
> devtool" to catch this issue?
>
> Thanks,
>
> Richard
>
diff mbox series

Patch

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 852672810a..86ca77f718 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1629,7 +1629,7 @@  def _update_recipe_patch(recipename, workspace, srctree, rd, appendlayerdir, wil
     else:
         patchdir_params = {'patchdir': relpatchdir}
 
-    def srcuri_entry(fname):
+    def srcuri_entry(basepath):
         if patchdir_params:
             paramstr = ';' + ';'.join('%s=%s' % (k,v) for k,v in patchdir_params.items())
         else: