[PATCH 0/2] Fix issue with npm shrinkwrap fetcher

[PATCH 0/2] Fix issue with npm shrinkwrap fetcher

[kamel.bouhara]

Hello all,

This small patch series aims to fix the following build issue faced
with some npm packages:

DEBUG: Executing python function do_fetch
DEBUG: Executing python function base_do_fetch
DEBUG: Python function base_do_fetch finished
DEBUG: Python function do_fetch finished
ERROR: Failure expanding variable TUNE_FEATURES_tune-armv7at-neon, expression was ${TUNE_FEATURES_tune-armv7at} neon which triggered exception RecursionError: maximum recursion depth exceeded while calling a Python object

After struggling a lot, I figured out that this only happen for npm
packages not having dependencies listed in their shrinkwrap file (e.g. bcryptjs).

Yet I still didn't got how is the json parsing impacting
the python context here ?

Please feel free to comment.

Thanks.

Kamel Bouhara (2):
  npm.bbclass: make shrinkwrap file optional
  recipetool: create: only add npmsw url if required

 meta/classes/npm.bbclass             | 31 +++++++++++++++++++++----------
 scripts/lib/recipetool/create_npm.py |  6 +++++-
 2 files changed, 26 insertions(+), 11 deletions(-)

-- 
2.11.0

[PATCH 1/2] npm.bbclass: make shrinkwrap file optional

[Kamel Bouhara]

Some packages don't have shrinkwrap file which
means no npmsw uri is provided in the recipe.

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
 meta/classes/npm.bbclass | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index 068032a1e5..d3dd1a9ab8 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -130,11 +130,17 @@ python npm_do_configure() {
     cached_manifest.pop("dependencies", None)
     cached_manifest.pop("devDependencies", None)
 
-    with open(orig_shrinkwrap_file, "r") as f:
-        orig_shrinkwrap = json.load(f)
+    has_shrinkwrap_file = True
 
-    cached_shrinkwrap = copy.deepcopy(orig_shrinkwrap)
-    cached_shrinkwrap.pop("dependencies", None)
+    try:
+        with open(orig_shrinkwrap_file, "r") as f:
+            orig_shrinkwrap = json.load(f)
+    except IOError:
+        has_shrinkwrap_file = False
+
+    if has_shrinkwrap_file:
+       cached_shrinkwrap = copy.deepcopy(orig_shrinkwrap)
+       cached_shrinkwrap.pop("dependencies", None)
 
     # Manage the dependencies
     progress = OutOfProgressHandler(d, r"^(\d+)/(\d+)$")
@@ -165,8 +171,10 @@ python npm_do_configure() {
             progress.write("%d/%d" % (progress_done, progress_total))
 
     dev = bb.utils.to_boolean(d.getVar("NPM_INSTALL_DEV"), False)
-    foreach_dependencies(orig_shrinkwrap, _count_dependency, dev)
-    foreach_dependencies(orig_shrinkwrap, _cache_dependency, dev)
+
+    if has_shrinkwrap_file:
+        foreach_dependencies(orig_shrinkwrap, _count_dependency, dev)
+        foreach_dependencies(orig_shrinkwrap, _cache_dependency, dev)
 
     # Configure the main package
     with tempfile.TemporaryDirectory() as tmpdir:
@@ -181,16 +189,19 @@ python npm_do_configure() {
                 cached_manifest[depkey] = {}
             cached_manifest[depkey][name] = version
 
-    _update_manifest("dependencies")
+    if has_shrinkwrap_file:
+        _update_manifest("dependencies")
 
     if dev:
-        _update_manifest("devDependencies")
+        if has_shrinkwrap_file:
+            _update_manifest("devDependencies")
 
     with open(cached_manifest_file, "w") as f:
         json.dump(cached_manifest, f, indent=2)
 
-    with open(cached_shrinkwrap_file, "w") as f:
-        json.dump(cached_shrinkwrap, f, indent=2)
+    if has_shrinkwrap_file:
+        with open(cached_shrinkwrap_file, "w") as f:
+            json.dump(cached_shrinkwrap, f, indent=2)
 }
 
 python npm_do_compile() {
-- 
2.11.0

[PATCH 2/2] recipetool: create: only add npmsw url if required

[Kamel Bouhara]

Before adding a npmsw fetcher to a recipe we
should first check if the generated shrinkwrap file
contains dependencies.

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
 scripts/lib/recipetool/create_npm.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/recipetool/create_npm.py b/scripts/lib/recipetool/create_npm.py
index 579b7ae48a..2bcae91dfa 100644
--- a/scripts/lib/recipetool/create_npm.py
+++ b/scripts/lib/recipetool/create_npm.py
@@ -204,6 +204,9 @@ class NpmRecipeHandler(RecipeHandler):
         self._run_npm_install(d, srctree, registry, dev)
         shrinkwrap_file = self._generate_shrinkwrap(d, srctree, dev)
 
+        with open(shrinkwrap_file, "r") as f:
+            shrinkwrap = json.load(f)
+
         if os.path.exists(lock_copy):
             bb.utils.movefile(lock_copy, lock_file)
 
@@ -226,7 +229,8 @@ class NpmRecipeHandler(RecipeHandler):
             value = origvalue.replace("version=" + data["version"], "version=${PV}")
             value = value.replace("version=latest", "version=${PV}")
             values = [line.strip() for line in value.strip('\n').splitlines()]
-            values.append(url_recipe)
+            if "dependencies" in shrinkwrap:
+                values.append(url_recipe)
             return values, None, 4, False
 
         (_, newlines) = bb.utils.edit_metadata(lines_before, ["SRC_URI"], _handle_srcuri)
-- 
2.11.0

Re: [OE-core] [PATCH 0/2] Fix issue with npm shrinkwrap fetcher

[Jean-Marie Lemetayer]

Hi Kamel,

Thanks for your work. I should have fixed this a long time ago but I
didn't have the time :(

There is actually an open bug for this issue:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=13901
I think you can add the reference to your commit messages. And close
it if merged.

Please add some tests to ensure that:
 1. devtool / recipetool do not create empty shrinkwrap file
 2. the npmsw fetcher allows empty shrinkwrap file
 3. the npm class allows empty shrinkwrap

See:
https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=22dd46cc34629d0750177fddff2e1c178c854340
https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=44b2ab8d5eb4fd1fc9a157fce37aaa7b7a7065e5
https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=f6728edb7e022b27223d28bd80ce40c2f2374a13

Also Sandra has already worked on it, but it didn't get merged. I
don't know why.

See:
https://lists.openembedded.org/g/bitbake-devel/topic/76459311#11643
https://lists.openembedded.org/g/bitbake-devel/message/11648

BR
Jean-Marie

On Thu, Jan 14, 2021 at 8:13 AM Kamel Bouhara <kamel.bouhara@bootlin.com> wrote:
>
> Hello all,
>
> This small patch series aims to fix the following build issue faced
> with some npm packages:
>
> DEBUG: Executing python function do_fetch
> DEBUG: Executing python function base_do_fetch
> DEBUG: Python function base_do_fetch finished
> DEBUG: Python function do_fetch finished
> ERROR: Failure expanding variable TUNE_FEATURES_tune-armv7at-neon, expression was ${TUNE_FEATURES_tune-armv7at} neon which triggered exception RecursionError: maximum recursion depth exceeded while calling a Python object
>
> After struggling a lot, I figured out that this only happen for npm
> packages not having dependencies listed in their shrinkwrap file (e.g. bcryptjs).
>
> Yet I still didn't got how is the json parsing impacting
> the python context here ?
>
> Please feel free to comment.
>
> Thanks.
>
> Kamel Bouhara (2):
>   npm.bbclass: make shrinkwrap file optional
>   recipetool: create: only add npmsw url if required
>
>  meta/classes/npm.bbclass             | 31 +++++++++++++++++++++----------
>  scripts/lib/recipetool/create_npm.py |  6 +++++-
>  2 files changed, 26 insertions(+), 11 deletions(-)
>
> --
> 2.11.0
>
>
> 
>

Re: [OE-core] [PATCH 0/2] Fix issue with npm shrinkwrap fetcher

[Kamel Bouhara]

On Thu, Jan 14, 2021 at 03:25:50PM +0100, Jean-Marie Lemetayer wrote:
> Hi Kamel,
>

Hi Jean-Marie,

> Thanks for your work. I should have fixed this a long time ago but I
> didn't have the time :(
>

Well its never too late !

> There is actually an open bug for this issue:
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=13901
> I think you can add the reference to your commit messages. And close
> it if merged.
>

OK, let's see if there are other comments then I shall resent with
proper commit messages.

> Please add some tests to ensure that:
>  1. devtool / recipetool do not create empty shrinkwrap file
>  2. the npmsw fetcher allows empty shrinkwrap file
>  3. the npm class allows empty shrinkwrap

Actually the fix is only to check wether or not the dependencies are
listed and if not we make sure the npm fetcher will not look for them.

So the empty shrinkwrap file is still created and allowed.

I've already done some test in both case with and w/o dependencies.

>
> See:
> https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=22dd46cc34629d0750177fddff2e1c178c854340
> https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=44b2ab8d5eb4fd1fc9a157fce37aaa7b7a7065e5
> https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=f6728edb7e022b27223d28bd80ce40c2f2374a13
>
> Also Sandra has already worked on it, but it didn't get merged. I
> don't know why.
>
> See:
> https://lists.openembedded.org/g/bitbake-devel/topic/76459311#11643
> https://lists.openembedded.org/g/bitbake-devel/message/11648
>

I see, yet I went a bit further ensuring the npm recipes will not call
the npmsw fetcher when no dependencies are given.

Thanks for you comments.

Cheers,
Kamel

> BR
> Jean-Marie
>
> On Thu, Jan 14, 2021 at 8:13 AM Kamel Bouhara <kamel.bouhara@bootlin.com> wrote:
> >
> > Hello all,
> >
> > This small patch series aims to fix the following build issue faced
> > with some npm packages:
> >
> > DEBUG: Executing python function do_fetch
> > DEBUG: Executing python function base_do_fetch
> > DEBUG: Python function base_do_fetch finished
> > DEBUG: Python function do_fetch finished
> > ERROR: Failure expanding variable TUNE_FEATURES_tune-armv7at-neon, expression was ${TUNE_FEATURES_tune-armv7at} neon which triggered exception RecursionError: maximum recursion depth exceeded while calling a Python object
> >
> > After struggling a lot, I figured out that this only happen for npm
> > packages not having dependencies listed in their shrinkwrap file (e.g. bcryptjs).
> >
> > Yet I still didn't got how is the json parsing impacting
> > the python context here ?
> >
> > Please feel free to comment.
> >
> > Thanks.
> >
> > Kamel Bouhara (2):
> >   npm.bbclass: make shrinkwrap file optional
> >   recipetool: create: only add npmsw url if required
> >
> >  meta/classes/npm.bbclass             | 31 +++++++++++++++++++++----------
> >  scripts/lib/recipetool/create_npm.py |  6 +++++-
> >  2 files changed, 26 insertions(+), 11 deletions(-)
> >
> > --
> > 2.11.0
> >
> >
> > 
> >

--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [OE-core] [PATCH 0/2] Fix issue with npm shrinkwrap fetcher

[Zoltan Boszormenyi]

2021. 01. 14. 16:40 keltezéssel, Kamel Bouhara írta:
> On Thu, Jan 14, 2021 at 03:25:50PM +0100, Jean-Marie Lemetayer wrote:
>> Hi Kamel,
>>
> 
> Hi Jean-Marie,
> 
>> Thanks for your work. I should have fixed this a long time ago but I
>> didn't have the time :(
>>
> 
> Well its never too late !
> 
>> There is actually an open bug for this issue:
>> https://bugzilla.yoctoproject.org/show_bug.cgi?id=13901
>> I think you can add the reference to your commit messages. And close
>> it if merged.

Please backport this series to Gatesgarth as soon
as possible because it's a problem there, too.
The series applies cleanly.

It fixes some of the issues I see with
https://www.npmjs.com/package/nanomsg which
still fails for a different reason.

Thanks in advance,
Zoltán

>>
> 
> OK, let's see if there are other comments then I shall resent with
> proper commit messages.
> 
>> Please add some tests to ensure that:
>>   1. devtool / recipetool do not create empty shrinkwrap file
>>   2. the npmsw fetcher allows empty shrinkwrap file
>>   3. the npm class allows empty shrinkwrap
> 
> Actually the fix is only to check wether or not the dependencies are
> listed and if not we make sure the npm fetcher will not look for them.
> 
> So the empty shrinkwrap file is still created and allowed.
> 
> I've already done some test in both case with and w/o dependencies.
> 
>>
>> See:
>> https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=22dd46cc34629d0750177fddff2e1c178c854340
>> https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=44b2ab8d5eb4fd1fc9a157fce37aaa7b7a7065e5
>> https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=f6728edb7e022b27223d28bd80ce40c2f2374a13
>>
>> Also Sandra has already worked on it, but it didn't get merged. I
>> don't know why.
>>
>> See:
>> https://lists.openembedded.org/g/bitbake-devel/topic/76459311#11643
>> https://lists.openembedded.org/g/bitbake-devel/message/11648
>>
> 
> I see, yet I went a bit further ensuring the npm recipes will not call
> the npmsw fetcher when no dependencies are given.
> 
> Thanks for you comments.
> 
> Cheers,
> Kamel
> 
>> BR
>> Jean-Marie
>>
>> On Thu, Jan 14, 2021 at 8:13 AM Kamel Bouhara <kamel.bouhara@bootlin.com> wrote:
>>>
>>> Hello all,
>>>
>>> This small patch series aims to fix the following build issue faced
>>> with some npm packages:
>>>
>>> DEBUG: Executing python function do_fetch
>>> DEBUG: Executing python function base_do_fetch
>>> DEBUG: Python function base_do_fetch finished
>>> DEBUG: Python function do_fetch finished
>>> ERROR: Failure expanding variable TUNE_FEATURES_tune-armv7at-neon, expression was ${TUNE_FEATURES_tune-armv7at} neon which triggered exception RecursionError: maximum recursion depth exceeded while calling a Python object
>>>
>>> After struggling a lot, I figured out that this only happen for npm
>>> packages not having dependencies listed in their shrinkwrap file (e.g. bcryptjs).
>>>
>>> Yet I still didn't got how is the json parsing impacting
>>> the python context here ?
>>>
>>> Please feel free to comment.
>>>
>>> Thanks.
>>>
>>> Kamel Bouhara (2):
>>>    npm.bbclass: make shrinkwrap file optional
>>>    recipetool: create: only add npmsw url if required
>>>
>>>   meta/classes/npm.bbclass             | 31 +++++++++++++++++++++----------
>>>   scripts/lib/recipetool/create_npm.py |  6 +++++-
>>>   2 files changed, 26 insertions(+), 11 deletions(-)
>>>
>>> --
>>> 2.11.0
>>>
>>>
>>>
>>>
> 
> --
> Kamel Bouhara, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
> 
> 
> 
> 
> 

Re: [OE-core] [PATCH 0/2] Fix issue with npm shrinkwrap fetcher

[Anuj Mittal]

On Wed, 2021-02-03 at 10:55 +0100, Zoltan Boszormenyi via
lists.openembedded.org wrote:
> 2021. 01. 14. 16:40 keltezéssel, Kamel Bouhara írta:
> > On Thu, Jan 14, 2021 at 03:25:50PM +0100, Jean-Marie Lemetayer
> > wrote:
> > > Hi Kamel,
> > > 
> > 
> > Hi Jean-Marie,
> > 
> > > Thanks for your work. I should have fixed this a long time ago
> > > but I
> > > didn't have the time :(
> > > 
> > 
> > Well its never too late !
> > 
> > > There is actually an open bug for this issue:
> > > https://bugzilla.yoctoproject.org/show_bug.cgi?id=13901
> > > I think you can add the reference to your commit messages. And
> > > close
> > > it if merged.
> 
> Please backport this series to Gatesgarth as soon
> as possible because it's a problem there, too.
> The series applies cleanly.

I will pick these for the next pull request.

Thanks,

Anuj