Node: update error & fix compile error #3751

ianchi · 2017-01-04T02:03:37Z

Compile tested: ipq806x / EA8500
Run tested: ipq806x / EA8500
Description: update version and fix compile error.

hnyman · 2017-01-04T15:38:33Z

@ianchi @nxhack
The node: update to v4.5.0 PR #3179 from @nxhack has been open for months and the current node maintainer @blogic has not reacted to it. One of you might need to grab the maintainership of node as you are currently interested in it.

ianchi · 2017-01-04T22:12:15Z

I have no problem grabbing it. I also add @jow- as we were discussing this package as a possible host dependency for LuCI-NG.
I think we should go directly to v6 as it is the current LTS and v4 will enter maintenance in a few months.

nxhack · 2017-01-05T04:57:48Z

@ianchi Thank you. I agree go to v6.x.

diizzyy · 2017-01-07T11:23:13Z

https://github.com/nxhack/openwrt-node-packages/blob/master/node/Makefile is a no go?

ianchi · 2017-01-07T23:29:55Z

I think that both PR need to be integrated.
I'll resend it rebased on top of that.

nxhack · 2017-01-10T06:32:26Z

@diizzyy
It try to new v7.x branch. but in current openwrt-packages, it need update icu package version to v58.2. So I sent PR. : #3703

This Makefile is based on @artynet edition : https://github.com/artynet/openwrt-git/tree/openwrt-1505-setup-03/package/linino/node7

ianchi · 2017-02-27T03:57:29Z

@nxhack, your ICU package is already integrated and now node is compiling OK.
Are your planning on sending a PR for your branch, which has options for v4, v6 an v7? I think it is the most complete.
If so I'll close this PR.

My only doubt is why are you making node dependent on node/host itself? I was able to build it without this dependency
PKG_BUILD_DEPENDS:=python/host node/host

ianchi · 2017-02-27T04:31:59Z

Another comment, ICU package is huge (11mb final ipk), I think the default option should be with no i18n.

nxhack · 2017-02-27T05:40:03Z

@ianchi

My version-selectable package has following problems. Packages are not built except for selected version. Therefore, the package user can not use the desired version of the package.

The reason that "node/host" is in "PKG_BUILD_DEPENDS" is need for building v7.x.

"--with-intl" is preferably "none" by default. Because it is important that the embedded system has a small footprint.

nxhack · 2017-02-27T05:45:04Z

And host-build is better "--with-intl=none".

nxhack · 2017-02-27T06:09:50Z

For small footprints, it may be better to have "v4.x" too.

ianchi · 2017-02-27T14:42:52Z

Hi @nxhack, some comments:

The reason that "node/host" is in "PKG_BUILD_DEPENDS" is need for building v7.x.
How come a package depends on itself for building?

I agree with "--with-intl" set to "none" by default.

Regarding version selection, perhaps it is best to define two variants of the package: leave "node" as the latest LTS version (v6 at this time), and add another "node_edge" (or something similar) for the latest version (v7 at this moment).

nxhack · 2017-02-28T00:12:38Z

@ianchi

How come a package depends on itself for building?

Let's see : nodejs/node#9707

versions

It might be better to create a simple version selection mechanism like 'n'.
But now I think it is better to upgrade v6.x LTS in this package tree.

nxhack · 2017-02-28T02:46:10Z

5353003 bytes node_v4.7.3-1_mips_24kc.ipk
6075460 bytes node-without-i18n_v6.9.5-1_mips_24kc.ipk
6100069 bytes node-with-i18n_v6.9.5-1_mips_24kc.ipk
6293501 bytes node-without-i18n_v7.5.0-1_mips_24kc.ipk
6533991 bytes node-with-i18n_v7.5.0-1_mips_24kc.ipk
12796440 bytes icu_58.2-5_mips_24kc.ipk

ianchi · 2017-02-28T05:07:51Z

Weird issue regarding host dependency, but it is a good workaround.

Regarding versions, ICU must surely be left out of the default builds.

Having a separate package per version might be a good option, but it can't be sustained with each major version changing once a year.
Node takes a lot of time and space to compile, I'm not sure if it is really worth to have that many versions.
At most I would keep two, LTS and edge.

nxhack · 2017-02-28T05:27:54Z

At most I would keep two, LTS and edge.

Roger.

And there was an update 6 days ago.
https://nodejs.org/en/download/
v6.10.0
v7.6.0

nxhack · 2017-03-07T06:31:57Z

lang/node/Makefile

+ifneq ($(findstring neon,$(CPU_SUBTYPE)),)
+CONFIGURE_ARGS+= --with-arm-fpu=neon
+endif
+


Sorry I made mistake. it was 'CONFIG_CPU_TYPE', not 'CPU_SUBTYPE.'

To prevent matching to unexpected strings, only use 'neon' and 'vfpv3'
('vfp' is default value)

hauke

I would suggest to only take the version 6 or 7 or even more recent and do not care about th older versions. Please also rebase this pull request.

hauke · 2017-06-05T15:49:44Z

lang/node/Makefile

+
+	config NODEJS_ICU
+		bool "enable i18n features"
+		default y


Please make this default to n

ianchi · 2017-06-08T05:05:02Z

Rebased and bumped to 6.11, which is the current LTS.
I leave an upgrade to V8.0.0 for a new PR, as it requires some workarrounds to compile (the same as v7). As explained by @nxhack

Anyway, I think we should at least finally move to v6.

ianchi · 2017-07-12T01:53:15Z

Hi any comments on this?
This PR added to its predecessors have been waiting for over a year.
I think this should be merged soon, Node is already in V8 and switching LTS to it soon.

ianchi · 2017-08-16T02:59:53Z

Hi @blogic, any comments to merge this so to move Node to V6?

artynet · 2017-08-16T04:27:21Z

Hello @ianchi,

please don't hesitate to ping me if you ever need help for the node v8.x.x release series. I will test in a few hours the build for ar71xx and x86 systems.

Best, Arturo

hnyman · 2017-08-16T05:23:53Z

@ianchi
blogic has not been very active regarding node packages for a while, so you should maybe add yourself as the second maintainer for the package.

artynet · 2017-08-16T19:38:14Z

@iachi @nxhack please take a look at my new Makefile for node v8.4.0 build :

https://github.com/artynet/LEDE/blob/linino-32/package/linino/node8/Makefile

and instead of HOST build I would suggest the download of a binary tarball as done starting from here :

https://github.com/artynet/LEDE/blob/linino-32/package/linino/node/Makefile#L57

please let me know your opinions about it. It would save a lot of build time and cpu load every time....

Bump version to 6.11.0 Add ICU dependency when using i18n Tweak fpu configs Signed-off-by: Adrian Panella <[email protected]>

Signed-off-by: Adrian Panella <[email protected]>

ianchi · 2017-08-17T06:18:38Z

@hnyman I added myself as maintainer. (and updated minor version in the process)

@artynet, as I said before, I'd like to finally get this merged and Node updated to v6 (current LTS).
After that we can start a new PR discussing the best approach to go to v8.
Otherwise I fear we can get stuck another year in this thread.

PD: @hnyman if you are involved with Travis CI checks, please note that initially it was marked as passed even though it was failing when applying patches. (as I missed pushing a commit)

neocturne · 2017-08-17T13:21:35Z

@artynet IMO, use of binary distributions instead of doing a host build should be avoided. In particular, it would limit the usable host platforms.

jow- · 2017-08-17T13:23:20Z

I agree that bootstrapping the host build from source is preferable but for local development it would be great to provide the possibility to rely on binary distribution.

Maybe a menuconfig item could be provided, possibly defaulting on CONFIG_DEVEL

ianchi · 2017-08-17T13:42:11Z

This could be complemented with a fallback to a full host build dependency if the host machine arch is not listed in the distributed ones, so that no platform is left out. Some will simply not benefit from the performance gain.
But it should be done in a more general way as the one proposed, as here only Node's host dependency to build itself is tackled, but if other packages have host dependencies to Node, the complete host build is triggered.

Anyhow, lets please move that discussion to a new PR after we finally take the first (easier) step of merging this one to go to v6, keeping the building logic as it was.
I promise to submit it afterwards.

jow- · 2017-08-17T13:44:47Z

Agreed.

This was referenced Jan 10, 2017

Add i2c support for Javascript in OpenWRT, Node > 4.4 #2832

Closed

node: update to v4.5.0 #3179

Closed

ianchi force-pushed the node branch from 5c30f30 to ac222bb Compare February 27, 2017 03:36

nxhack reviewed Mar 7, 2017

View reviewed changes

hauke reviewed Jun 5, 2017

View reviewed changes

lang/node/Makefile Outdated

config NODEJS_ICU

bool "enable i18n features"

default y

Copy link

Member

hauke Jun 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this default to n

ianchi force-pushed the node branch 2 times, most recently from f4bcdf0 to 9e208ee Compare June 8, 2017 05:02

ianchi force-pushed the node branch from 9e208ee to ad6656d Compare June 8, 2017 05:06

champtar assigned jow- and blogic and unassigned jow- Jul 15, 2017

ianchi added 3 commits August 16, 2017 21:02

node: bump version and tweak config

653e9ff

Bump version to 6.11.0 Add ICU dependency when using i18n Tweak fpu configs Signed-off-by: Adrian Panella <[email protected]>

node: bump version and add maintainer

162c6a7

Signed-off-by: Adrian Panella <[email protected]>

node: fix path patch

2a42eda

Signed-off-by: Adrian Panella <[email protected]>

ianchi force-pushed the node branch from af56529 to 2a42eda Compare August 17, 2017 06:14

jow- merged commit 12cf3d7 into openwrt:master Aug 17, 2017

ianchi deleted the node branch August 17, 2017 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node: update error & fix compile error #3751

Node: update error & fix compile error #3751

ianchi commented Jan 4, 2017

hnyman commented Jan 4, 2017

ianchi commented Jan 4, 2017

nxhack commented Jan 5, 2017

diizzyy commented Jan 7, 2017

ianchi commented Jan 7, 2017

nxhack commented Jan 10, 2017 •

edited

Loading

ianchi commented Feb 27, 2017

ianchi commented Feb 27, 2017

nxhack commented Feb 27, 2017 •

edited

Loading

nxhack commented Feb 27, 2017

nxhack commented Feb 27, 2017

ianchi commented Feb 27, 2017

nxhack commented Feb 28, 2017

nxhack commented Feb 28, 2017

ianchi commented Feb 28, 2017

nxhack commented Feb 28, 2017

nxhack Mar 7, 2017

hauke left a comment

hauke Jun 5, 2017

ianchi commented Jun 8, 2017

ianchi commented Jul 12, 2017

ianchi commented Aug 16, 2017

artynet commented Aug 16, 2017

hnyman commented Aug 16, 2017 •

edited

Loading

artynet commented Aug 16, 2017

ianchi commented Aug 17, 2017

neocturne commented Aug 17, 2017

jow- commented Aug 17, 2017

ianchi commented Aug 17, 2017

jow- commented Aug 17, 2017

Node: update error & fix compile error #3751

Node: update error & fix compile error #3751

Conversation

ianchi commented Jan 4, 2017

hnyman commented Jan 4, 2017

ianchi commented Jan 4, 2017

nxhack commented Jan 5, 2017

diizzyy commented Jan 7, 2017

ianchi commented Jan 7, 2017

nxhack commented Jan 10, 2017 • edited Loading

ianchi commented Feb 27, 2017

ianchi commented Feb 27, 2017

nxhack commented Feb 27, 2017 • edited Loading

nxhack commented Feb 27, 2017

nxhack commented Feb 27, 2017

ianchi commented Feb 27, 2017

nxhack commented Feb 28, 2017

nxhack commented Feb 28, 2017

ianchi commented Feb 28, 2017

nxhack commented Feb 28, 2017

nxhack Mar 7, 2017

Choose a reason for hiding this comment

hauke left a comment

Choose a reason for hiding this comment

hauke Jun 5, 2017

Choose a reason for hiding this comment

ianchi commented Jun 8, 2017

ianchi commented Jul 12, 2017

ianchi commented Aug 16, 2017

artynet commented Aug 16, 2017

hnyman commented Aug 16, 2017 • edited Loading

artynet commented Aug 16, 2017

ianchi commented Aug 17, 2017

neocturne commented Aug 17, 2017

jow- commented Aug 17, 2017

ianchi commented Aug 17, 2017

jow- commented Aug 17, 2017

nxhack commented Jan 10, 2017 •

edited

Loading

nxhack commented Feb 27, 2017 •

edited

Loading

hnyman commented Aug 16, 2017 •

edited

Loading