diff mbox series

[1/1] support/scripts: add node modules to check-bin-arch's IGNORES list

Message ID 20190531075800.7064-1-vcrini@gmail.com
State Changes Requested
Headers show
Series [1/1] support/scripts: add node modules to check-bin-arch's IGNORES list | expand

Commit Message

Valerio Crini May 31, 2019, 7:58 a.m. UTC
From: Valerio Crini <valerio.crini@docomodigital.com>

Some node modules as stf ship compiled binaries causing check-bin-arch script
to return errors.

Signed-off-by: Valerio Crini <valerio.crini@docomodigital.com>
---
 support/scripts/check-bin-arch | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Thomas Petazzoni Aug. 4, 2019, 3:52 p.m. UTC | #1
Hello Valerio,

+Martin Bark in Cc.

On Fri, 31 May 2019 09:58:00 +0200
Valerio Crini <vcrini@gmail.com> wrote:

> From: Valerio Crini <valerio.crini@docomodigital.com>
> 
> Some node modules as stf ship compiled binaries causing check-bin-arch script
> to return errors.
> 
> Signed-off-by: Valerio Crini <valerio.crini@docomodigital.com>
> ---
>  support/scripts/check-bin-arch | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
> index 3449bd1aeb..a276b75b92 100755
> --- a/support/scripts/check-bin-arch
> +++ b/support/scripts/check-bin-arch
> @@ -25,6 +25,11 @@ declare -a IGNORES=(
>  	# it for a different architecture (e.g. i386 grub on x86_64).
>  	"/lib/grub"
>  	"/usr/lib/grub"
> +
> +	# Skip files in /usr/lib/node_modules, since it is possible to have it
> +	# for a different architecture (e.g. stf module has some packages already
> +	# compiled for several architectures).
> +	"/usr/lib/node_modules"

Thanks for providing this patch. Could you give a bit more details
about which libraries get installed, what is their name, etc. ?

Indeed, rather than ignoring files from /usr/lib/node_modules, it would
be a lot better to remove the libraries that are anyway not useful
because they don't match the CPU architecture of the target. This would
both reduce the filesystem size *and* solve the check-bin-arch issue.

If really this isn't possible, then we could define
NODEJS_BIN_ARCH_EXCLUDE in nodejs.mk, to exclude /usr/lib/node_modules/
from the check-bin-arch verification. It's not as good as preventing
the problem, but it's better than a global exclusion.

In order to investigate this, I tried to do a build with the stf NPM
package, but it failed on bufferutil. I tried the following Buildroot
defconfig:

BR2_arm=y
BR2_cortex_a8=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_NODEJS=y
BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL="stf"
# BR2_TARGET_ROOTFS_TAR is not set

And it resulted in this:

[...]
.../home/thomas/projets/buildroot/output/host/bin/npm install -g stf
[...]
> node-gyp rebuild || node suppress-error.js

  TOUCH Release/obj.target/DTraceProviderStub.stamp

> bufferutil@1.3.0 install /home/thomas/projets/buildroot/output/target/usr/lib/node_modules/stf/node_modules/bufferutil
> node-gyp rebuild

  CXX(target) Release/obj.target/bufferutil/src/bufferutil.o
In file included from ../../nan/nan.h:190:0,
                 from ../src/bufferutil.cc:16:
../../nan/nan_maybe_43_inl.h: In function ‘Nan::Maybe<bool> Nan::ForceSet(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Value>, v8::PropertyAttribute)’:
../../nan/nan_maybe_43_inl.h:88:15: error: ‘class v8::Object’ has no member named ‘ForceSet’
   return obj->ForceSet(GetCurrentContext(), key, value, attribs);
               ^~~~~~~~

Do you know what is going on ?

Thanks!

Thomas
Martin Bark Aug. 10, 2019, 3:36 p.m. UTC | #2
Hi,

On Sun, 4 Aug 2019 at 16:52, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Valerio,
>
> +Martin Bark in Cc.
>
> On Fri, 31 May 2019 09:58:00 +0200
> Valerio Crini <vcrini@gmail.com> wrote:
>
> > From: Valerio Crini <valerio.crini@docomodigital.com>
> >
> > Some node modules as stf ship compiled binaries causing check-bin-arch script
> > to return errors.
> >
> > Signed-off-by: Valerio Crini <valerio.crini@docomodigital.com>
> > ---
> >  support/scripts/check-bin-arch | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
> > index 3449bd1aeb..a276b75b92 100755
> > --- a/support/scripts/check-bin-arch
> > +++ b/support/scripts/check-bin-arch
> > @@ -25,6 +25,11 @@ declare -a IGNORES=(
> >       # it for a different architecture (e.g. i386 grub on x86_64).
> >       "/lib/grub"
> >       "/usr/lib/grub"
> > +
> > +     # Skip files in /usr/lib/node_modules, since it is possible to have it
> > +     # for a different architecture (e.g. stf module has some packages already
> > +     # compiled for several architectures).
> > +     "/usr/lib/node_modules"
>
> Thanks for providing this patch. Could you give a bit more details
> about which libraries get installed, what is their name, etc. ?
>
> Indeed, rather than ignoring files from /usr/lib/node_modules, it would
> be a lot better to remove the libraries that are anyway not useful
> because they don't match the CPU architecture of the target. This would
> both reduce the filesystem size *and* solve the check-bin-arch issue.

Yes i agree Thomas, this is the correct solution.  I would recommend
you create a custom package for your application and in the BUILD_CMDS
use $(NPM) install to install you node_modules followed by code to
filter out any files that do not match your CPU architecture.  Here is
a script i've used before to do this

# Remove executables which are not the correct target arch
for f in $(find $(@D) -executable -type f) ; do
    a=$(${TARGET_READELF} -h $f 2>&1 | sed -r -e '/^  Machine:
+(.+)/!d; s//\1/;')
    if [ ! -z "$a" -a "$a" != "${BR2_READELF_ARCH_NAME}" ] ; then
        echo "Remove: $f Arch: $a"
        rm -f $f
    fi
done

Thanks

Martin

>
> If really this isn't possible, then we could define
> NODEJS_BIN_ARCH_EXCLUDE in nodejs.mk, to exclude /usr/lib/node_modules/
> from the check-bin-arch verification. It's not as good as preventing
> the problem, but it's better than a global exclusion.
>
> In order to investigate this, I tried to do a build with the stf NPM
> package, but it failed on bufferutil. I tried the following Buildroot
> defconfig:
>
> BR2_arm=y
> BR2_cortex_a8=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
> BR2_INIT_NONE=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> BR2_PACKAGE_NODEJS=y
> BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL="stf"
> # BR2_TARGET_ROOTFS_TAR is not set
>
> And it resulted in this:
>
> [...]
> .../home/thomas/projets/buildroot/output/host/bin/npm install -g stf
> [...]
> > node-gyp rebuild || node suppress-error.js
>
>   TOUCH Release/obj.target/DTraceProviderStub.stamp
>
> > bufferutil@1.3.0 install /home/thomas/projets/buildroot/output/target/usr/lib/node_modules/stf/node_modules/bufferutil
> > node-gyp rebuild
>
>   CXX(target) Release/obj.target/bufferutil/src/bufferutil.o
> In file included from ../../nan/nan.h:190:0,
>                  from ../src/bufferutil.cc:16:
> ../../nan/nan_maybe_43_inl.h: In function ‘Nan::Maybe<bool> Nan::ForceSet(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Value>, v8::PropertyAttribute)’:
> ../../nan/nan_maybe_43_inl.h:88:15: error: ‘class v8::Object’ has no member named ‘ForceSet’
>    return obj->ForceSet(GetCurrentContext(), key, value, attribs);
>                ^~~~~~~~
>
> Do you know what is going on ?
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Yegor Yefremov Dec. 8, 2020, 3:36 p.m. UTC | #3
Hi all,

On Sat, Aug 10, 2019 at 5:44 PM Martin Bark <martin@barkynet.com> wrote:
>
> Hi,
>
> On Sun, 4 Aug 2019 at 16:52, Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > Hello Valerio,
> >
> > +Martin Bark in Cc.
> >
> > On Fri, 31 May 2019 09:58:00 +0200
> > Valerio Crini <vcrini@gmail.com> wrote:
> >
> > > From: Valerio Crini <valerio.crini@docomodigital.com>
> > >
> > > Some node modules as stf ship compiled binaries causing check-bin-arch script
> > > to return errors.
> > >
> > > Signed-off-by: Valerio Crini <valerio.crini@docomodigital.com>
> > > ---
> > >  support/scripts/check-bin-arch | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
> > > index 3449bd1aeb..a276b75b92 100755
> > > --- a/support/scripts/check-bin-arch
> > > +++ b/support/scripts/check-bin-arch
> > > @@ -25,6 +25,11 @@ declare -a IGNORES=(
> > >       # it for a different architecture (e.g. i386 grub on x86_64).
> > >       "/lib/grub"
> > >       "/usr/lib/grub"
> > > +
> > > +     # Skip files in /usr/lib/node_modules, since it is possible to have it
> > > +     # for a different architecture (e.g. stf module has some packages already
> > > +     # compiled for several architectures).
> > > +     "/usr/lib/node_modules"
> >
> > Thanks for providing this patch. Could you give a bit more details
> > about which libraries get installed, what is their name, etc. ?
> >
> > Indeed, rather than ignoring files from /usr/lib/node_modules, it would
> > be a lot better to remove the libraries that are anyway not useful
> > because they don't match the CPU architecture of the target. This would
> > both reduce the filesystem size *and* solve the check-bin-arch issue.
>
> Yes i agree Thomas, this is the correct solution.  I would recommend
> you create a custom package for your application and in the BUILD_CMDS
> use $(NPM) install to install you node_modules followed by code to
> filter out any files that do not match your CPU architecture.  Here is
> a script i've used before to do this
>
> # Remove executables which are not the correct target arch
> for f in $(find $(@D) -executable -type f) ; do
>     a=$(${TARGET_READELF} -h $f 2>&1 | sed -r -e '/^  Machine:
> +(.+)/!d; s//\1/;')
>     if [ ! -z "$a" -a "$a" != "${BR2_READELF_ARCH_NAME}" ] ; then
>         echo "Remove: $f Arch: $a"
>         rm -f $f
>     fi
> done

Any update on this issue?

I cannot reproduce this issue when installing ref-napi or ffi-napi via
BR Node.js BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL option and "make
nodejs-rebuild".

This issue comes only when I install my own package libonrisc [1] that
uses npm to install the module having these dependencies.Then I get
the following errors:

ERROR: architecture for
"/usr/lib/node_modules/libonrisc/node_modules/ffi-napi/node_modules/ref-napi/prebuilds/linux-arm64/electron.napi.armv8.node"
is "AArch64", should be "ARM"
ERROR: architecture for
"/usr/lib/node_modules/libonrisc/node_modules/ref-napi/prebuilds/linux-arm64/electron.napi.armv8.node"
is "AArch64", should be "ARM"
ERROR: architecture for
"/usr/lib/node_modules/libonrisc/node_modules/ref-array-napi/node_modules/ref-napi/prebuilds/linux-x64/electron.napi.node"
is "Advanced Micro Devices X86-64", should be "ARM"
ERROR: architecture for
"/usr/lib/node_modules/libonrisc/node_modules/ref-struct-napi/node_modules/ref-napi/prebuilds/linux-x64/electron.napi.node"
is "Advanced Micro Devices X86-64", should be "ARM"
ERROR: architecture for
"/usr/lib/node_modules/libonrisc/node_modules/ffi-napi/node_modules/ref-napi/prebuilds/linux-x64/electron.napi.node"
is "Advanced Micro Devices X86-64", should be "ARM"

[1] https://github.com/visionsystemsgmbh/onrisc_br_bsp/blob/master/package/libonrisc/libonrisc.mk#L23

Best regards,
Yegor

> >
> > If really this isn't possible, then we could define
> > NODEJS_BIN_ARCH_EXCLUDE in nodejs.mk, to exclude /usr/lib/node_modules/
> > from the check-bin-arch verification. It's not as good as preventing
> > the problem, but it's better than a global exclusion.
> >
> > In order to investigate this, I tried to do a build with the stf NPM
> > package, but it failed on bufferutil. I tried the following Buildroot
> > defconfig:
> >
> > BR2_arm=y
> > BR2_cortex_a8=y
> > BR2_TOOLCHAIN_EXTERNAL=y
> > BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
> > BR2_INIT_NONE=y
> > BR2_SYSTEM_BIN_SH_NONE=y
> > # BR2_PACKAGE_BUSYBOX is not set
> > BR2_PACKAGE_NODEJS=y
> > BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL="stf"
> > # BR2_TARGET_ROOTFS_TAR is not set
> >
> > And it resulted in this:
> >
> > [...]
> > .../home/thomas/projets/buildroot/output/host/bin/npm install -g stf
> > [...]
> > > node-gyp rebuild || node suppress-error.js
> >
> >   TOUCH Release/obj.target/DTraceProviderStub.stamp
> >
> > > bufferutil@1.3.0 install /home/thomas/projets/buildroot/output/target/usr/lib/node_modules/stf/node_modules/bufferutil
> > > node-gyp rebuild
> >
> >   CXX(target) Release/obj.target/bufferutil/src/bufferutil.o
> > In file included from ../../nan/nan.h:190:0,
> >                  from ../src/bufferutil.cc:16:
> > ../../nan/nan_maybe_43_inl.h: In function ‘Nan::Maybe<bool> Nan::ForceSet(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Value>, v8::PropertyAttribute)’:
> > ../../nan/nan_maybe_43_inl.h:88:15: error: ‘class v8::Object’ has no member named ‘ForceSet’
> >    return obj->ForceSet(GetCurrentContext(), key, value, attribs);
> >                ^~~~~~~~
> >
> > Do you know what is going on ?
> >
> > Thanks!
> >
> > Thomas
> > --
> > Thomas Petazzoni, CTO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Yegor Yefremov Jan. 30, 2023, 12:17 p.m. UTC | #4
Hi All,

On Wed, Dec 9, 2020 at 7:35 AM Valerio Crini <vcrini@gmail.com> wrote:
>
> Hi Yegor,
> Thanks for trying to fix the issue.
> Unfortunately I do not work for the company that gave me the opportunity to use the code that gave me the original error. So I am not able  to reproduce the issue anymore.
>
>
> Il mar 8 dic 2020, 16:36 Yegor Yefremov <yegorslists@googlemail.com> ha scritto:
>>
>> Hi all,
>>
>> On Sat, Aug 10, 2019 at 5:44 PM Martin Bark <martin@barkynet.com> wrote:
>> >
>> > Hi,
>> >
>> > On Sun, 4 Aug 2019 at 16:52, Thomas Petazzoni
>> > <thomas.petazzoni@bootlin.com> wrote:
>> > >
>> > > Hello Valerio,
>> > >
>> > > +Martin Bark in Cc.
>> > >
>> > > On Fri, 31 May 2019 09:58:00 +0200
>> > > Valerio Crini <vcrini@gmail.com> wrote:
>> > >
>> > > > From: Valerio Crini <valerio.crini@docomodigital.com>
>> > > >
>> > > > Some node modules as stf ship compiled binaries causing check-bin-arch script
>> > > > to return errors.
>> > > >
>> > > > Signed-off-by: Valerio Crini <valerio.crini@docomodigital.com>
>> > > > ---
>> > > >  support/scripts/check-bin-arch | 5 +++++
>> > > >  1 file changed, 5 insertions(+)
>> > > >
>> > > > diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
>> > > > index 3449bd1aeb..a276b75b92 100755
>> > > > --- a/support/scripts/check-bin-arch
>> > > > +++ b/support/scripts/check-bin-arch
>> > > > @@ -25,6 +25,11 @@ declare -a IGNORES=(
>> > > >       # it for a different architecture (e.g. i386 grub on x86_64).
>> > > >       "/lib/grub"
>> > > >       "/usr/lib/grub"
>> > > > +
>> > > > +     # Skip files in /usr/lib/node_modules, since it is possible to have it
>> > > > +     # for a different architecture (e.g. stf module has some packages already
>> > > > +     # compiled for several architectures).
>> > > > +     "/usr/lib/node_modules"
>> > >
>> > > Thanks for providing this patch. Could you give a bit more details
>> > > about which libraries get installed, what is their name, etc. ?
>> > >
>> > > Indeed, rather than ignoring files from /usr/lib/node_modules, it would
>> > > be a lot better to remove the libraries that are anyway not useful
>> > > because they don't match the CPU architecture of the target. This would
>> > > both reduce the filesystem size *and* solve the check-bin-arch issue.
>> >
>> > Yes i agree Thomas, this is the correct solution.  I would recommend
>> > you create a custom package for your application and in the BUILD_CMDS
>> > use $(NPM) install to install you node_modules followed by code to
>> > filter out any files that do not match your CPU architecture.  Here is
>> > a script i've used before to do this
>> >
>> > # Remove executables which are not the correct target arch
>> > for f in $(find $(@D) -executable -type f) ; do
>> >     a=$(${TARGET_READELF} -h $f 2>&1 | sed -r -e '/^  Machine:
>> > +(.+)/!d; s//\1/;')
>> >     if [ ! -z "$a" -a "$a" != "${BR2_READELF_ARCH_NAME}" ] ; then
>> >         echo "Remove: $f Arch: $a"
>> >         rm -f $f
>> >     fi
>> > done

I now also have two cases where I get precompiled Node.js module binaries:

- my own BR package
- when installing the modules via Node.js config option

Where should Martin's script reside? In a script before
support/scripts/check-bin-arch and handling only /usr/lib/node_modules
or inside support/scripts/check-bin-arch and handing all folders that
are not in the IGNORE list?

Best regards,
Yegor
diff mbox series

Patch

diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
index 3449bd1aeb..a276b75b92 100755
--- a/support/scripts/check-bin-arch
+++ b/support/scripts/check-bin-arch
@@ -25,6 +25,11 @@  declare -a IGNORES=(
 	# it for a different architecture (e.g. i386 grub on x86_64).
 	"/lib/grub"
 	"/usr/lib/grub"
+
+	# Skip files in /usr/lib/node_modules, since it is possible to have it
+	# for a different architecture (e.g. stf module has some packages already
+	# compiled for several architectures).
+	"/usr/lib/node_modules"
 )
 
 while getopts p:l:r:a:i: OPT ; do