v20.6.1 proposal #49528

RafaelGSS · 2023-09-07T12:32:34Z

2023-09-08, Version 20.6.1 (Current), @RafaelGSS

Commits

[8acbe6d8e8] - esm: fix loading of CJS modules from ESM (Antoine du Hamel) #49500

PR-URL: #49500 Fixes: #49497 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>

nodejs-github-bot · 2023-09-07T12:32:38Z

Review requested:

@nodejs/loaders

RafaelGSS · 2023-09-07T12:34:13Z

As said before, I'm afraid I won't be able to sign/promote this release. I'm creating the proposal to accelerate the process. For the releaser that will move forward with the release, feel free to remove my name in the CHANGELOG or change the release date.

cc: @nodejs/releasers

Notable changes: esm: * fix loading of CJS modules from ESM (Antoine du Hamel) #49500 PR-URL: #49528

RafaelGSS · 2023-09-07T12:40:00Z

CITGM: https://ci.nodejs.org/job/citgm-smoker/3213/ -> https://ci.nodejs.org/job/citgm-smoker/3214/.
📦 https://ci-release.nodejs.org/job/iojs+release/9619/
CI: https://ci.nodejs.org/job/node-test-pull-request/53780/

nodejs-github-bot · 2023-09-07T12:44:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/53780/

mcollina · 2023-09-07T17:12:11Z

Could #49225 be included?

mcollina

lgtm

RafaelGSS · 2023-09-07T18:23:20Z

CITGM result (comparing with last v20.6.0 run)

FAILURE: 43 failures in 3213 not present in 3212


┌────────────────────────┬────────────────────────────┬────────────────────────────┬────────────────────────────┬────────────────────────────┬───────────────────────┬─────────────────────────┬─────────────────┐
│        (index)         │             0              │             1              │             2              │             3              │           4           │            5            │        6        │
├────────────────────────┼────────────────────────────┼────────────────────────────┼────────────────────────────┼────────────────────────────┼───────────────────────┼─────────────────────────┼─────────────────┤
│      aix72-ppc64       │       'async-v3.2.4'       │     'fastify-v4.22.2'      │ 'https-proxy-agent-v7.0.2' │                            │                       │                         │                 │
│       rhel8-x64        │     'fastify-v4.22.2'      │ 'https-proxy-agent-v7.0.2' │       'npm-v10.0.0'        │    'spawn-wrap-v2.0.0'     │   'winston-v3.10.0'   │                         │                 │
│        osx1015         │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │     'ember-cli-v5.2.1'     │     'resolve-v1.22.4'      │     'npm-v10.0.0'     │                         │                 │
│      rhel8-s390x       │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │   'multer-v1.4.5-lts.1'    │                            │                       │                         │                 │
│     rhel8-ppc64le      │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │       'pump-v3.0.0'        │       'npm-v10.0.0'        │ 'tough-cookie-v4.1.3' │ 'torrent-stream-v1.2.1' │                 │
│       win-vs2019       │ 'https-proxy-agent-v7.0.2' │       'npm-v10.0.0'        │                            │                            │                       │                         │                 │
│      debian10-x64      │      'bcrypt-v5.1.1'       │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │       'npm-v10.0.0'        │     'pump-v3.0.0'     │                         │                 │
│ fedora-last-latest-x64 │       'async-v3.2.4'       │        'ava-v5.3.1'        │      'clinic-v13.0.0'      │ 'https-proxy-agent-v7.0.2' │   'fastify-v4.22.2'   │      'npm-v10.0.0'      │ 'rewire-v7.0.0' │
│   fedora-latest-x64    │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │       'npm-v10.0.0'        │                            │                       │                         │                 │
│     ubuntu1804-64      │       'async-v3.2.4'       │     'leveldown-v6.1.1'     │      'rewire-v7.0.0'       │        'ws-v8.13.0'        │                       │                         │                 │
└────────────────────────┴────────────────────────────┴────────────────────────────┴────────────────────────────┴────────────────────────────┴───────────────────────┴─────────────────────────┴─────────────────┘

I've looked at some of them and apparently, it's a machine issue. Mostly, timeouts. Since it's just a revert, most of the issues should already exist in v20.6.0. My only concern is: https-proxy-agent failing with Error: incorrect header check.

Looking at the stack trace:

error:                     | fatal: not a git repository (or any parent up to mount point /)

it also seems a file system issue in the CITGM machine.

dnalborczyk · 2023-09-07T18:33:03Z

Since it's just a revert, most of the issues should already exist in v20.6.0.

@RafaelGSS I think the commit is a fix, not a revert.

RafaelGSS · 2023-09-07T18:44:43Z

I ran another CITGM test just to be sure: https://ci.nodejs.org/job/citgm-smoker/3214/.

bricss · 2023-09-07T19:02:34Z

It will be extremally 🆒 to have #49225 pilled in 🥳

RafaelGSS · 2023-09-07T19:10:40Z

Could #49225 be included?

It will be extremally 🆒 to have #49225 pilled in 🥳

It should go in the next week's release (v20.7.0). It contains a semver-minor PR.

bricss · 2023-09-08T10:42:54Z

That's will be eleventh-hour request 📨 but maybe #49424 can be pulled in 🤞

RafaelGSS · 2023-09-08T11:14:25Z

CITGM results:

FAILURE: 42 failures in 3214 not present in 3212


┌────────────────────────┬────────────────────────────┬────────────────────────────┬───────────────────┬────────────────────┬───────────────────┬─────────────────────────┐
│        (index)         │             0              │             1              │         2         │         3          │         4         │            5            │
├────────────────────────┼────────────────────────────┼────────────────────────────┼───────────────────┼────────────────────┼───────────────────┼─────────────────────────┤
│     ubuntu1804-64      │       'async-v3.2.4'       │     'leveldown-v6.1.1'     │  'rewire-v7.0.0'  │    'ws-v8.13.0'    │                   │                         │
│     rhel8-ppc64le      │       'async-v3.2.4'       │ 'https-proxy-agent-v7.0.2' │ 'fastify-v4.22.2' │   'pump-v3.0.0'    │   'npm-v10.0.0'   │ 'torrent-stream-v1.2.1' │
│       rhel8-x64        │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │   'npm-v10.0.0'   │                    │                   │                         │
│ fedora-last-latest-x64 │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │   'npm-v10.0.0'   │   'pino-v8.15.0'   │  'rewire-v7.0.0'  │                         │
│      debian10-x64      │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │   'npm-v10.0.0'   │   'pump-v3.0.0'    │                   │                         │
│   fedora-latest-x64    │      'bcrypt-v5.1.1'       │ 'https-proxy-agent-v7.0.2' │ 'fastify-v4.22.2' │   'npm-v10.0.0'    │ 'winston-v3.10.0' │                         │
│      aix72-ppc64       │     'fastify-v4.22.2'      │ 'https-proxy-agent-v7.0.2' │   'koa-v2.14.2'   │                    │                   │                         │
│        osx1015         │ 'express-session-v1.17.3'  │ 'https-proxy-agent-v7.0.2' │ 'fastify-v4.22.2' │ 'ember-cli-v5.2.1' │   'npm-v10.0.0'   │  'tough-cookie-v4.1.3'  │
│       win-vs2019       │ 'https-proxy-agent-v7.0.2' │    'browserify-v17.0.0'    │   'ws-v8.14.0'    │   'npm-v10.0.0'    │                   │                         │
│      rhel8-s390x       │ 'https-proxy-agent-v7.0.2' │     'fastify-v4.22.2'      │                   │                    │                   │                         │
└────────────────────────┴────────────────────────────┴────────────────────────────┴───────────────────┴────────────────────┴───────────────────┴─────────────────────────┘

We need to investigate those before landing this patch. @UlisesGascon will take the lead on this release.

See the failures at: https://ci.nodejs.org/job/citgm-smoker/3214/. Could you guys help in this investigation? cc: @aduh95

nicolo-ribaudo · 2023-09-08T11:40:20Z

Regarding the npm 10 failures, it looks like the problem is that:

when running in cigtm, the cwd is /Users/iojs/tmp/citgm_tmp/a2a9ff9c-f683-41e9-83de-b8943b84af1c/npm
npm has some cleanup logic for logged output that replaces UUIDs with the string npm_*** (defined here, and used for example here)
that replaces /Users/iojs/tmp/citgm_tmp/a2a9ff9c-f683-41e9-83de-b8943b84af1c/npm in logs with /Users/iojs/tmp/citgm_tmp/npm_***/npm
npm has tests to validate their logs, and to make them portable replaces the cwd with the string {CWD}. It also performs some other replacements, and then replaces {CWD}/{TESTDIR}/{TAPDIR} with just {CWD} (defined here)

That's why all the failures look like this:

                 --- expected
               
                 +++ actual
               
                 @@ -1,3 +1,3 @@
               
                 -extraneous: chai@1.0.0 {CWD}/prefix/node_modules/chai
               
                 -invalid: foo@1.0.0 {CWD}/prefix/node_modules/foo
               
                 +extraneous: chai@1.0.0 /Users/iojs/tmp/citgm_tmp/npm_***/npm/{TESTDIR}/{TAPDIR}/prefix/node_modules/chai
               
                 +invalid: foo@1.0.0 /Users/iojs/tmp/citgm_tmp/npm_***/npm/{TESTDIR}/{TAPDIR}/prefix/node_modules/foo
                  missing: ipsum@^1.0.0, required by test-npm-ls@1.0.0

it fails to replace the cwd because it has been butchered.

This is not a bug with Node.js: it's npm that does not support running their tests in a folder that contains a UUID in its name.

(note: I didn't actually try running npm tests locally, so somebody with knowledge about npm tests should confirm this)

dnalborczyk · 2023-09-08T12:52:36Z

CITGM results:

FAILURE: 42 failures in 3214 not present in 3212

something is off with the diffing, e.g. https-proxy-agent fails in 3212 as well.

that said, you can ignore the https-proxy-agent failures, citgm is not able to download the tarball, as the location has changed.

nodejs/citgm#959 (comment)

dnalborczyk · 2023-09-08T13:03:11Z

[most/all?] fastify errors exist in 3212 as well , just ran citgm locally on a macOS M2 and they pass.

dnalborczyk · 2023-09-08T13:10:18Z

express-session seems to be a flaky test (failure has nothing to do with the fix)

dnalborczyk · 2023-09-08T13:13:48Z

bcrypt looks like a test timeout issue, unrelated to the fix, and also fails in 3212.

dnalborczyk · 2023-09-08T13:15:18Z

leveldown is an infra issue nodejs/citgm#959 (comment)

dnalborczyk · 2023-09-08T13:25:57Z

rewire seems to be a flaky test, same failure in 3212. looks unrelated to fix.

dnalborczyk · 2023-09-08T13:37:29Z

winston seems like a flaky test, and appears to be unrelated to fix.

dnalborczyk · 2023-09-08T14:02:56Z

(the one) pino failure appears to be unrelated, and fails in more tests for a variety of reasons on [almost] every OS in 3212. if anything, the 3214 run looks way better.

dnalborczyk · 2023-09-08T14:06:55Z

koa has a "connect time out" error on AIX (didn't run any tests at all). I'd guess infra/flaky, hard to tell.

dnalborczyk · 2023-09-08T14:13:03Z

ember-cli fails in both runs, the same on each Ubuntu and OSX. the runs are also harder to compare since they picked up a variety of versions: 5.1.0, 5.2.0 and 5.2.1. might be another short coming of citgm. anyhow, I'd say flaky tests.

dnalborczyk · 2023-09-08T14:15:58Z

pump has the same (1) timeout failure on 3212. I'd guess flaky.

dnalborczyk · 2023-09-08T14:22:24Z

torrent-stream has the same failure on a variety of OS in both runs. therefore I'd think it's unrelated to the fix.

dnalborczyk · 2023-09-08T14:24:23Z

tough-cookie has the same failure in both runs in diff OS. seems unrelated to fix.

dnalborczyk · 2023-09-08T14:28:53Z

browserify has some failures in both runs in diff OSs. looks flaky, unrelated, hard to tell. 🤷‍♂️

dnalborczyk · 2023-09-08T14:31:49Z

async same failures in both runs on a variety of OSs. looks flaky, unrelated.

dnalborczyk · 2023-09-08T14:37:59Z

ws appears to have the same (1) failure in both runs, although more often in 3212. looks unrelated/flaky. if anything, the 3214 run looks better. nodejs/citgm#959 (comment)

dnalborczyk · 2023-09-08T14:51:43Z

I think that's it (including Nicolo's npm analysis #49528 (comment) )

citgm related discussion: nodejs/citgm#959

ruyadorno · 2023-09-08T15:18:34Z

Thank you so much @dnalborczyk! I'm confident enough and will proceed with the promotion steps!

Also thanks @nicolo-ribaudo for debugging the npm citgm issue, I forwarded your msg to the npm team 😊

PR-URL: #49528

Notable changes: esm: * fix loading of CJS modules from ESM (Antoine du Hamel) #49500 PR-URL: #49528

Refs: nodejs/node#49528

Notable changes: esm: * fix loading of CJS modules from ESM (Antoine du Hamel) nodejs#49500 PR-URL: nodejs#49528

esm: fix loading of CJS modules from ESM

8acbe6d

PR-URL: #49500 Fixes: #49497 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>

RafaelGSS added the release Issues and PRs related to Node.js releases. label Sep 7, 2023

RafaelGSS mentioned this pull request Sep 7, 2023

esm: fix loading of (recursive) CJS modules from ESM #49500

Merged

RafaelGSS added request-ci Add this label to start a Jenkins CI on a PR. and removed meta Issues and PRs related to the general management of the project. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Sep 7, 2023

2023-09-08, Version 20.6.1 (Current)

d2c7c36

Notable changes: esm: * fix loading of CJS modules from ESM (Antoine du Hamel) #49500 PR-URL: #49528

RafaelGSS force-pushed the v20.6.1-proposal branch from 0d92ff0 to d2c7c36 Compare September 7, 2023 12:39

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 7, 2023

mcollina approved these changes Sep 7, 2023

View reviewed changes

JakobJingleheimer approved these changes Sep 7, 2023

View reviewed changes

steveluscher mentioned this pull request Sep 7, 2023

ci: pin node to under 20.5.1 solana-labs/solana-web3.js#1575

Closed

ruyadorno added a commit that referenced this pull request Sep 8, 2023

Working on: v20.6.2

3dbc61c

PR-URL: #49528

ruyadorno merged commit d2c7c36 into v20.x Sep 8, 2023
49 checks passed

ruyadorno deleted the v20.6.1-proposal branch September 8, 2023 15:41

ruyadorno pushed a commit that referenced this pull request Sep 8, 2023

2023-09-08, Version 20.6.1 (Current)

0a2ab4c

Notable changes: esm: * fix loading of CJS modules from ESM (Antoine du Hamel) #49500 PR-URL: #49528

ruyadorno mentioned this pull request Sep 8, 2023

Blog: v20.6.1 release post nodejs/nodejs.org#5763

Merged

ruyadorno added a commit to ruyadorno/nodejs.org that referenced this pull request Sep 8, 2023

Blog: v20.6.1 release post

ab47794

Refs: nodejs/node#49528

ruyadorno added a commit to ruyadorno/nodejs.org that referenced this pull request Sep 8, 2023

Blog: v20.6.1 release post

5377a0a

Refs: nodejs/node#49528

ruyadorno added a commit to nodejs/nodejs.org that referenced this pull request Sep 8, 2023

Blog: v20.6.1 release post

74d68bc

Refs: nodejs/node#49528

UlisesGascon mentioned this pull request Sep 10, 2023

v20.7.0 proposal #49592

Merged

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023

2023-09-08, Version 20.6.1 (Current)

5d8d9e3

Notable changes: esm: * fix loading of CJS modules from ESM (Antoine du Hamel) nodejs#49500 PR-URL: nodejs#49528

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v20.6.1 proposal #49528

v20.6.1 proposal #49528

RafaelGSS commented Sep 7, 2023

nodejs-github-bot commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023 •

edited

Loading

nodejs-github-bot commented Sep 7, 2023

mcollina commented Sep 7, 2023

mcollina left a comment

RafaelGSS commented Sep 7, 2023

dnalborczyk commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023

bricss commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023 •

edited

Loading

bricss commented Sep 8, 2023

RafaelGSS commented Sep 8, 2023

nicolo-ribaudo commented Sep 8, 2023 •

edited

Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023 •

edited

Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023 •

edited

Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023 •

edited

Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

ruyadorno commented Sep 8, 2023

v20.6.1 proposal #49528

v20.6.1 proposal #49528

Conversation

RafaelGSS commented Sep 7, 2023

2023-09-08, Version 20.6.1 (Current), @RafaelGSS

Commits

nodejs-github-bot commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023 • edited Loading

nodejs-github-bot commented Sep 7, 2023

mcollina commented Sep 7, 2023

mcollina left a comment

Choose a reason for hiding this comment

RafaelGSS commented Sep 7, 2023

dnalborczyk commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023

bricss commented Sep 7, 2023

RafaelGSS commented Sep 7, 2023 • edited Loading

bricss commented Sep 8, 2023

RafaelGSS commented Sep 8, 2023

nicolo-ribaudo commented Sep 8, 2023 • edited Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023 • edited Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023 • edited Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023 • edited Loading

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

dnalborczyk commented Sep 8, 2023

ruyadorno commented Sep 8, 2023

RafaelGSS commented Sep 7, 2023 •

edited

Loading

RafaelGSS commented Sep 7, 2023 •

edited

Loading

nicolo-ribaudo commented Sep 8, 2023 •

edited

Loading

dnalborczyk commented Sep 8, 2023 •

edited

Loading

dnalborczyk commented Sep 8, 2023 •

edited

Loading

dnalborczyk commented Sep 8, 2023 •

edited

Loading