Re: [Buildroot] [PATCH v3 2/2] package/nodejs: bump version to 14.17.6

[Thomas Petazzoni <thomas.petazzoni@bootlin.com>]

Hello,

On Fri,  8 Oct 2021 13:43:42 -0700
Adam Duskett <aduskett@gmail.com> wrote:

> Changes include:
>   - Remove the dependency on Python2, as nodejs 14 supports Python 3.
>   - Remove --without-snapshot as it's no longer a supported config option.
>   - Remove /openssl to the shared-openssl-includes config option, as the build
>     system automatically appends /openssl to the includes path.
> 
>   - Add a qemu wrapper. V8's JIT infrastructure requires binaries such as
>     mksnapshot and mkpeephole to be run in the host during the build.
>     However, these binaries must have the same bit-width as the target
>     (e.g. a x86_64 host targeting ARMv6 needs to produce a 32-bit binary).
>     To work around this issue, cross-compile the binaries for the target and
>     run them on the host with QEMU, much like gobject-introspection.
> 
> Signed-off-by: Adam Duskett <aduskett@gmail.com>
> ---
>  .../0001-add-qemu-wrapper-support.patch       | 76 ++++++++++++++++++
>  package/nodejs/Config.in                      | 15 +++-
>  package/nodejs/nodejs.hash                    |  6 +-
>  package/nodejs/nodejs.mk                      | 78 +++++++++++--------
>  package/nodejs/v8-qemu-wrapper.in             | 12 +++
>  5 files changed, 146 insertions(+), 41 deletions(-)
>  create mode 100644 package/nodejs/0001-add-qemu-wrapper-support.patch
>  create mode 100644 package/nodejs/v8-qemu-wrapper.in
> 
> diff --git a/package/nodejs/0001-add-qemu-wrapper-support.patch b/package/nodejs/0001-add-qemu-wrapper-support.patch
> new file mode 100644
> index 0000000000..9b81e3adab
> --- /dev/null
> +++ b/package/nodejs/0001-add-qemu-wrapper-support.patch
> @@ -0,0 +1,76 @@
> +From fa09fa3ad6a21ae0b35fb860f76d1762e5f29972 Mon Sep 17 00:00:00 2001
> +From: Adam Duskett <aduskett@gmail.com>
> +Date: Mon, 27 Sep 2021 12:55:09 -0700
> +Subject: [PATCH] 1

This is perhaps a bit weak as a commit title and commit description :-)


> +# V8's JIT infrastructure requires binaries such as mksnapshot and
> +# mkpeephole to be run in the host during the build. However, these
> +# binaries must have the same bit-width as the target (e.g. a x86_64
> +# host targeting ARMv6 needs to produce a 32-bit binary). To work around this
> +# issue, cross-compile the binaries for the target and run them on the
> +# host with QEMU, much like gobject-introspection.
> +define NODEJS_INSTALL_V8_QEMU_WRAPPER
> +	$(INSTALL) -D -m 755 $(NODEJS_PKGDIR)/v8-qemu-wrapper.in \
> +		$(@D)/out/Release/v8-qemu-wrapper
> +	$(SED) "s%@QEMU_USER@%$(QEMU_USER)%g" \
> +		$(@D)/out/Release/v8-qemu-wrapper
> +	$(SED) "s%@TOOLCHAIN_HEADERS_VERSION@%$(BR2_TOOLCHAIN_HEADERS_AT_LEAST)%g" \
> +		$(@D)/out/Release/v8-qemu-wrapper
> +	$(SED) "s%@QEMU_USERMODE_ARGS@%$(call qstrip,$(BR2_PACKAGE_HOST_QEMU_USER_MODE_ARGS))%g" \
> +		$(@D)/out/Release/v8-qemu-wrapper
> +endef
> +NODEJS_PRE_CONFIGURE_HOOKS += NODEJS_INSTALL_V8_QEMU_WRAPPER
>  
> +define NODEJS_WRAPPER_FIXUP
> +	$(SED) "s%@MAYBE_WRAPPER@%'<(PRODUCT_DIR)/v8-qemu-wrapper',%g" $(@D)/node.gyp
> +	$(SED) "s%@MAYBE_WRAPPER@%'<(PRODUCT_DIR)/v8-qemu-wrapper',%g" $(@D)/tools/v8_gypfiles/v8.gyp
> +endef
> +NODEJS_PRE_CONFIGURE_HOOKS += NODEJS_WRAPPER_FIXUP
>
> +# Do not run the qemu-wrapper for the host build.
> +define HOST_NODEJS_WRAPPER_FIXUP
> +	$(SED) "s%@MAYBE_WRAPPER@%%g" $(@D)/node.gyp
> +	$(SED) "s%@MAYBE_WRAPPER@%%g" $(@D)/tools/v8_gypfiles/v8.gyp
> +endef
> +HOST_NODEJS_PRE_CONFIGURE_HOOKS += HOST_NODEJS_WRAPPER_FIXUP

Would have been great for MAYBE_WRAPPER to be a ./configure option :-)
But fair enough.

Overall, except the commit title/log of the patch, I don't see much
problem with this patch.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot