* [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available @ 2020-01-26 7:58 James Hilliard 2020-01-26 16:26 ` Yann E. MORIN 2020-01-29 17:28 ` Thomas Preston 0 siblings, 2 replies; 6+ messages in thread From: James Hilliard @ 2020-01-26 7:58 UTC (permalink / raw) To: buildroot Fixes: - http://autobuild.buildroot.net/results/1ef947553ec762dba6a6202b1cfc84ceed75dbb2/ Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- package/nodejs/nodejs.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk index e6eb73d576..abc868c364 100644 --- a/package/nodejs/nodejs.mk +++ b/package/nodejs/nodejs.mk @@ -65,7 +65,7 @@ define HOST_NODEJS_CONFIGURE_CMDS --shared-openssl-libpath=$(HOST_DIR)/lib \ --shared-zlib \ --no-cross-compiling \ - --with-intl=small-icu \ + --with-intl=$(if $(BR2_PACKAGE_ICU),system-icu,small-icu) \ ) endef @@ -80,6 +80,7 @@ define HOST_NODEJS_BUILD_CMDS $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ $(MAKE) -C $(@D) \ $(HOST_CONFIGURE_OPTS) \ + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ LDFLAGS.host="$(HOST_LDFLAGS)" \ NO_LOAD=cctest.target.mk \ PATH=$(@D)/bin:$(BR_PATH) @@ -89,6 +90,7 @@ define HOST_NODEJS_INSTALL_CMDS $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ $(MAKE) -C $(@D) install \ $(HOST_CONFIGURE_OPTS) \ + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ LDFLAGS.host="$(HOST_LDFLAGS)" \ NO_LOAD=cctest.target.mk \ PATH=$(@D)/bin:$(BR_PATH) -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available 2020-01-26 7:58 [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available James Hilliard @ 2020-01-26 16:26 ` Yann E. MORIN 2020-01-26 20:57 ` James Hilliard 2020-01-28 9:20 ` Thomas Petazzoni 2020-01-29 17:28 ` Thomas Preston 1 sibling, 2 replies; 6+ messages in thread From: Yann E. MORIN @ 2020-01-26 16:26 UTC (permalink / raw) To: buildroot James, All, On 2020-01-26 00:58 -0700, James Hilliard spake thusly: > Fixes: > - http://autobuild.buildroot.net/results/1ef947553ec762dba6a6202b1cfc84ceed75dbb2/ Pointing to an autobuild failure is nice, but you really need to provide a bit more context and explanations on what you are doing here... > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > package/nodejs/nodejs.mk | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk > index e6eb73d576..abc868c364 100644 > --- a/package/nodejs/nodejs.mk > +++ b/package/nodejs/nodejs.mk > @@ -65,7 +65,7 @@ define HOST_NODEJS_CONFIGURE_CMDS > --shared-openssl-libpath=$(HOST_DIR)/lib \ > --shared-zlib \ > --no-cross-compiling \ > - --with-intl=small-icu \ > + --with-intl=$(if $(BR2_PACKAGE_ICU),system-icu,small-icu) \ We already have a conditional block about icu, lines 35-40. > ) > endef > > @@ -80,6 +80,7 @@ define HOST_NODEJS_BUILD_CMDS > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > $(MAKE) -C $(@D) \ > $(HOST_CONFIGURE_OPTS) \ > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ Why do you need to account for a target package when building the host variant of nodejs? If this is needed, then explain it in the commit log. Then, the way we usually pass such options is to append to the existing CXXFLAGS. With the above comment, that would probably yield something like (e.g. for target, ditto for host): NODEJS_CXXFLAGS = $(TARGET_CXXFLAGS) ifeq ($(BR2_PACKAGE_ICU),y) NODEJS_DEPENDENCIES += icu NODEJS_CONF_OPTS += --with-intl=system-icu NODEJS_CXXFLAGS += -DU_DISABLE_RENAMING=1 else NODEJS_CONF_OPTS += --with-intl=small-icu endif [...] $(TARGET_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ $(MAKE) -C $(@D) \ $(TARGET_CONFIGURE_OPTS) \ CXXFLAGS="$(NODEJS_CXXFLAGS)" \ [...] Regards, Yann E. MORIN. > LDFLAGS.host="$(HOST_LDFLAGS)" \ > NO_LOAD=cctest.target.mk \ > PATH=$(@D)/bin:$(BR_PATH) > @@ -89,6 +90,7 @@ define HOST_NODEJS_INSTALL_CMDS > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > $(MAKE) -C $(@D) install \ > $(HOST_CONFIGURE_OPTS) \ > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ > LDFLAGS.host="$(HOST_LDFLAGS)" \ > NO_LOAD=cctest.target.mk \ > PATH=$(@D)/bin:$(BR_PATH) > -- > 2.20.1 > > _______________________________________________ > buildroot mailing list > buildroot at busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot -- .-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available 2020-01-26 16:26 ` Yann E. MORIN @ 2020-01-26 20:57 ` James Hilliard 2020-01-28 9:20 ` Thomas Petazzoni 1 sibling, 0 replies; 6+ messages in thread From: James Hilliard @ 2020-01-26 20:57 UTC (permalink / raw) To: buildroot On Sun, Jan 26, 2020 at 9:27 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > James, All, > > On 2020-01-26 00:58 -0700, James Hilliard spake thusly: > > Fixes: > > - http://autobuild.buildroot.net/results/1ef947553ec762dba6a6202b1cfc84ceed75dbb2/ > > Pointing to an autobuild failure is nice, but you really need to provide > a bit more context and explanations on what you are doing here... > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > package/nodejs/nodejs.mk | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk > > index e6eb73d576..abc868c364 100644 > > --- a/package/nodejs/nodejs.mk > > +++ b/package/nodejs/nodejs.mk > > @@ -65,7 +65,7 @@ define HOST_NODEJS_CONFIGURE_CMDS > > --shared-openssl-libpath=$(HOST_DIR)/lib \ > > --shared-zlib \ > > --no-cross-compiling \ > > - --with-intl=small-icu \ > > + --with-intl=$(if $(BR2_PACKAGE_ICU),system-icu,small-icu) \ > > We already have a conditional block about icu, lines 35-40. > > > ) > > endef > > > > @@ -80,6 +80,7 @@ define HOST_NODEJS_BUILD_CMDS > > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > > $(MAKE) -C $(@D) \ > > $(HOST_CONFIGURE_OPTS) \ > > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ > > Why do you need to account for a target package when building the host > variant of nodejs? If this is needed, then explain it in the commit log. I was doing that because the target icu package determines if host-icu is built, which breaks the small-icu host-nodejs build. > > Then, the way we usually pass such options is to append to the existing > CXXFLAGS. With the above comment, that would probably yield something > like (e.g. for target, ditto for host): I tried that approach first but it didn't seem to work seemingly due to the weird nodejs build system. > > NODEJS_CXXFLAGS = $(TARGET_CXXFLAGS) > > ifeq ($(BR2_PACKAGE_ICU),y) > NODEJS_DEPENDENCIES += icu > NODEJS_CONF_OPTS += --with-intl=system-icu > NODEJS_CXXFLAGS += -DU_DISABLE_RENAMING=1 > else > NODEJS_CONF_OPTS += --with-intl=small-icu > endif > > [...] > > $(TARGET_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > $(MAKE) -C $(@D) \ > $(TARGET_CONFIGURE_OPTS) \ > CXXFLAGS="$(NODEJS_CXXFLAGS)" \ > [...] > > Regards, > Yann E. MORIN. > > > LDFLAGS.host="$(HOST_LDFLAGS)" \ > > NO_LOAD=cctest.target.mk \ > > PATH=$(@D)/bin:$(BR_PATH) > > @@ -89,6 +90,7 @@ define HOST_NODEJS_INSTALL_CMDS > > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > > $(MAKE) -C $(@D) install \ > > $(HOST_CONFIGURE_OPTS) \ > > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ > > LDFLAGS.host="$(HOST_LDFLAGS)" \ > > NO_LOAD=cctest.target.mk \ > > PATH=$(@D)/bin:$(BR_PATH) > > -- > > 2.20.1 > > > > _______________________________________________ > > buildroot mailing list > > buildroot at busybox.net > > http://lists.busybox.net/mailman/listinfo/buildroot > > -- > .-----------------.--------------------.------------------.--------------------. > | 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. | > '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available 2020-01-26 16:26 ` Yann E. MORIN 2020-01-26 20:57 ` James Hilliard @ 2020-01-28 9:20 ` Thomas Petazzoni 2020-01-28 9:57 ` James Hilliard 1 sibling, 1 reply; 6+ messages in thread From: Thomas Petazzoni @ 2020-01-28 9:20 UTC (permalink / raw) To: buildroot On Sun, 26 Jan 2020 17:26:59 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > James, All, > > On 2020-01-26 00:58 -0700, James Hilliard spake thusly: > > Fixes: > > - http://autobuild.buildroot.net/results/1ef947553ec762dba6a6202b1cfc84ceed75dbb2/ > > Pointing to an autobuild failure is nice, but you really need to provide > a bit more context and explanations on what you are doing here... > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > package/nodejs/nodejs.mk | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk > > index e6eb73d576..abc868c364 100644 > > --- a/package/nodejs/nodejs.mk > > +++ b/package/nodejs/nodejs.mk > > @@ -65,7 +65,7 @@ define HOST_NODEJS_CONFIGURE_CMDS > > --shared-openssl-libpath=$(HOST_DIR)/lib \ > > --shared-zlib \ > > --no-cross-compiling \ > > - --with-intl=small-icu \ > > + --with-intl=$(if $(BR2_PACKAGE_ICU),system-icu,small-icu) \ > > We already have a conditional block about icu, lines 35-40. > > > ) > > endef > > > > @@ -80,6 +80,7 @@ define HOST_NODEJS_BUILD_CMDS > > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > > $(MAKE) -C $(@D) \ > > $(HOST_CONFIGURE_OPTS) \ > > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ > > Why do you need to account for a target package when building the host > variant of nodejs? If this is needed, then explain it in the commit log. > > Then, the way we usually pass such options is to append to the existing > CXXFLAGS. With the above comment, that would probably yield something > like (e.g. for target, ditto for host): > > NODEJS_CXXFLAGS = $(TARGET_CXXFLAGS) > > ifeq ($(BR2_PACKAGE_ICU),y) > NODEJS_DEPENDENCIES += icu > NODEJS_CONF_OPTS += --with-intl=system-icu > NODEJS_CXXFLAGS += -DU_DISABLE_RENAMING=1 > else > NODEJS_CONF_OPTS += --with-intl=small-icu > endif I think you're missing the fact that James is tweaking HOST_NODEJS_CONF_OPTS, depending on the value of BR2_PACKAGE_ICU, which looks very wrong. The issue is apparently that when host-icu has been built, the host-nodejs internal small-icu causes a build failure. However, while BR2_PACKAGE_ICU=y implies that host-icu will be built, host-icu can potentially be built even with BR2_PACKAGE_ICU is disabled. It is not the case today in Buildroot, because host-icu is only in the build dependencies of the target icu package, but relying on this assumption seems wrong. Best regards, Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available 2020-01-28 9:20 ` Thomas Petazzoni @ 2020-01-28 9:57 ` James Hilliard 0 siblings, 0 replies; 6+ messages in thread From: James Hilliard @ 2020-01-28 9:57 UTC (permalink / raw) To: buildroot On Tue, Jan 28, 2020 at 2:20 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Sun, 26 Jan 2020 17:26:59 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > James, All, > > > > On 2020-01-26 00:58 -0700, James Hilliard spake thusly: > > > Fixes: > > > - http://autobuild.buildroot.net/results/1ef947553ec762dba6a6202b1cfc84ceed75dbb2/ > > > > Pointing to an autobuild failure is nice, but you really need to provide > > a bit more context and explanations on what you are doing here... > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > --- > > > package/nodejs/nodejs.mk | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk > > > index e6eb73d576..abc868c364 100644 > > > --- a/package/nodejs/nodejs.mk > > > +++ b/package/nodejs/nodejs.mk > > > @@ -65,7 +65,7 @@ define HOST_NODEJS_CONFIGURE_CMDS > > > --shared-openssl-libpath=$(HOST_DIR)/lib \ > > > --shared-zlib \ > > > --no-cross-compiling \ > > > - --with-intl=small-icu \ > > > + --with-intl=$(if $(BR2_PACKAGE_ICU),system-icu,small-icu) \ > > > > We already have a conditional block about icu, lines 35-40. > > > > > ) > > > endef > > > > > > @@ -80,6 +80,7 @@ define HOST_NODEJS_BUILD_CMDS > > > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > > > $(MAKE) -C $(@D) \ > > > $(HOST_CONFIGURE_OPTS) \ > > > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ > > > > Why do you need to account for a target package when building the host > > variant of nodejs? If this is needed, then explain it in the commit log. > > > > Then, the way we usually pass such options is to append to the existing > > CXXFLAGS. With the above comment, that would probably yield something > > like (e.g. for target, ditto for host): > > > > NODEJS_CXXFLAGS = $(TARGET_CXXFLAGS) > > > > ifeq ($(BR2_PACKAGE_ICU),y) > > NODEJS_DEPENDENCIES += icu > > NODEJS_CONF_OPTS += --with-intl=system-icu > > NODEJS_CXXFLAGS += -DU_DISABLE_RENAMING=1 > > else > > NODEJS_CONF_OPTS += --with-intl=small-icu > > endif > > I think you're missing the fact that James is tweaking > HOST_NODEJS_CONF_OPTS, depending on the value of BR2_PACKAGE_ICU, which > looks very wrong. > > The issue is apparently that when host-icu has been built, the > host-nodejs internal small-icu causes a build failure. However, while > BR2_PACKAGE_ICU=y implies that host-icu will be built, host-icu can > potentially be built even with BR2_PACKAGE_ICU is disabled. It is not > the case today in Buildroot, because host-icu is only in the build > dependencies of the target icu package, but relying on this assumption > seems wrong. Is there a way to make it depend on host-icu directly? > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available 2020-01-26 7:58 [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available James Hilliard 2020-01-26 16:26 ` Yann E. MORIN @ 2020-01-29 17:28 ` Thomas Preston 1 sibling, 0 replies; 6+ messages in thread From: Thomas Preston @ 2020-01-29 17:28 UTC (permalink / raw) To: buildroot On 26/01/2020 07:58, James Hilliard wrote: > Fixes: > - http://autobuild.buildroot.net/results/1ef947553ec762dba6a6202b1cfc84ceed75dbb2/ > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > package/nodejs/nodejs.mk | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk > index e6eb73d576..abc868c364 100644 > --- a/package/nodejs/nodejs.mk > +++ b/package/nodejs/nodejs.mk > @@ -65,7 +65,7 @@ define HOST_NODEJS_CONFIGURE_CMDS > --shared-openssl-libpath=$(HOST_DIR)/lib \ > --shared-zlib \ > --no-cross-compiling \ > - --with-intl=small-icu \ > + --with-intl=$(if $(BR2_PACKAGE_ICU),system-icu,small-icu) \ > ) > endef > > @@ -80,6 +80,7 @@ define HOST_NODEJS_BUILD_CMDS > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > $(MAKE) -C $(@D) \ > $(HOST_CONFIGURE_OPTS) \ > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ > LDFLAGS.host="$(HOST_LDFLAGS)" \ > NO_LOAD=cctest.target.mk \ > PATH=$(@D)/bin:$(BR_PATH) > @@ -89,6 +90,7 @@ define HOST_NODEJS_INSTALL_CMDS > $(HOST_MAKE_ENV) PYTHON=$(HOST_DIR)/bin/python2 \ > $(MAKE) -C $(@D) install \ > $(HOST_CONFIGURE_OPTS) \ > + $(if $(BR2_PACKAGE_ICU),CXXFLAGS.target="-DU_DISABLE_RENAMING=1") \ > LDFLAGS.host="$(HOST_LDFLAGS)" \ > NO_LOAD=cctest.target.mk \ > PATH=$(@D)/bin:$(BR_PATH) > I was actually able to fix this by removing `--shared-zlib`, see below. commit 8ddaef98e72449c41a3255374a5f61da20a167df Author: Thomas Preston <thomas.preston@codethink.co.uk> Date: Fri Jan 24 13:40:49 2020 +0000 package/nodejs: Remove --shared-zlib configure arg The nodejs configure.py file orders zlib headers before the bundled ICU headers. The zlib headers happen to be located in the system include directory, next to some system ICU headers (not bundled). If these are built before nodejs is, nodejs will get confused and try to use the system ICU headers instead of the bundled ones. Fix this by removing the --shared-zlib configure argument, since we search in the system include directory after bundled ICU headers anyway. Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk> diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk index e6eb73d576..205e8a8bd5 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 \ ) ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-29 17:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-26 7:58 [Buildroot] [PATCH 1/1] package/nodejs: use system-icu for host-nodejs when available James Hilliard 2020-01-26 16:26 ` Yann E. MORIN 2020-01-26 20:57 ` James Hilliard 2020-01-28 9:20 ` Thomas Petazzoni 2020-01-28 9:57 ` James Hilliard 2020-01-29 17:28 ` Thomas Preston
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.