1716257 - `mach npm` executes npm with the node from PATH, sometimes causing node-gyp module builds to fail

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

To make activity-stream fail to install its dependencies, ensure that from your shell, node --version show Node 16 on Mac OS X 64 on Intel, and do this:

rm -fr browser/components/newtab/node_modules && ~/.mozbuild/node/bin/npm install --prefix browser/components/newtab

It will blow up, and the error messages will make it clear that it was using Node 16 from the PATH, rather than the version of node in ~/.mozbuild/node/bin

Strictly speaking, I don't think this blocks bug 1690377 from landing, but I think it'd be awfully good to get this landed around the same time.

What's going on:

npm currently is executed by the OS with:

#!/usr/bin/env node

which means it just uses the node in the current PATH.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Bug 1716257 - make `mach npm` use the same version of node, r?mhentges!,Mardak!

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Julien, I'd be interested to hear if this patch solves the problem you've been having with PATH stuff, or if wants its own bug...

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c3b77bb3032
make `mach npm` use the same version of node, r=mhentges

Comment 4

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/9c3b77bb3032

Comment 5

[Julien Wajsberg [:julienw]]

The problem I see is when building the debugger source code, the lines I see in the output look like this:

0:30.42 Executing "/usr/bin/nodejs /home/julien/travail/git/mozilla-central-artifacts/devtools/client/shared/build/build.js /home/julien/travail/git/mozilla-central-artifacts/devtools/client/debugger/src/main.js /home/julien/travail/git/mozilla-central-artifacts/devtools/client/debugger/src/vendors.js /home/julien/travail/git/mozilla-central-artifacts/obj-x86_64-pc-linux-gnu-opt-artifact/dist/bin/browser/chrome/devtools/modules/devtools/client/debugger/src"

In the configure part of the build, I get:

 0:02.06 checking for nodejs... /usr/bin/nodejs (10.24.0)

I'm especially surprised because I see that ./mach bootstrap downloads a node installation (currently v10):

 0:02.82 Setting up artifact node.tar.zst
 0:02.82 Using artifact from local cache: /home/julien/.mozbuild/toolchains/6aef59b6410bef2e-node.tar.zst
 0:02.88 rm tree: /home/julien/.mozbuild/node
 0:03.06 untarring "/home/julien/.mozbuild/node.tar.zst"

(in this case it reused a previously downloaded version)

So why don't we use that one?

I still see the same thing after this patch.

Comment 6

[Mike Hommey [:glandium]]

(In reply to Julien Wajsberg [:julienw] from comment #5)

The problem I see is when building the debugger source code, the lines I see in the output look like this:

0:30.42 Executing "/usr/bin/nodejs /home/julien/travail/git/mozilla-central-artifacts/devtools/client/shared/build/build.js /home/julien/travail/git/mozilla-central-artifacts/devtools/client/debugger/src/main.js /home/julien/travail/git/mozilla-central-artifacts/devtools/client/debugger/src/vendors.js /home/julien/travail/git/mozilla-central-artifacts/obj-x86_64-pc-linux-gnu-opt-artifact/dist/bin/browser/chrome/devtools/modules/devtools/client/debugger/src"

In the configure part of the build, I get:

 0:02.06 checking for nodejs... /usr/bin/nodejs (10.24.0)

I'm especially surprised because I see that ./mach bootstrap downloads a node installation (currently v10):

 0:02.82 Setting up artifact node.tar.zst
 0:02.82 Using artifact from local cache: /home/julien/.mozbuild/toolchains/6aef59b6410bef2e-node.tar.zst
 0:02.88 rm tree: /home/julien/.mozbuild/node
 0:03.06 untarring "/home/julien/.mozbuild/node.tar.zst"

(in this case it reused a previously downloaded version)

So why don't we use that one?

I still see the same thing after this patch.

You're building with --enable-release, aren't you?

Comment 7

[Julien Wajsberg [:julienw]]

You're building with --enable-release, aren't you?

I'm not :-)

I filed Bug 1716600 about my issue (I should have noticed earlier), Dan looked at it yesterday and concluded this wasn't a big problem, as we're taking the system's nodejs only if it satisfies the version requirement.
I guess we could still see the same issue as this one if the system's node is too recent... but that shouldn't happen on a Debian installation :-D