1471028 - Detect node.js in configure as an optional dependency

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Gregory Szorc [:gps]]

As the first step to integrating Node.js in the build system, we want to introduce a configure check that finds the location of a valid Node.js executable. Landing a check for an *optional* Node dependency will enable us to flush out problems with the configure check before we make it mandatory.

Comment 1

[Phabricator Automation]

Attachment: Bug 1471028 - Detect Node.js in configure; r?glandium

The intent is for the build system to soon require Node.js to build
Firefox. But we aren't ready to make Node.js a build requirement
just yet.

The goal of this commit is to implement configure detection for
Node.js so that we can a) work out detection bugs b) give people a
means to validate system compatibility *before* we throw the switch to
require Node.js.

This commit introduces configure logic for finding a Node.js
executable, resolving its version, and validating its suitability.

By default, if Node.js cannot be found or there is an error resolving
its version, we print some warning messages and move on.

If --enable-nodejs is used (not the default), errors are raised
if Node.js cannot be found or its version isn't suitable.

Once we require Node.js, the added code can likely be simplied.
When writing the code, I went out of my way to make failures as
non-fatal as possible. e.g. normally we'd say that failures to run
`node --version` would be fatal. I'm purposefully trying to not have
this configure check break anyone's environment, even if failure
occurs. Again, the goal is to introduce the configure checks first
in a non-fatal way such that we can debug failures so the flag day
transition is simpler.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Greg, thanks for putting this together!

Comment 3

[Phabricator Automation]

Comment on attachment 8987678 [details]
Bug 1471028 - Detect Node.js in configure; r?glandium

Mike Hommey [:glandium] has approved the revision.

https://phabricator.services.mozilla.com/D1818

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe18414fe685
Detect Node.js in configure; r=glandium

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/fe18414fe685

Comment 6

[Richard Marti (:Paenglab)]

Attachment: build-error.txt

On Win 10 I get this error when I try to build. What is wrong or missing on my side? Backing out this patch let it build again.

On Mac I have no problem.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/0c5ad222605b
fix check for outdated node versions; r=bustage, a=bustage

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to Richard Marti (:Paenglab) from comment #6)
> On Win 10 I get this error when I try to build. What is wrong or missing on
> my side? Backing out this patch let it build again.
> 
> On Mac I have no problem.

It looks like the:

env[b'NODE_DISABLE_COLORS'] = b'1'

in build/moz.configure/node.configure is unusual; all the other `env` modifications in configure don't use binary strings.  Can you try changing it to:

env['NODE_DISABLE_COLORS'] = '1'

and seeing if you can get through configure?

Comment 9

[Jorg K (CEST = GMT+2)]

  env['NODE_DISABLE_COLORS'] = '1'
doesn't help. Commenting out the line doesn't help either.

Comment 10

[Richard Marti (:Paenglab)]

Jörg beat me with the reply. ;-)

It doesn't help.

Comment 11

[Jorg K (CEST = GMT+2)]

The error pointed to line 29:
 0:06.14   File "c:/mozilla-source/comm-central/build/moz.configure/node.configure", line 29, in nodejs_version
 0:06.14     stderr=subprocess.STDOUT)
so making it:
        out = subprocess.check_output([node, '--version'],  <===== removed env=env,
                                      stderr=subprocess.STDOUT)
by removing |env=env| where indicated gets me past the problem.

Comment 12

[Nathan Froyd [:froydnj]]

(In reply to Jorg K (GMT+2) from comment #11)
> The error pointed to line 29:
>  0:06.14   File
> "c:/mozilla-source/comm-central/build/moz.configure/node.configure", line
> 29, in nodejs_version
>  0:06.14     stderr=subprocess.STDOUT)
> so making it:
>         out = subprocess.check_output([node, '--version'],  <===== removed
> env=env,
>                                       stderr=subprocess.STDOUT)
> by removing |env=env| where indicated gets me past the problem.

This is not the right fix.

In any event, this is obviously not a straightforward fix, so let's take the discussion to bug 1473331.