1690377 - upgrade NodeJS from v10 to v12 LTS

Status: RESOLVED FIXED

(Firefox Build System :: Toolchains, task)

Description

[Dan Mosedale (:dmosedale, :dmose)]

NodeJS version 12 LTS becomes will be at the end of its life (ie security bugs no longer will be fixed) on April 30th. We should upgrade both Nightly and ESR to NodeJS version 12 LTS before that happens.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Here's the LTS version support roadmap: https://nodejs.org/en/about/releases/

Comment 2

[Punam Dahiya [:pdahiya]]

See below for Node 8 to 10 upgrade
https://bugzilla.mozilla.org/show_bug.cgi?id=1547823

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Note that several external projects are dropping support for Node 10, and in those cases when we do downstream syncs we no longer can run the tests in our CI. I assume there is no ETA yet when this upgrade will be done?

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

I'm hoping to put a patch on try for this today. That said, if mhommey would prefer to do it instead, I have no objection. :-)

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1690377 - Upgrade Node 10 toolchain to Node 12

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Try build: https://treeherder.mozilla.org/jobs?repo=try&revision=787e08a5417382e50e9cf89c7ea5b76e4d8f453d

Comment 7

[Mike Hommey [:glandium]]

Can we wait for next cycle?

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Meaning 92 Nightly?

Fine by me.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

But maybe this is also a question for Henrik...

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP: Bug 1690377 - Upgrade Node 10 toolchain to Node 12

Comment 11

[Mike Hommey [:glandium]]

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

Meaning 92 Nightly?

Yes

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I mean it's fine for us and we will upgrade to Puppeteer 9.1.0 instead. But I wonder why we have to keep an unsupported release of Node.js running on our ESR branch for the next year. Is that safe enough?

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

I don't know that it's a huge risk, and in the sense that we don't run web-facing servers on it, but we do trust its certificate handling to download and use various things. On balance, I'd prefer that we land it for 91 so that it can be in the ESR, and thus supported on the Node side and more easily upliftable if necessary.

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

This also means that if there are sec fixes in packages that no longer support Node 10 but do support Node 12, those will be much more easily upliftable as well.

Comment 15

[Dan Mosedale (:dmosedale, :dmose)]

What do you think, Mike?

Comment 16

[Mike Hommey [:glandium]]

We could keep node 10 for the base-toolchain jobs for esr, while upgrading the other jobs to node 12.

Comment 17

[Dan Mosedale (:dmosedale, :dmose)]

I have yet to find any docs (either via firefox-source-docs or searchfox/yaml inspection) describing the intent behind base-toolchains versus other toolchains, so the intent behind your suggestion and what consequences it would have are not clear to me. Would you please unpack this in a bit more detail? Thanks!

Comment 18

[Mitchell Hentges [:mhentges] 🦀]

I have yet to find any docs (either via firefox-source-docs or searchfox/yaml inspection) describing the intent behind base-toolchains versus other toolchains

"Base toolchain" jobs are CI tasks that assert that Firefox sources are still compatible with "baseline" versions of the platforms we depend on, such as GCC, Clang, and (according to this ticket) NodeJS.
Normally, we decide on that "baseline" version that we want to support based on our supported configurations. However, (IIUC), none of the supported systems mandate that NodeJS 10 is still supported - besides, as Dan linked earlier, NodeJS 10 LTS is already entirely unsupported. If we were to decide on a "base toolchain" for NodeJS, it should be NodeJS 12 LTS.

The part that confuses me is that Mike says that we should keep base-toolchain jobs on Node 10 for ESR. I agree that ESR should still use Node 10 instead of 12 for compatibility, but I'm unsure how changing the Node toolchain in central will affect the Node toolchain in ESR. My mistake, missed the part about changing the toolchain for ESR.

Comment 19

[Mitchell Hentges [:mhentges] 🦀]

Chatted with Dan a bit in Matrix. A few notes:

• Above, when Dan mentioned ESR, he was referring to ESR 91, not ESR 78. Perhaps there was some confusion here, as I got tripped up by that.
• Node 10 is EOL as of April 2021. ESR 78 depends on Node 10. It's unclear whether we should:
  • Leave ESR 78 with Node 10, despite the security risk (it's around until late 2021), or
  • Bump ESR 78 to use Node 12 and handle the compatibility issues (it's unclear how many there could be, or how tough they'll be to track down).
  • :glandium, I'm interested in your thoughts on ^
• Either way, since toolchain configuration only affects current revisions (right?), it's unclear why a base-toolchain job is needed.

Comment 20

[Dan Mosedale (:dmosedale, :dmose)]

:nchevobbe, I'm seeing regular jest failures on the node(debugger) target. Some of them appear to be related to acorn:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './dist/acorn' is not defined by "exports" in /builds/worker/checkouts/gecko/devtools/client/debugger/node_modules/pretty-fast/node_modules/acorn/package.json

Others are async timeouts.

https://firefoxci.taskcluster-artifacts.net/c7Z_9rjtRVWaSo_6OqDpEQ/0/public/logs/live_backing.log is the log, and yarn install has an awful lot of warnings. Does anything jump out at you? Can you reproduce these locally?

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

For anyone who is interested, here's a try build of the current push:

https://treeherder.mozilla.org/jobs?repo=try&revision=01f2b7091a9733ad8d4c385a1031ede74686cf0b&selectedTaskRun=doAHRMhcSdueVGZKEJVyaQ.0

Comment 22

[Dan Mosedale (:dmosedale, :dmose)]

If anyone has any insights as to what's going on with the bc7 failures in that try push, I'd love to hear them, because in the log I looked at, node didn't appear to be invoked anywhere.

Comment 23

[Nicolas Chevobbe [:nchevobbe]]

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

:nchevobbe, I'm seeing regular jest failures on the node(debugger) target. Some of them appear to be related to acorn:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './dist/acorn' is not defined by "exports" in /builds/worker/checkouts/gecko/devtools/client/debugger/node_modules/pretty-fast/node_modules/acorn/package.json

Others are async timeouts.

https://firefoxci.taskcluster-artifacts.net/c7Z_9rjtRVWaSo_6OqDpEQ/0/public/logs/live_backing.log is the log, and yarn install has an awful lot of warnings. Does anything jump out at you? Can you reproduce these locally?

I do reproduce locally and found the offender. I'll investigate today

Comment 24

[Mark Banner (:standard8)]

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

If anyone has any insights as to what's going on with the bc7 failures in that try push, I'd love to hear them, because in the log I looked at, node didn't appear to be invoked anywhere.

I added some more retriggers, and there's some different failures as well as more greens - I think what you saw is the currently highly frequent bug 1678102.

Comment 25

[Mike Hommey [:glandium]]

There are multiple dimensions to the problem.

• While node upstream EOLed version 10, some downstreams still support it (like Linux distros).
• The build system doesn't need a ton of node, and I wouldn't be surprised if building Firefox still works with even node 8. The build itself is not at risk, security-wise, it only uses in-tree stuff, in a controlled manner.
• Running tests has different requirements wrt node.
• We'd like to force Firefox developers to use a known-secure version of node.

The way we handle the latter is to make configure fail with older versions. This is already a problem with perfectly valid and secure versions of node 10 (because Linux distros backport the fixes, so they don't have the latest version, but they have all the relevant fixes). Soon enough the latter will be handled by --enable-bootstrap being the default on mozilla-central, and it downloading whatever version CI uses.

As per the above, and also more generally, the version configure requires doesn't have to be the exact version the CI uses (or above). This is true for compilers too. The base toolchain jobs test that: the set of oldest versions of things that configure accepts (not for strictly everything, but there's only so much we can do).

Anyways, as long as building and testing with node 10 works, I'd rather configure keep accepting node 10. Hopefully, --enable-bootstrap will be the default before 12 actually becomes a requirement for tests.

Comment 26

[Mark Banner (:standard8)]

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

Anyways, as long as building and testing with node 10 works, I'd rather configure keep accepting node 10. Hopefully, --enable-bootstrap will be the default before 12 actually becomes a requirement for tests.

Is there a tracker for that work?

As a data point, ESLint is planning on dropping node 10 support end of June/start of July. Babel is meant to be dropping support "soon", though their v8 release has been significantly delayed currently.

With both of these, we tend to need to keep them reasonably up to date to cope with new language features (obviously it depends on exactly what is released when, but just to give you an indication).

Comment 27

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #23)

I do reproduce locally and found the offender. I'll investigate today

Wonderful; thanks!

Comment 28

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to Mark Banner (:standard8) from comment #24)

If anyone has any insights as to what's going on with the bc7 failures in that try push...

I added some more retriggers, and there's some different failures as well as more greens - I think what you saw is the currently highly frequent bug 1678102.

Thanks! Looking through them, it seems like the rational thing to do is ignore the bc7 failures, since it has tons of unknown intermittents, and (so far at least), all the logs I've looked at don't implicate Node in any way. I'll keep an eye on future try builds, though.

Comment 29

[Dan Mosedale (:dmosedale, :dmose)]

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

There are multiple dimensions to the problem.

[...]

Thanks for the clear, concise explanation of the key factors in play here.

Anyways, as long as building and testing with node 10 works, I'd rather configure keep accepting node 10. Hopefully, --enable-bootstrap will be the default before 12 actually becomes a requirement for tests.

Do you feel it's important to leave the base toolchain at node 10 also (or as a result of that reasoning)?

Comment 30

[Dan Mosedale (:dmosedale, :dmose)]

@glandium 2nd question: am I correct in thinking that any tests that get their node from mach bootstrap should have any issues automatically caught by try runs? If not, do you know any folks I should check in with to make sure they're going to be ok?

Comment 31

[Mike Hommey [:glandium]]

(In reply to Mark Banner (:standard8) from comment #26)

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

Anyways, as long as building and testing with node 10 works, I'd rather configure keep accepting node 10. Hopefully, --enable-bootstrap will be the default before 12 actually becomes a requirement for tests.

Is there a tracker for that work?

bug 1694889, but it will be enabled before all the dependencies are closed. I'm planning to enable on windows and/or mac next week.

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

Do you feel it's important to leave the base toolchain at node 10 also (or as a result of that reasoning)?

Yes, at least for 91.

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

@glandium 2nd question: am I correct in thinking that any tests that get their node from mach bootstrap should have any issues automatically caught by try runs? If not, do you know any folks I should check in with to make sure they're going to be ok?

Try runs would use the same thing as mach bootstrap does, which is the same as what normal builds (not base-toolchain) use.

Comment 32

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Bug 1690377 - Upgrade Node 10 toolchain to Node 12, r?mhentges!,Standard8!,glandium

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4fdba1128382
Upgrade Node 10 toolchain to Node 12, r=Standard8,mhentges

Comment 34

[Sandor Molnar[:smolnar]]

Backed out for causing nodejs-12 bustages

Backout link: https://hg.mozilla.org/integration/autoland/rev/bbf79ed487b9eeec1c574ac52f2b590aa7e2cfff

Push with failures

Failure log

Comment 35

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Bug 1690377 - Fixup Mac fetch bustage from Node 12 landing, r?mhentges!

Comment 36

[Dan Mosedale (:dmosedale, :dmose)]

Not sure why try didn't catch that error. Ah well. New patch fixes.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd54d705a55d
Upgrade Node 10 toolchain to Node 12, r=Standard8,mhentges

Comment 38

[Cosmin Sabou [:CosminS]]

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