Skip to content

[v12.x backport] module: drop support for extensionless main entry points in esm#32280

Closed
GeoffreyBooth wants to merge 1 commit into
nodejs:v12.x-stagingfrom
GeoffreyBooth:backport-31415-to-12.x-staging
Closed

[v12.x backport] module: drop support for extensionless main entry points in esm#32280
GeoffreyBooth wants to merge 1 commit into
nodejs:v12.x-stagingfrom
GeoffreyBooth:backport-31415-to-12.x-staging

Conversation

@GeoffreyBooth
Copy link
Copy Markdown
Member

@GeoffreyBooth GeoffreyBooth commented Mar 15, 2020 •
edited
Loading

Backport of #31415. Thank you @codebytere for the prompt!

This isn't as clean as it could be because of 04d07ed. @bmeck or @guybedford do you mind double-checking this? I'm assuming that we don't want to merge into 12 the commit that was reverted.
@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. v12.x labels Mar 15, 2020
@GeoffreyBooth GeoffreyBooth force-pushed the backport-31415-to-12.x-staging branch from 4d3b533 to 8e8b655 Compare March 15, 2020 05:37
@GeoffreyBooth GeoffreyBooth requested a review from bmeck March 15, 2020 05:37
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 15, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29844/
@GeoffreyBooth

This comment has been minimized.

Sign in to view
@GeoffreyBooth GeoffreyBooth force-pushed the backport-31415-to-12.x-staging branch from 8e8b655 to e1a4de2 Compare March 15, 2020 06:12
@GeoffreyBooth GeoffreyBooth force-pushed the backport-31415-to-12.x-staging branch from e1a4de2 to 287ef5f Compare March 15, 2020 06:18
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 15, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29845/
@puzpuzpuz puzpuzpuz mentioned this pull request Mar 15, 2020
Closed
4 tasks
@puzpuzpuz
Copy link
Copy Markdown
Member

puzpuzpuz commented Mar 15, 2020

This PR doesn't edit repl.md. Is v12.x-staging in a broken state?

I've created #32282 to deal with that.
@GeoffreyBooth GeoffreyBooth force-pushed the backport-31415-to-12.x-staging branch from 287ef5f to 4ee0478 Compare March 16, 2020 03:15
@codebytere

This comment has been minimized.

Sign in to view
@GeoffreyBooth GeoffreyBooth force-pushed the backport-31415-to-12.x-staging branch from 4ee0478 to 3f31b1a Compare March 17, 2020 03:58
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 17, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29880/
@GeoffreyBooth GeoffreyBooth force-pushed the backport-31415-to-12.x-staging branch from 3f31b1a to bb4fbbe Compare March 18, 2020 03:16
@GeoffreyBooth

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 19, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29927/
@codebytere

This comment has been minimized.

Sign in to view
@guybedford guybedford force-pushed the backport-31415-to-12.x-staging branch from bb4fbbe to 3e13dc7 Compare March 23, 2020 01:34
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 23, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29989/
@GeoffreyBooth GeoffreyBooth force-pushed the backport-31415-to-12.x-staging branch from 009d0a2 to 0da01a7 Compare March 24, 2020 22:23
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 24, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30039/
@GeoffreyBooth
Copy link
Copy Markdown
Member Author

GeoffreyBooth commented Mar 24, 2020 •
edited
Loading

@codebytere I've been struggling with this for over a week now and I just can't get CI to pass.

From what I can tell of the CI jobs that are failing, like node-test-commit-aix, it appears that make test-ci and/or make run-ci are failing:

18:11:37 gmake[1]: *** [Makefile:529: test-ci] Error 1
18:11:37 gmake: *** [Makefile:557: run-ci] Error 2

When I run locally, I get this error with make run-ci:

not ok 636 parallel/test-fs-stat-bigint
  ---
  duration_ms: 0.248
  severity: fail
  exitcode: 1
  stack: |-
    assert.js:385
        throw err;
        ^

    AssertionError [ERR_ASSERTION]: Number version atimeMs = 1585172518639.4456, BigInt version atimeMs = 1585172518625n, Allowable delta = 14
        at verifyStats (/Users/geoffrey/Sites/node/test/parallel/test-fs-stat-bigint.js:71:7)

Perhaps we need to increase the allowable delta here?
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 25, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30070/
@GeoffreyBooth GeoffreyBooth mentioned this pull request Mar 25, 2020
Closed
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30079/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30097/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30101/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30102/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30103/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30105/
@GeoffreyBooth
Copy link
Copy Markdown
Member Author

GeoffreyBooth commented Mar 26, 2020

@codebytere I keep resuming CI, but node-test-commit-aix keeps failing. It fails for v12.x-staging too: https://ci.nodejs.org/job/node-test-commit/36866/.

Can we land this, since the problem appears to be in the staging branch?
@MylesBorins
Copy link
Copy Markdown
Contributor

MylesBorins commented Mar 26, 2020

As the failure seems unrelated to this PR and on the staging branch I think this is ok to land

@codebytere thoughts?
MylesBorins
MylesBorins approved these changes Mar 26, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30107/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 26, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30108/
@codebytere codebytere force-pushed the v12.x-staging branch 2 times, most recently from 63a03d2 to d577190 Compare March 31, 2020 23:57
@codebytere
Copy link
Copy Markdown
Member

codebytere commented Apr 1, 2020 •
edited
Loading

@GeoffreyBooth staging should be good to rebase against, then we can get this landed.
@GeoffreyBooth @guybedford
module: drop support for extensionless main entry points in esm
b6ad757 
PR-URL: nodejs#31415
Backport-PR-URL: nodejs#32280
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@guybedford guybedford force-pushed the backport-31415-to-12.x-staging branch from 0da01a7 to b6ad757 Compare April 1, 2020 02:36
@guybedford
Copy link
Copy Markdown
Contributor

guybedford commented Apr 1, 2020

I've updated the rebase here.
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 1, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30309/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 1, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30310/
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 1, 2020 •
edited by MylesBorins
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/30314/
rerun linuxone: https://ci.nodejs.org/job/node-test-commit-linuxone/19987/
MylesBorins pushed a commit that referenced this pull request Apr 1, 2020
@GeoffreyBooth @MylesBorins
module: drop support for extensionless main entry points in esm
4ee41c5 
Backport-PR-URL: #32280
PR-URL: #31415
Backport-PR-URL: #32280
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins
Copy link
Copy Markdown
Contributor

MylesBorins commented Apr 1, 2020

finally green!

Landed in 4ee41c5
@MylesBorins MylesBorins closed this Apr 1, 2020
@GeoffreyBooth GeoffreyBooth deleted the backport-31415-to-12.x-staging branch April 1, 2020 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

esm Issues and PRs related to the ECMAScript Modules implementation.

8 participants

@GeoffreyBooth @nodejs-github-bot @puzpuzpuz @codebytere @MylesBorins @guybedford @bmeck @awibox