Closed Bug 1471028 Opened 5 years ago Closed 5 years ago
.js in configure as an optional dependency
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.
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.
Greg, thanks for putting this together!
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
Attachment #8987678 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/fe18414fe685 Detect Node.js in configure; r=glandium
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.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/mozilla-central/rev/0c5ad222605b fix check for outdated node versions; r=bustage, a=bustage
(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?
env['NODE_DISABLE_COLORS'] = '1' doesn't help. Commenting out the line doesn't help either.
Jörg beat me with the reply. ;-) It doesn't help.
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.
(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.
5 years ago
Depends on: 1473377
You need to log in before you can comment on or make changes to this bug.