Closed Bug 1547823 Opened 5 years ago Closed 4 years ago

Upgrade Node build/test environment to v10.x

Categories

(Firefox Build System :: General, task, P3)

Product:
Firefox Build System
Component:
General
Type:
task

Tracking

(firefox75 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla75
Tracking Flags:
Tracking Status
firefox75 --- fixed

People

(Reporter: loganfsmyth, Assigned: dmosedale)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Bug 1547823 - Upgrade NodeJS from 8.x to 10.19.0
47 bytes, text/x-phabricator-request
Details | Review
Bug 1547823 - Change node aliases to point to node-10 builds
47 bytes, text/x-phabricator-request
Details | Review
Bug 1547823 - Remove obsolete Node 8 support
47 bytes, text/x-phabricator-request
Details | Review

Currently we use 8.x , but it went into maintenance mode in January 1st and will be end-of-lifed in December, so we should upgrade Node to a newer version. 12.x came came out last week, but presumably that would be a bit rushed and may life hard for distro maintainers, so 10.x seems like a reasonable choice.

Several projects that use Node for building, bundling, and unit testing are beginning to run into frustrations with 8.x due to missing ECMAScript and Node APIs, and the fact that some dependencies are beginning to drop support for it. For example https://bugzilla.mozilla.org/show_bug.cgi?id=1546370

I asked this in https://www.mail-archive.com/dev-builds@lists.mozilla.org/msg01363.html, with no answer, but the question still applies: There are two consumers of node: builds and developer workflow things like eslint. Is it necessary to bump the version required for the former?

It would be nice to upgrade for two reasons.

  1. we have node based unit tests for the debugger that have a Promise polyfill
  2. we would like to use v8's new snapshots in the build and that requires a newer version

(In reply to Jason Laster [:jlast] from comment #3)

It would be nice to upgrade for two reasons.

  1. we have node based unit tests for the debugger that have a Promise polyfill
  2. we would like to use v8's new snapshots in the build and that requires a newer version

None of these seem related to building Firefox.

I think my question would mostly be about how to make CI handle 2 versions. Is there much additional complexity to having build logic that runs in an environment that is different from the test logic?

The build and test environments are already different.

(In reply to Mike Hommey [:glandium] from comment #4)

(In reply to Jason Laster [:jlast] from comment #3)

It would be nice to upgrade for two reasons.

  1. we have node based unit tests for the debugger that have a Promise polyfill
  2. we would like to use v8's new snapshots in the build and that requires a newer version

None of these seem related to building Firefox.

The motivation for number 2 is to speed up build time, at least by a bit. Presumably we'd need to benchmark a bit to know by how much. That said, having two different versions of node for those different things seems like it adds complexity and fragility, so we'd want to think carefully about that.

Note also even if we were to separate build and test versions, Node v8 (what we're currently running) will hit end-of-life at the end of 2019, so it doesn't seem wise to keep them separated longer than that.

Are you saying that newer versions of node can't run code that runs on older versions?

The concern once Node 8 is end-of-lifed is that build tooling in the Node ecosystem will start dropping support for it quickly, which would prevent us from adopting newer versions of tools.

Additionally, once it is end-of-lifed, it will no longer get security fixes.

(In reply to Logan Smyth [:loganfsmyth] from comment #10)

The concern once Node 8 is end-of-lifed is that build tooling in the Node ecosystem will start dropping support for it quickly, which would prevent us from adopting newer versions of tools.

What tool we use to build would be impacted by that?

(In reply to Dan Mosedale (:dmose, :dmosedale) from comment #11)

Additionally, once it is end-of-lifed, it will no longer get security fixes.

That's not our concern to have. It's downstream's. Downstreams that use desupported versions of software is not unheard of, neither is downstreams fixing security issues in those.

What tool we use to build would be impacted by that?

I could certainly see that happening for Webpack, and I'm sure other things.

Once Node 8 is end-of-lifed, new major versions of libraries published to npm will begin to assume that Node 10 is the minimum version anyone will be using and start relying on new functionality introduced in Node 10. For instance, we've already accidentally tried to introduce dependencies in the past (because the dev was using Node 12 locally) that required async generator support in Node , which only landed as part of Node 10. Those patches had to be undone because we were still required to support Node 8.

Blocks: 1490802

(In reply to Logan Smyth [:loganfsmyth] from comment #13)

What tool we use to build would be impacted by that?

I could certainly see that happening for Webpack, and I'm sure other things.

Where do we use Webpack in the current build?

(In reply to Nathan Froyd [:froydnj] from comment #14)

Where do we use Webpack in the current build?

In retrospect probably not the best example. I don't think it runs in CI at the moment, we just run it manually locally which is ugly. Though again if we had people running newer node locally that increases the chance of accidental breakage because something could work locally by chance. Babel is the biggest thing we run in CI, which at the moment supports older Node versions, but again will drop it in future versions, making it harder to upgrade.

(In reply to Nathan Froyd [:froydnj] from comment #14)

(In reply to Logan Smyth [:loganfsmyth] from comment #13)

What tool we use to build would be impacted by that?

I could certainly see that happening for Webpack, and I'm sure other things.

Where do we use Webpack in the current build?

We do not use Webpack but we do use node to build the DevTools Debugger frontend.
It starts with declaration like this:
https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/moz.build#16-20

CompiledModules(
    'main.development.js',
    'main.js',
    'vendors.js',
)

Which is being translated into a nodejs call around here:
https://searchfox.org/mozilla-central/source/devtools/client/shared/build/node-templates.mozbuild#38-41
To finally execute this JS file on nodejs:
https://searchfox.org/mozilla-central/source/devtools/client/shared/build/build.js

It is only there to convert "JSX" files to pain javascript that gecko can evaluate as well as stripping Flow types annotations.
To do that it uses Babel.

All of this is ran to build any Firefox. Full builds as well as artifact builds. So on CI but also local builds.

For tests, we use node in toolchain artifact instead of node in tooltool after Bug 1571573.
I believe it's the right move since it's easier to update and we can see the provenance of the tools we are using

However, we figure out v8.11.3 is not new enough for some TLS proxy testing, which is important at this moment
node crashes after client ends a TLS session (detail in bug 1580976 comment 4)
v8.13.0 and v8.16.1 don't work neither.

Upgrading to node v10 for testing seems a good step to move forward.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb8da28aa9d80ff0d23a3a2d46323550038558dc
node doesn't crash for v10 under current test.

Blocks: 1584397
Assignee: loganfsmyth → nobody
Status: ASSIGNED → NEW

I'm not working on JS parts of the codebase right now so I won't be moving this along, but reminder that Node 8 is reaching end-of-life at the end of December, so it really would be good for us to upgrade to 10 so we can stay on a supported release.

Attachment #9061486 - Attachment is obsolete: true

Thanks for kicking things off, Logan, and I'm gonna iterate on your patch and run with it...

Assignee: nobody → dmose
Blocks: node-toplevel-packages

FWIW, the exact situation in https://bugzilla.mozilla.org/show_bug.cgi?id=1547823#c13 is now happening over in Bug 1607851. Obviously, I think we should update to Node 10: yes, it's not strictly necessary for the build but it's necessary for all sorts of things around the build.

(In reply to Nick Alexander :nalexander [he/him] from comment #20)

FWIW, the exact situation in https://bugzilla.mozilla.org/show_bug.cgi?id=1547823#c13 is now happening over in Bug 1607851. Obviously, I think we should update to Node 10: yes, it's not strictly necessary for the build but it's necessary for all sorts of things around the build.

One thing that we could do for my use case, and that might help glandium's general push to not pull the build requirements forward particularly quickly, is to add node-$VERSION toolchains for specific tasks to consume. It's possible the issue in https://bugzilla.mozilla.org/show_bug.cgi?id=1547823#c17 could be addressed by using a different Node.js for the test jobs.

That's an interesting idea. I'm a bit concerned about the amount of complexity that would add. Maybe it wouldn't be substantial; I'm not sure.

Anyway, you're right that we need to drive Node 10 forward in some way soon; I'll be setting up a meeting to discuss further in Berlin; details here soon.

(In reply to Dan Mosedale (:dmose, :dmosedale) from comment #22)

That's an interesting idea. I'm a bit concerned about the amount of complexity that would add. Maybe it wouldn't be substantial; I'm not sure.

It's a well trodden path -- we have lots of versions of Rust, clang, etc -- and in fact the new toolchain is green at https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9949daae6066c8eed03886ffe1e5a07bd60f7eb&selectedJob=285833074. (I'm waiting for perf testing HW to see if it actually addresses my issue, in which case we can "just do this" and not worry about Node 10 across the board for this specific use case.)

Anyway, you're right that we need to drive Node 10 forward in some way soon; I'll be setting up a meeting to discuss further in Berlin; details here soon.

Sadly, I won't be in Berlin, but my vote is to upgrade :)

(In reply to Nick Alexander :nalexander [he/him] from comment #21)

(In reply to Nick Alexander :nalexander [he/him] from comment #20)

FWIW, the exact situation in https://bugzilla.mozilla.org/show_bug.cgi?id=1547823#c13 is now happening over in Bug 1607851. Obviously, I think we should update to Node 10: yes, it's not strictly necessary for the build but it's necessary for all sorts of things around the build.

One thing that we could do for my use case, and that might help glandium's general push to not pull the build requirements forward particularly quickly, is to add node-$VERSION toolchains for specific tasks to consume. It's possible the issue in https://bugzilla.mozilla.org/show_bug.cgi?id=1547823#c17 could be addressed by using a different Node.js for the test jobs.

Dan reminds me that addressing the issue in automation doesn't help local consumers (in my case, folks running mach browsertime locally, which invokes ~/.mozbuild/node/node or similar). What we will likely do is work with upstream to revert the dependency bump that forces Node 10 and trust that we will bump the global Node version for builds in a reasonable time frame, say within Q1 2020. I'd prefer more like one month, but the All Hands will slow everything down.

Fundamentally my position is that the lowest we can reasonably follow is the Node community's oldest supported LTS, which right now is Node 10. (And I thought that was clear in the initial policy proposals, although I don't have a link to hand).

We will be having a meeting at 10 AM Friday, CET in Berlin, in one of the Schinkel rooms in the InterContinental to discuss and decide how to move forward here. https://docs.google.com/document/d/1vv0-bbGpSkTFNEtQAgPcA35yjmBu1tykGC2YD3cLv10/edit?usp=sharing has the agenda; feel free to add to it.

Hi Dan. I wonder if you have an update for us what were the outcomes from the discussion in Berlin last week. Thanks!

Flags: needinfo?(dmose)

dmose can provide more details, but the quick summary is to stay on the oldest "Long-Term Support" version. With Node 8 ending Maintenance at the end of December 2019, Node 10 is now the target version and currently Active and switching to Maintenance April 2020 until end of April 2021.

https://nodejs.org/en/about/releases/

And to avoid breakages late in the nightly cycle, we'll try to make these major version changes soon after merge dates.

Sorry for the delay here; I was out of the office.

TL;DR:

On the policy side, the intent is to add some info to the policy doc proposals with a policy of requiring the oldest Node LTS version, and that will be one of the things we'll be getting reviewed as those docs get signed off on.

More immediately, now that there has been a Node 10 security release with several HTTP-layer fixes not available in Node 8, the plan is to start requiring Node 10 in mozilla-central (Firefox 75 and higher) this week. Ed posted publicly about this last week; I'm going to post again today and prepare a patch for landing that requirement and updating the toolchain archives used by mach bootstrap.

For those who want somewhat more detail, the agenda from that meeting has additional info, though it's not in a super readable form: https://docs.google.com/document/d/1vv0-bbGpSkTFNEtQAgPcA35yjmBu1tykGC2YD3cLv10/edit

Flags: needinfo?(dmose)

Depends on D62782

Pushed by dmosedale@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/678f7a190ecb
Upgrade NodeJS from 8.x to 10.19.0 r=froydnj
https://hg.mozilla.org/integration/autoland/rev/69879135d52e
Change node aliases to point to node-10 builds r=froydnj
https://hg.mozilla.org/integration/autoland/rev/4a95f898cab8
Remove obsolete Node 8 support r=froydnj

I filed bug 1629993 since the version of node specified here (10.19.0) is no longer appears to be available on nodejs servers. This causes problems when the linux-based docker images (lint, ubuntu1804-test, etc) need to be re-generated, since install-node.sh calls 10.19.0 specifically.

Is there a way to make docker images also dependent on the linux64-node toolchain task or would that approach not be appropriate?

Blocks: 1641863
Blocks: 1643234
No longer blocks: 1643234
Blocks: 1666172
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: