Re: [Buildroot] [PATCH] package/nodejs: fix parallel build

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Jens, All,

+Thomas for the qemu part...

On 2023-09-23 11:39 +0200, Jens Maus via buildroot spake thusly:
> In ninja-based nodejs builds performing parallel builds using "make -jX"
> is not working during buildroot initiated builds because the JOBS
> variable is set in the MAKE_OPTS variables in nodejs.mk. This commit
> remedies the issue by setting JOBS to BR2_JLEVEL and also by fixing the
> top-level Makefile of nodejs due to an incorrect tab-based indentation
> being used. Furthermore, it is important to also set LD_LIBRARY_PATH in
> v8-qemu-wrapper.in to our staging libdirs as otherwise execution of
> build tools fail due to host OS libraries being preferred.

That are two different changes, so they should be two different patches.

The commit log is thus misleading, becausee you are not "just" fixing
the parallel build issue.

See below for more comments.

> Signed-off-by: Jens Maus <mail@jens-maus.de>
> ---
>  ...-fixing-Makefile-for-parallel-builds.patch | 37 +++++++++++++++++++
>  package/nodejs/nodejs.mk                      |  6 ++-
>  package/nodejs/v8-qemu-wrapper.in             |  1 +
>  3 files changed, 42 insertions(+), 2 deletions(-)
>  create mode 100644 package/nodejs/0005-fixing-Makefile-for-parallel-builds.patch
> 
> diff --git a/package/nodejs/0005-fixing-Makefile-for-parallel-builds.patch b/package/nodejs/0005-fixing-Makefile-for-parallel-builds.patch
> new file mode 100644
> index 0000000000..73db129979
> --- /dev/null
> +++ b/package/nodejs/0005-fixing-Makefile-for-parallel-builds.patch
> @@ -0,0 +1,37 @@
> +From 1c4582c7dd7c80fbac89031d8fa8cf418249d01e Mon Sep 17 00:00:00 2001
> +From: Jens Maus <mail@jens-maus.de>
> +Date: Sat, 23 Sep 2023 11:09:50 +0200
> +Subject: [PATCH] fixing Makefile for parallel builds
> +
> +This change fixing incorrect GNU makefile indentation in the top-level
> +Makefile which causes issues with parallel "make -jXX" builds.
> +
> +Upstream: N/A (specific to buildroot builds)

If the indentation is causing issues, then that's surely a fix that
*must* be sent upstream.

Also, please explain what the actual error is.

[--SNIP--]
> diff --git a/package/nodejs/v8-qemu-wrapper.in b/package/nodejs/v8-qemu-wrapper.in
> index e1083f47f7..48222d089b 100644
> --- a/package/nodejs/v8-qemu-wrapper.in
> +++ b/package/nodejs/v8-qemu-wrapper.in
> @@ -5,5 +5,6 @@
>  exec @QEMU_USER@ -r @TOOLCHAIN_HEADERS_VERSION@ \
>      @QEMU_USERMODE_ARGS@ \
>     -L "${STAGING_DIR}/" \
> +   -E LD_LIBRARY_PATH="${STAGING_DIR}/lib:${STAGING_DIR}/usr/lib/" \
>      "$@"

We've already had hints about doing something like that, but this is
going to require a much more detailed commit log than you provided.
Ah, I see you were part of:

    https://bugs.busybox.net/show_bug.cgi?id=14366

Also, Thomas did quite some in-depth analysis of the qemu issue (you
even were in Cc of that mail):

    https://lore.kernel.org/buildroot/20221031213926.50d3c778@windsurf/

As you see, just stating "it is important to also set LD_LIBRARY_PATH in
v8-qemu-wrapper.in to our staging libdirs as otherwise execution of
build tools fail due to host OS libraries being preferred" is not
enough. We really need an actual explanation.

So, please, canyou split this patch into two:
  - fix the parallel build issue,
  - fix the qemu wrapper with a synthesis of Thomas' extensive analysis

And in that second patch, please add a reference to the bug you close.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot