| Message ID | 20200121023242.94475-1-james.hilliard1@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] package/nodejs: bump to version 12.14.1 | expand |
>>>>> "James" == James Hilliard <james.hilliard1@gmail.com> writes: > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> Committed, thanks.
Hi all, On 21/01/2020 02:32, James Hilliard wrote: > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > package/nodejs/nodejs.hash | 4 ++-- > package/nodejs/nodejs.mk | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/package/nodejs/nodejs.hash b/package/nodejs/nodejs.hash > index bde0ac0167..92369105ff 100644 > --- a/package/nodejs/nodejs.hash > +++ b/package/nodejs/nodejs.hash > @@ -1,5 +1,5 @@ > -# From https://nodejs.org/dist/v12.14.0/SHASUMS256.txt > -sha256 088a217ba2af641b8cc15be29f6e2956b8a33e6badb85596bbc2cdea9df9be71 node-v12.14.0.tar.xz > +# From https://nodejs.org/dist/v12.14.1/SHASUMS256.txt > +sha256 877b4b842318b0e09bc754faf7343f2f097f0fc4f88ab9ae57cf9944e88e7adb node-v12.14.1.tar.xz > > # Hash for license file > sha256 950bbc741dc021489c47683e34e7637e9b96fb4a1f430b2f77a744130516e293 LICENSE > diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk > index 62c4c1abb1..e6eb73d576 100644 > --- a/package/nodejs/nodejs.mk > +++ b/package/nodejs/nodejs.mk > @@ -4,7 +4,7 @@ > # > ################################################################################ > > -NODEJS_VERSION = 12.14.0 > +NODEJS_VERSION = 12.14.1 > NODEJS_SOURCE = node-v$(NODEJS_VERSION).tar.xz > NODEJS_SITE = http://nodejs.org/dist/v$(NODEJS_VERSION) > NODEJS_DEPENDENCIES = host-python host-nodejs c-ares \ > This breaks host-nodejs for me. Looks like an error with icu-small conflicting with host-icu, but I can't tell for sure. It works again when I revert this patch (nodejs 12.14.0) and clean-rebuild. I can reproduce with something similar to this: docker build -t "repro" support/docker docker run \ --mount=type=bind,src="$(pwd)",dst=/home/br-user \ --user "$UID:$UID" -it "repro" \ make host-nodejs The error looks like this: make[2]: *** [/home/br-user/output/build/host-nodejs-12.14.1/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/number_multiplier.o] Error 1 In file included from ../deps/icu-small/source/i18n/number_types.h:11:0, from ../deps/icu-small/source/i18n/number_decimalquantity.h:14, from ../deps/icu-small/source/i18n/numparse_types.h:11, from ../deps/icu-small/source/i18n/number_currencysymbols.cpp:12: ../deps/icu-small/source/i18n/unicode/decimfmt.h:899:13: error: invalid covariant return type for ‘virtual icu_65::Format* icu_65::DecimalFormat::clone() const’ Format* clone(void) const U_OVERRIDE; ^~~~~ In file included from ../deps/icu-small/source/i18n/unicode/decimfmt.h:39:0, from ../deps/icu-small/source/i18n/number_types.h:11, from ../deps/icu-small/source/i18n/number_decimalquantity.h:14, from ../deps/icu-small/source/i18n/numparse_types.h:11, from ../deps/icu-small/source/i18n/number_currencysymbols.cpp:12: /home/br-user/output/host/include/unicode/numfmt.h:271:27: error: overriding ‘virtual icu_65::NumberFormat* icu_65::NumberFormat::clone() const’ virtual NumberFormat* clone() const = 0; ^~~~~
I thought that issue was caused by the ICU 65.1 bump, not the nodejs bump. I have a pending pull request to nodejs that might fix this issue by the way https://github.com/nodejs/node/pull/31433. On Thu, Jan 23, 2020 at 6:46 AM Thomas Preston <thomas.preston@codethink.co.uk> wrote: > > Hi all, > > On 21/01/2020 02:32, James Hilliard wrote: > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > package/nodejs/nodejs.hash | 4 ++-- > > package/nodejs/nodejs.mk | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/package/nodejs/nodejs.hash b/package/nodejs/nodejs.hash > > index bde0ac0167..92369105ff 100644 > > --- a/package/nodejs/nodejs.hash > > +++ b/package/nodejs/nodejs.hash > > @@ -1,5 +1,5 @@ > > -# From https://nodejs.org/dist/v12.14.0/SHASUMS256.txt > > -sha256 088a217ba2af641b8cc15be29f6e2956b8a33e6badb85596bbc2cdea9df9be71 node-v12.14.0.tar.xz > > +# From https://nodejs.org/dist/v12.14.1/SHASUMS256.txt > > +sha256 877b4b842318b0e09bc754faf7343f2f097f0fc4f88ab9ae57cf9944e88e7adb node-v12.14.1.tar.xz > > > > # Hash for license file > > sha256 950bbc741dc021489c47683e34e7637e9b96fb4a1f430b2f77a744130516e293 LICENSE > > diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk > > index 62c4c1abb1..e6eb73d576 100644 > > --- a/package/nodejs/nodejs.mk > > +++ b/package/nodejs/nodejs.mk > > @@ -4,7 +4,7 @@ > > # > > ################################################################################ > > > > -NODEJS_VERSION = 12.14.0 > > +NODEJS_VERSION = 12.14.1 > > NODEJS_SOURCE = node-v$(NODEJS_VERSION).tar.xz > > NODEJS_SITE = http://nodejs.org/dist/v$(NODEJS_VERSION) > > NODEJS_DEPENDENCIES = host-python host-nodejs c-ares \ > > > > This breaks host-nodejs for me. Looks like an error with icu-small conflicting > with host-icu, but I can't tell for sure. It works again when I revert this > patch (nodejs 12.14.0) and clean-rebuild. > > I can reproduce with something similar to this: > > docker build -t "repro" support/docker > docker run \ > --mount=type=bind,src="$(pwd)",dst=/home/br-user \ > --user "$UID:$UID" -it "repro" \ > make host-nodejs > > The error looks like this: > > make[2]: *** [/home/br-user/output/build/host-nodejs-12.14.1/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/number_multiplier.o] Error 1 > In file included from ../deps/icu-small/source/i18n/number_types.h:11:0, > from ../deps/icu-small/source/i18n/number_decimalquantity.h:14, > from ../deps/icu-small/source/i18n/numparse_types.h:11, > from ../deps/icu-small/source/i18n/number_currencysymbols.cpp:12: > ../deps/icu-small/source/i18n/unicode/decimfmt.h:899:13: error: invalid covariant return type for ‘virtual icu_65::Format* icu_65::DecimalFormat::clone() const’ > Format* clone(void) const U_OVERRIDE; > ^~~~~ > In file included from ../deps/icu-small/source/i18n/unicode/decimfmt.h:39:0, > from ../deps/icu-small/source/i18n/number_types.h:11, > from ../deps/icu-small/source/i18n/number_decimalquantity.h:14, > from ../deps/icu-small/source/i18n/numparse_types.h:11, > from ../deps/icu-small/source/i18n/number_currencysymbols.cpp:12: > /home/br-user/output/host/include/unicode/numfmt.h:271:27: error: overriding ‘virtual icu_65::NumberFormat* icu_65::NumberFormat::clone() const’ > virtual NumberFormat* clone() const = 0; > ^~~~~
Hey, Thanks for getting back to me. On 23/01/2020 21:03, James Hilliard wrote: > I thought that issue was caused by the ICU 65.1 bump, not the nodejs bump. > > I have a pending pull request to nodejs that might fix this issue by > the way https://github.com/nodejs/node/pull/31433. > It looks more like a header conflict to me - nodejs should be using the bundled icu-small but buildroot makes it use the host-icu package headers. Maybe the discrepancy is because of the ICU 65.1 bump though... Unfortunately I'm unable to reliably reproduce it with different combinations of the the following conditions: - Upstream - Reverted ae1efb62e7 package/nodejs: bump to version 12.14.1 - Reverted e2a2fab11b package/icu: bump to version 65-1 But I still see it sometimes... >> In file included from ../deps/icu-small/source/i18n/unicode/decimfmt.h:39:0, >> from ../deps/icu-small/source/i18n/number_types.h:11, >> from ../deps/icu-small/source/i18n/number_decimalquantity.h:14, >> from ../deps/icu-small/source/i18n/numparse_types.h:11, >> from ../deps/icu-small/source/i18n/number_currencysymbols.cpp:12: >> /home/br-user/output/host/include/unicode/numfmt.h:271:27: error: overriding ‘virtual icu_65::NumberFormat* icu_65::NumberFormat::clone() const’ >> virtual NumberFormat* clone() const = 0; >> ^~~~~ > All the headers are from icu-small except unicode/numfmt.h, which is from buildroot output/host. This is one of the build commands: /home/br-user/output/host/bin/ccache /usr/bin/g++ \ -o /home/br-user/output/build/host-nodejs-12.14.0/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/zonemeta.o ../deps/icu-small/source/i18n/zonemeta.cpp '-DV8_DEPRECATION_WARNINGS' '-DV8_IMMINENT_DEPRECATION_WARNINGS' '-DU_I18N_IMPLEMENTATION=1' '-DU_ATTRIBUTE_DEPRECATED=' '-D_CRT_SECURE_NO_DEPRECATE=' '-DU_STATIC_IMPLEMENTATION=1' '-DUCONFIG_NO_SERVICE=1' '-DU_ENABLE_DYLOAD=0' '-DU_HAVE_STD_STRING=1' '-DUCONFIG_NO_BREAK_ITERATION=0' \ -I/home/br-user/output/host/include \ -I/home/br-user/output/host/include/openssl \ -I../deps/icu-small/source/i18n \ -I../deps/icu-small/source/common \ -pthread \ -Wall \ -Wextra \ -Wno-unused-parameter \ -m32 \ -Wno-deprecated-declarations \ -Wno-strict-aliasing \ -O3 \ -fno-omit-frame-pointer \ -fno-exceptions \ -std=gnu++1y \ -frtti-MMD \ -MF /home/br-user/output/build/host-nodejs-12.14.0/out/Release/.deps//home/br-user/output/build/host-nodejs-12.14.0/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/zonemeta.o.d.raw \ -I/home/br-user/output/host/include \ -O2 \ -I/home/br-user/output/host/include \ -c There's a few things fishy; 1. Three instances of `-I/home/br-user/output/host/include` 2. The -MF argument is wrong, looks like an absolute path got copied somewhere it shouldn't have. What we want is for `-I../deps/icu-small/source/i18n` to take priority so that the bundled icu-small is used instead of the host-icu package headers.
On 24/01/2020 11:31, Thomas Preston wrote: > There's a few things fishy; > 1. Three instances of `-I/home/br-user/output/host/include` The first `-I$(HOST_DIR)/include` is because nodejs configure.py uses buildroot's host-pkg-config script to get the include directories for zlib, which happens to be /home/br-user/output/host/include - which also includes host-icu headers. The final two `-I$(HOST_DIR)/include` are from: CFLAGS.target CXXFLAGS.target It's really an ordering issue, because nodejs configure.py looks for ICU headers in `-I$(HOST_DIR)/include` before the bundled icu-small directory. One workaround is to remove "--shared-zlib" so that `-I$(HOST_DIR)/include` isn't listed before the icu-small arguments -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common Zlib is still found because it's in the host system include dir, although I'm not sure how acceptable this solution is to upstream Buildroot. Maybe remove it but add a comment? diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk index 62c4c1abb1..181668b743 100644 --- a/package/nodejs/nodejs.mk +++ b/package/nodejs/nodejs.mk @@ -63,7 +63,6 @@ define HOST_NODEJS_CONFIGURE_CMDS --shared-openssl \ --shared-openssl-includes=$(HOST_DIR)/include/openssl \ --shared-openssl-libpath=$(HOST_DIR)/lib \ - --shared-zlib \ --no-cross-compiling \ --with-intl=small-icu \ )
diff --git a/package/nodejs/nodejs.hash b/package/nodejs/nodejs.hash index bde0ac0167..92369105ff 100644 --- a/package/nodejs/nodejs.hash +++ b/package/nodejs/nodejs.hash @@ -1,5 +1,5 @@ -# From https://nodejs.org/dist/v12.14.0/SHASUMS256.txt -sha256 088a217ba2af641b8cc15be29f6e2956b8a33e6badb85596bbc2cdea9df9be71 node-v12.14.0.tar.xz +# From https://nodejs.org/dist/v12.14.1/SHASUMS256.txt +sha256 877b4b842318b0e09bc754faf7343f2f097f0fc4f88ab9ae57cf9944e88e7adb node-v12.14.1.tar.xz # Hash for license file sha256 950bbc741dc021489c47683e34e7637e9b96fb4a1f430b2f77a744130516e293 LICENSE diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk index 62c4c1abb1..e6eb73d576 100644 --- a/package/nodejs/nodejs.mk +++ b/package/nodejs/nodejs.mk @@ -4,7 +4,7 @@ # ################################################################################ -NODEJS_VERSION = 12.14.0 +NODEJS_VERSION = 12.14.1 NODEJS_SOURCE = node-v$(NODEJS_VERSION).tar.xz NODEJS_SITE = http://nodejs.org/dist/v$(NODEJS_VERSION) NODEJS_DEPENDENCIES = host-python host-nodejs c-ares \
Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- package/nodejs/nodejs.hash | 4 ++-- package/nodejs/nodejs.mk | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)