Skip to contentSkip to conversation

(node.js) fix npm CC, other npm tweaks

PATCH New Michael Wei 2 participants 7 messages Nov 23, 2014 – Jul 14, 2015 1 message not in the archive

lore
Share & tools
lore

Michael Wei

author
#0 lore raw

cover (node.js) fix npm CC, other npm tweaks

For some reason, node modules with native code (compiled using tools like gyp) were not being cross compiled properly without the --target_arch switch.

In addition, the version of npm that is provided with the version of node uses is old, and has some bugs like race conditions in dependency resolution which can break builds, so this patch also updates the host npm before install.

Finally, node modules are now installed globally. This allows modules to install things such as command line utilities.

Michael Wei (1):
Fix npm module cross compilation, use up-to-date npm and install
modules globally

 package/nodejs/nodejs.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/1 Fix npm module cross compilation, use up-to-date npm and install modules globally

Signed-off-by Michael Wei <[email protected]>
---
 package/nodejs/nodejs.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
package/nodejs/nodejs.mk +3 -1
index 312aaa9..fbcb730 100644
@@ -110,11 +110,13 @@ define NODEJS_INSTALL_MODULES
 	# npm install call below and setting npm_config_rollback=false can both
 	# help in diagnosing the problem.
 	(cd $(TARGET_DIR)/usr/lib && mkdir -p node_modules && \
+		$(HOST_DIR)/usr/bin/npm install -g npm --prefix=$(HOST_DIR) && \
 		$(TARGET_CONFIGURE_OPTS) \
 		LD="$(TARGET_CXX)" \
 		npm_config_arch=$(NODEJS_CPU) \
 		npm_config_nodedir=$(BUILD_DIR)/nodejs-$(NODEJS_VERSION) \
-		$(HOST_DIR)/usr/bin/npm install \
+		$(HOST_DIR)/usr/bin/npm install -g --target_arch=$(NODEJS_CPU) \
+		--prefix=$(TARGET_DIR) \
 		$(NODEJS_MODULES_LIST) \
 	)
 endef
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/1 Fix npm module cross compilation, use up-to-date npm and install modules globally

Dear Michael Wei,

package/nodejs/nodejs.mk define NODEJS_INSTALL_MODULES · line 113

Quoted 9 lines · Michael Wei
package/nodejs/nodejs.mk +1 -0
index 312aaa9..fbcb730 100644
@@ -110,11 +110,13 @@ define NODEJS_INSTALL_MODULES
 	# npm install call below and setting npm_config_rollback=false can both
 	# help in diagnosing the problem.
 	(cd $(TARGET_DIR)/usr/lib && mkdir -p node_modules && \
+		$(HOST_DIR)/usr/bin/npm install -g npm --prefix=$(HOST_DIR) && \

If I understand correctly, this asks npm to update itself, i.e update the host npm. If that's the case, then I believe this should rather go in the HOST_NODEJS_INSTALL_CMDS.

Also, could you explain what the "-g" option changes? I understand it installs modules "globally", but what means "globally" compared to what was done before? Sorry for the silly questions, but I'm far from being a NodeJS guy, so I don't really know how npm works.

Thanks a lot,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#3 lore raw

PATCH 1/1 Fix npm module cross compilation, use up-to-date npm and install modules globally

Sorry, been busy:

Yes, it does ask npm to update itself first. Perhaps we should make that the default, or some option? I'll look into moving it into HOST_NODEJS_INSTALL_CMDS.

From the npm page:

1. globally ?- This drops modules in {prefix}/lib/node_modules, and puts executable files in{prefix}/bin, where {prefix} is usually something like /usr/local. It also installs man pages in{prefix}/share/man, if they?re supplied.

So before, executables and manpages weren't being installed. So for example, serialport installs the following, typically:

/home/mwei/devel/buildroot/output/target/usr/bin/serialportlist -> /home/mwei/devel/buildroot/output/target/usr/lib/node_modules/serialport/bin/serialportList.js /home/mwei/devel/buildroot/output/target/usr/bin/serialportterm -> /home/mwei/devel/buildroot/output/target/usr/lib/node_modules/serialport/bin/serialportTerminal.js

Without -g, those utilities don't get installed. This is more of an issue for npm modules that you run from the commandline rather than code. typescript, pm2, gulp, and bunyan are some examples.

Quoted 33 lines · Thomas Petazzoni

Dear Michael Wei,

package/nodejs/nodejs.mk define NODEJS_INSTALL_MODULES · line 110

Quoted 6 lines · Michael Wei
package/nodejs/nodejs.mk +0 -0
index 312aaa9..fbcb730 100644
@@ -110,11 +110,13 @@ define NODEJS_INSTALL_MODULES
      # npm install call below and setting npm_config_rollback=false can

both

package/nodejs/nodejs.mk define NODEJS_INSTALL_MODULES

Quoted 3 lines
package/nodejs/nodejs.mk +1 -0
      # help in diagnosing the problem.
      (cd $(TARGET_DIR)/usr/lib && mkdir -p node_modules && \
+             $(HOST_DIR)/usr/bin/npm install -g npm

--prefix=$(HOST_DIR) && \

If I understand correctly, this asks npm to update itself, i.e update the host npm. If that's the case, then I believe this should rather go in the HOST_NODEJS_INSTALL_CMDS.

Also, could you explain what the "-g" option changes? I understand it installs modules "globally", but what means "globally" compared to what was done before? Sorry for the silly questions, but I'm far from being a NodeJS guy, so I don't really know how npm works.

Thanks a lot,

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Signature · 8 lines
*Michael Wei*
Graduate Researcher
PhD Student
Non-Volatile Systems Laboratory @ UCSD
http://cseweb.ucsd.edu/users/m3wei/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20141127/7b017854/attachment-0001.html>
#4 lore raw

PATCH 1/1 Fix npm module cross compilation, use up-to-date npm and install modules globally

In reply to Michael Wei

Michael, J?rg, Martin,

We are currently to clean up the Buildroot patch tracking system and found this patch that hasn't been applied so far. It doesn't apply anymore, and other NodeJS changes have been done since then.

Could you have a look at whether this patch is still needed or not, and if it is still needed, resend an updated version with a more detailed commit log?

In the mean time, we'll mark the patch as Changes Requested in patchwork, which means that we will no longer see it in our list of pending patches: if you forget to resend a new version, we will no longer remember about it. We are not NodeJS experts/users so your help is really needed to improve/maintain this package in Buildroot.

Thanks a lot for your help!

Thomas

Quoted 24 lines · Michael Wei
Signed-off-by Michael Wei <[email protected]>
---
 package/nodejs/nodejs.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
package/nodejs/nodejs.mk +3 -1
index 312aaa9..fbcb730 100644
@@ -110,11 +110,13 @@ define NODEJS_INSTALL_MODULES
 	# npm install call below and setting npm_config_rollback=false can both
 	# help in diagnosing the problem.
 	(cd $(TARGET_DIR)/usr/lib && mkdir -p node_modules && \
+		$(HOST_DIR)/usr/bin/npm install -g npm --prefix=$(HOST_DIR) && \
 		$(TARGET_CONFIGURE_OPTS) \
 		LD="$(TARGET_CXX)" \
 		npm_config_arch=$(NODEJS_CPU) \
 		npm_config_nodedir=$(BUILD_DIR)/nodejs-$(NODEJS_VERSION) \
-		$(HOST_DIR)/usr/bin/npm install \
+		$(HOST_DIR)/usr/bin/npm install -g --target_arch=$(NODEJS_CPU) \
+		--prefix=$(TARGET_DIR) \
 		$(NODEJS_MODULES_LIST) \
 	)
 endef
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw

PATCH 1/1 Fix npm module cross compilation, use up-to-date npm and install modules globally

Dear J?rg Krause,

Quoted 9 lines · J?rg Krause

I replied to the patch on 15 Dec 2014 [1], but the mail was not added to the thread.

In short: The patch does not work as intended.

There was indeed an issue with cross-compilation for certain modules (eg serialport) which was fixed by Martin Bark [1].

I would suggest to mark this patch as Rejected.

Thanks a lot for the feedback! We already marked the patch as Changes Requested, which makes it no longer visible to us in the list of pending patches, so that's good enough.

Again, thanks for the feedback.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#7 lore raw

cover (node.js) fix npm CC, other npm tweaks

In reply to Michael Wei

Dear Michael Wei,

Quoted 11 lines · Michael Wei

For some reason, node modules with native code (compiled using tools like gyp) were not being cross compiled properly without the --target_arch switch.

In addition, the version of npm that is provided with the version of node uses is old, and has some bugs like race conditions in dependency resolution which can break builds, so this patch also updates the host npm before install.

Finally, node modules are now installed globally. This allows modules to install things such as command line utilities.

Thanks for this contribution. Could you include this detailed explanation into the commit log of the patch?

Currently, the detailed explanation is only part of the cover letter, which will not become part of the Buildroot commit history. It would therefore be better to have the same explanation inside the patch itself, so that it will become part of the Buildroot commit history.

Thanks a lot,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com