Closed Bug 1762571 Opened 3 years ago Closed 3 years ago

upgrade NodeJS from v12 to v14 LTS

Categories

(Firefox Build System :: Toolchains, task)

Product:
Firefox Build System
Component:
Toolchains
Type:
task

Tracking

(firefox104 fixed)

Status:
RESOLVED FIXED
Milestone:
104 Branch
Tracking Flags:
Tracking Status
firefox104 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

(Blocks 1 open bug)

Details

User Story

How to test locally:

- Apply patch from Phabricator (attachment link in this bug)
- Execute `MOZ_SCM_LEVEL=1 mach bootstrap` to get the Node 12 executables from the try servers
- `mach build`, `mach lint -l eslint`, `mach lint -l eslint --fix` with dirty tree (ie prettier),  etc.

Attachments

(5 files)

Bug 1762571 - Upgrade NodeJS to v12 for the build system and v16 for tests. r?#firefox-build-system-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1762571 - Use existing node_modules bundles for devtools tests. r?#firefox-build-system-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1762571 - Update node_modules bundles and package-lock.json files for new node versions. r?#firefox-build-system-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1762571 - Limit xpcshell-tests to node v12 whilst we still support Windows 7. r?#firefox-build-system-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1762571 - Don't force set nodejs_path in remove_executables as this causes tests to use the wrong node version. r?#firefox-build-system-reviewers
48 bytes, text/x-phabricator-request
Details | Review

+++ This bug was initially created as a clone of Bug #1690377 +++

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 14 LTS before that happens.

Mike, given the discussions that were held in bug 1690377, do you have a sense of if we should update base-toolchain from v10 to v12 or to v14? I'm guessing probably v12, but wanted to get your opinion on it.

Are we happy to land an upgrade patch before the next ESR (102)?

I'm assuming that we will leave ESR 91 as it is.

Flags: needinfo?(mh+mozilla)

Would it be possible to go directly to v16?

I'll be landing a browsertime update that requires node v16+ shortly (within the next month). We have that version available in CI also: https://hg.mozilla.org/integration/autoland/rev/51255667b6cc

v12 would probably more tractable downstream.

Flags: needinfo?(mh+mozilla)

(that said, if the build can still keep working with v10, that'd be even better)

Assignee: nobody → standard8
Depends on: 1770238

Hi Mark, I just saw the dependency on bug 1770238. Are we now going to upgrade to nodejs 14 LTS or 16 LTS? If it's the latter we should update the bug's summary.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #5)

Hi Mark, I just saw the dependency on bug 1770238. Are we now going to upgrade to nodejs 14 LTS or 16 LTS? If it's the latter we should update the bug's summary.

No decision yet - which is why that's a dependency rather than something this blocks. I was going to let the devtools team investigate for a couple of days first. I've not even looked to see if 14 would pass tests.

As far as devtools go, we can definitely go for 16. We already have a patch attached to Bug 1770238, which passes the tests with Node 16. It's not the cleanest approach, that's why we are still investigating but if it helps I can land it.

The eslint test already has these setup and uses them rather than running npm install each time.

Depends on D147173

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

(that said, if the build can still keep working with v10, that'd be even better)

I'm torn because I can sympathise with both sides of the argument. Whilst I understand that Linux distros want to keep supporting the older versions for longer, the truth is that those versions are now of support from the creators and are potentially vulnerable - so presumably Linux distros are taking on the responsibility for that.

For the Firefox build system, I would expect the risk of using out of date node versions is low. However, I'm not convinced that we should be taking on responsibility for using very old versions when it is only required by third parties. We are definitely not on the leading edge here, and I think there should be a defined limit about how out of date we allow ourselves to go.

Overall I think that we should update to v12 for the build system. Although we don't have a requirement for v12 at the moment, various packages have already dropped support for v10, and so if we find ourselves wanting or needing to update those for whatever reason (security, bugs etc), then it would be easier if we did not have to upgrade node at the same time. Additionally, we also keep ourselves on a regular cadence of upgrades which helps sets expectations and reduces the risk that in future we might end up with a situation where we have a significant amount of work/risk for a large upgrade jump.

Given that ESR 102.x is effectively branching tomorrow (soft freeze starts), I think we can leave that as it is. I would assume that will help the LTS releases of Linux, since I'm guessing they're on ESR by default. If they want to keep the latest version of Firefox on node 10, then they can still make it work with minimal effort for the time being, but it will be their responsibility to manage.

Attachment #9277936 - Attachment description: Bug 1762571 - Update node_modules bundles and package-lock.json files for new node versions. r?gijs → Bug 1762571 - Update node_modules bundles and package-lock.json files for new node versions. r?#firefox-build-system-reviewers
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0ead2f6d0582 Upgrade NodeJS to v12 for the build system and v16 for tests. r=firefox-build-system-reviewers,glandium https://hg.mozilla.org/integration/autoland/rev/f24051722cee Use existing node_modules bundles for devtools tests. r=marco https://hg.mozilla.org/integration/autoland/rev/9e6eb444acb4 Update node_modules bundles and package-lock.json files for new node versions. r=mossop

Backed out for causing xpcshell failures on test_trr_httpssvc_wrap.js.

Push with failures

Failure log

Backout link

[task 2022-06-30T15:40:37.838Z] 15:40:37     INFO -  TEST-START | netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js
[task 2022-06-30T15:40:37.970Z] 15:40:37  WARNING -  TEST-UNEXPECTED-FAIL | netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js | xpcshell return code: 0
[task 2022-06-30T15:40:37.971Z] 15:40:37     INFO -  TEST-INFO took 133ms
[task 2022-06-30T15:40:37.971Z] 15:40:37     INFO -  >>>>>>>
[task 2022-06-30T15:40:37.971Z] 15:40:37     INFO -  TEST-PASS | netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js | setup - [setup : 15] "" != null
[task 2022-06-30T15:40:37.972Z] 15:40:37  WARNING -  TEST-UNEXPECTED-FAIL | netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js | setup - [setup : 16] "" != ""
[task 2022-06-30T15:40:37.972Z] 15:40:37     INFO -  Z:/task_1656601555/build/tests/xpcshell/tests/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js:setup:16
[task 2022-06-30T15:40:37.973Z] 15:40:37     INFO -  Z:/task_1656601555/build/tests/xpcshell/tests/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js:null:48
[task 2022-06-30T15:40:37.973Z] 15:40:37     INFO -  Z:\task_1656601555\build\tests\xpcshell\head.js:load_file:755
[task 2022-06-30T15:40:37.973Z] 15:40:37     INFO -  Z:\task_1656601555\build\tests\xpcshell\head.js:_load_files:772
[task 2022-06-30T15:40:37.974Z] 15:40:37     INFO -  Z:\task_1656601555\build\tests\xpcshell\head.js:_execute_test:569
[task 2022-06-30T15:40:37.974Z] 15:40:37     INFO -  -e:null:1
[task 2022-06-30T15:40:37.974Z] 15:40:37     INFO -  exiting test
[task 2022-06-30T15:40:37.974Z] 15:40:37     INFO -  NS_ERROR_ABORT:
[task 2022-06-30T15:40:37.975Z] 15:40:37     INFO -  _abort_failed_test@Z:\task_1656601555\build\tests\xpcshell\head.js:868:20
[task 2022-06-30T15:40:37.975Z] 15:40:37     INFO -  do_report_result@Z:\task_1656601555\build\tests\xpcshell\head.js:969:5
[task 2022-06-30T15:40:37.975Z] 15:40:37     INFO -  Assert<@Z:\task_1656601555\build\tests\xpcshell\head.js:75:21
[task 2022-06-30T15:40:37.976Z] 15:40:37     INFO -  proto.report@resource://testing-common/Assert.jsm:228:10
[task 2022-06-30T15:40:37.976Z] 15:40:37     INFO -  notEqual@resource://testing-common/Assert.jsm:285:8
[task 2022-06-30T15:40:37.976Z] 15:40:37     INFO -  setup@Z:/task_1656601555/build/tests/xpcshell/tests/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js:16:10
[task 2022-06-30T15:40:37.977Z] 15:40:37     INFO -  @Z:/task_1656601555/build/tests/xpcshell/tests/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js:48:1
[task 2022-06-30T15:40:37.977Z] 15:40:37     INFO -  load_file@Z:\task_1656601555\build\tests\xpcshell\head.js:755:11
[task 2022-06-30T15:40:37.978Z] 15:40:37     INFO -  _load_files@Z:\task_1656601555\build\tests\xpcshell\head.js:772:10
[task 2022-06-30T15:40:37.978Z] 15:40:37     INFO -  _execute_test@Z:\task_1656601555\build\tests\xpcshell\head.js:569:14
[task 2022-06-30T15:40:37.978Z] 15:40:37     INFO -  @-e:1:1
[task 2022-06-30T15:40:37.978Z] 15:40:37     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2022-06-30T15:40:37.979Z] 15:40:37     INFO -  TypeError: can't access property "setIntPref", prefs is undefined at Z:/task_1656601555/build/tests/xpcshell/tests/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js:69
[task 2022-06-30T15:40:37.979Z] 15:40:37     INFO -  run_test@Z:/task_1656601555/build/tests/xpcshell/tests/netwerk/test/unit_ipc/test_trr_httpssvc_wrap.js:69:3
[task 2022-06-30T15:40:37.980Z] 15:40:37     INFO -  _execute_test@Z:\task_1656601555\build\tests\xpcshell\head.js:591:7
[task 2022-06-30T15:40:37.980Z] 15:40:37     INFO -  @-e:1:1
[task 2022-06-30T15:40:37.980Z] 15:40:37     INFO -  <<<<<<<
[task 2022-06-30T15:40:37.986Z] 15:40:37     INFO -  TEST-START | netwerk/test/unit_ipc/test_dns_by_type_resolve_wrap.js
Flags: needinfo?(standard8)

Ok, it looks like nodejs v14 dropped support for Windows 7, which we still run in xpcshell-tests (upgrading breaks the nodejs server run from xpcshell).

One option would be to keep all xpcshell-tests on Node v12 - this would enable the tests to keep running but ensure that only pushing one platform on try should fail.

I would like to keep at least the source linters on Node v16 if we can, since I keep on wanting to use the newer JavaScript constructs that aren't available there and the difference between local development and CI keeps catching me out.

Flags: needinfo?(standard8)
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5b3d5876d849 Upgrade NodeJS to v12 for the build system and v16 for tests. r=firefox-build-system-reviewers,glandium https://hg.mozilla.org/integration/autoland/rev/897705659a1f Use existing node_modules bundles for devtools tests. r=marco https://hg.mozilla.org/integration/autoland/rev/3ee22c60f6fb Update node_modules bundles and package-lock.json files for new node versions. r=mossop https://hg.mozilla.org/integration/autoland/rev/360cb5478445 Limit xpcshell-tests to node v12 whilst we still support Windows 7. r=firefox-build-system-reviewers,ahochheiden https://hg.mozilla.org/integration/autoland/rev/29ca041fa887 Don't force set nodejs_path in remove_executables as this causes tests to use the wrong node version. r=firefox-build-system-reviewers,ahochheiden
See Also: → 1782523
See Also: → 1785742
Regressions: 1787508
Blocks: 1842167
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: