GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [v20.x] deps: V8: remove references to non-existent flags by richardlau · Pull Request #52256 · nodejs/node

Conversation

@richardlau

@richardlau richardlau commented Mar 28, 2024

Copy link
Copy Markdown
Member

#49703 cherry-picked a commit from upstream V8 which references some maglev-related flags that are not present in the version of V8 in Node.js 20.


V8 CI on x64 is currently broken, e.g. https://ci.nodejs.org/job/node-test-commit-v8-linux/5892/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/console

16:38:36 ../../src/flags/flags.cc:590:33: error: no member named 'maglev_deopt_data_on_background' in 'v8::internal::FlagValues'
16:38:36         flag.PointsTo(&v8_flags.maglev_deopt_data_on_background) ||
16:38:36                        ~~~~~~~~ ^
16:38:36 ../../src/flags/flags.cc:591:33: error: no member named 'maglev_build_code_on_background' in 'v8::internal::FlagValues'
16:38:36         flag.PointsTo(&v8_flags.maglev_build_code_on_background) ||
16:38:36                        ~~~~~~~~ ^
16:38:36 2 errors generated.

This comes from #49703 which went out in Node.js 20.8.0 and was presumably missed because the x64 V8 CI has been unreliable due to networking issues on the machine. The V8 CI has been moved onto new x64 machines, which have revealed this compilation failure. (Other release lines successfully compile but are failing perf-related tests.)

Other platforms tested in the V8 CI, Linux on ppc64le and s390x, do not enable maglev so didn't surface this.

This is a fairly naive attempt to fix the compilation failure -- I don't know if there are alternate flags for the version of V8 in Node.js 20 that should be implied by --predictable. cc @joyeecheung

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot

nodejs-github-bot commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v20.x v8 engine Issues and PRs related to the V8 dependency. labels Mar 28, 2024
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 28, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 28, 2024
@nodejs-github-bot

nodejs-github-bot commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/5894/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/5894/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/5894/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joyeecheung
joyeecheung approved these changes Mar 28, 2024
View reviewed changes

@joyeecheung joyeecheung left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is just to ignore flags implied by —predictable when computing the hash of flags. If the flags are not present it’s safe to just not add code to…ignore rhem.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
lpinca
lpinca approved these changes Mar 28, 2024
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gengjiawen
gengjiawen approved these changes Mar 29, 2024
View reviewed changes
marco-ippolito
marco-ippolito approved these changes Mar 29, 2024
View reviewed changes
RafaelGSS
RafaelGSS approved these changes Mar 29, 2024
View reviewed changes
@RafaelGSS
RafaelGSS force-pushed the v20.x-staging branch from 5ff73f3 to ae243ec Compare April 3, 2024 13:49
@RafaelGSS
RafaelGSS force-pushed the v20.x-staging branch from ae243ec to 32014f5 Compare April 10, 2024 16:18
@richardlau
deps: V8: remove references to non-existent flags
eb7d9b7
nodejs#49703 cherry-picked a commit from
upstream V8 which references some maglev-related flags that are not
present in the version of V8 in Node.js 20.
@richardlau
richardlau force-pushed the maglevflags branch from 15291e7 to eb7d9b7 Compare April 10, 2024 19:07
@richardlau

richardlau commented Apr 10, 2024

Copy link
Copy Markdown
Member Author

Rebased

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Apr 10, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Apr 10, 2024
@nodejs-github-bot

nodejs-github-bot commented Apr 10, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Apr 10, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/5912/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Apr 10, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/5912/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Apr 10, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/5912/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

richardlau added a commit that referenced this pull request Apr 17, 2024
@richardlau
deps: V8: remove references to non-existent flags
ecadd63
#49703 cherry-picked a commit from
upstream V8 which references some maglev-related flags that are not
present in the version of V8 in Node.js 20.

PR-URL: #52256
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
@richardlau

richardlau commented Apr 17, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Landed in ecadd63.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@richardlau richardlau closed this Apr 17, 2024
@richardlau
richardlau deleted the maglevflags branch April 17, 2024 11:54
@marco-ippolito marco-ippolito mentioned this pull request May 2, 2024
v20.13.0 proposal #52793
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes
@gengjiawen gengjiawen gengjiawen approved these changes
@joyeecheung joyeecheung joyeecheung approved these changes
@RafaelGSS RafaelGSS RafaelGSS approved these changes
@marco-ippolito marco-ippolito marco-ippolito approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@richardlau @nodejs-github-bot @lpinca @gengjiawen @joyeecheung @RafaelGSS @marco-ippolito
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.