Re: [Buildroot] [PATCH 1/1] packages/nodejs: install npm packages on host

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

Hello Tom,

On Wed, 19 Jan 2022 15:07:03 +0100
Tom Marcuzzi <tom.marcuzzi@orolia.com> wrote:

> Signed-off-by: Tom Marcuzzi <tom.marcuzzi@orolia.com>
> ---
>  package/nodejs/Config.in.host | 32 ++++++++++++++++++++++++++++++++
>  package/nodejs/nodejs.mk      | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+)

Thanks for your patch. Since the change is non-trivial, it would be
good to have some details in the commit log about the motivations for
the change. I suppose your build process at some point requires NodeJS
on the host ?

> diff --git a/package/nodejs/Config.in.host b/package/nodejs/Config.in.host
> index 4ceaf0c73e..601eb6e052 100644
> --- a/package/nodejs/Config.in.host
> +++ b/package/nodejs/Config.in.host
> @@ -16,3 +16,35 @@ config BR2_PACKAGE_HOST_NODEJS
>  comment "host nodejs needs a host gcc >= 8"
>  	depends on BR2_PACKAGE_HOST_QEMU_USER_ARCH_SUPPORTS
>  	depends on !BR2_HOST_GCC_AT_LEAST_8
> +
> +if BR2_PACKAGE_HOST_NODEJS
> +
> +	config BR2_PACKAGE_HOST_NODEJS_MODULES_ADDITIONAL
> +		string "Additional modules"

Please don't indent options, so remove the leading tab on all those
lines.

> +		help
> +		  List of space-separated nodejs modules to install via npm.

Perhaps clarify that they will be installed for the host NodeJS.

> +		  See https://npmjs.org/ to find modules and 'npm help install'
> +		  for available installation methods. For repeatable builds,
> +		  download and save tgz files or clone git repos for the
> +		  components you care about.
> +
> +		  Example:
> +		  serialport uglify-js@1.3.4 /my/module/mymodule.tgz \
> +		  	git://github.com/someuser/somemodule.git#v1.2
> +
> +		  This would install the serialport module (at the newest
> +		  version), the uglify-js module at 1.3.4, a module from a
> +		  filesystem path, and a module from a git repository.
> +
> +	config BR2_PACKAGE_HOST_NODEJS_MODULES_ADDITIONAL_DEPS
> +		string "Additional module dependencies"
> +		help
> +		  List of space-separated buildroot recipes which must be
> +		  built before your npms can be installed. For example, if in
> +		  'Additional modules' you specified 'node-curl' (see:
> +		  https://github.com/jiangmiao/node-curl), you could then
> +		  specify 'libcurl' here, to ensure that buildroot builds the
> +		  libcurl package, and does so before building your node
> +		  modules.

The libcurl example here is bad, because this
BR2_PACKAGE_HOST_NODEJS_MODULES_ADDITIONAL_DEPS option should only
contain host packages, and never target packages. So the example should
mention host-libcurl and not libcurl.

Also, this new option BR2_PACKAGE_HOST_NODEJS_MODULES_ADDITIONAL_DEPS
is not used anywhere. You forgot to change:

HOST_NODEJS_DEPENDENCIES = host-icu host-libopenssl host-python3 host-zlib

to:

HOST_NODEJS_DEPENDENCIES = host-icu host-libopenssl host-python3 host-zlib \
	$(call qstrip,(BR2_PACKAGE_HOST_NODEJS_MODULES_ADDITIONAL_DEPS))

Could you have a look at the above questions, so that we can resolve
the open points, and converge towards a solution that we can merge ?

Thanks a lot!

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