Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: fix compatibility with V8's depot_tools #57330

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

richardlau
Copy link
Member

Recent changes to depot_tools's ninja.py proxy is causing infinite recursion in our V8 CI builds as we checkout depot_tools into a directory with a leading _ (i.e. _depot_tools) and the proxy now checks for an exact match (i.e. == "depot_tools") instead of endswith("depot_tools").

Rename our checkout to depot_tools (without the leading _) so the ninja.py proxy can exclude it when reinvoking ninja.

Refs: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6259139
Fixes: nodejs/build#4027


cc @nodejs/v8-update @targos

Recent changes to `depot_tools`'s `ninja.py` proxy is causing infinite
recursion in our V8 CI builds as we checkout `depot_tools` into a
directory with a leading `_` (i.e. `_depot_tools`) and the proxy now
checks for an exact match (i.e. `== "depot_tools"`) instead of
`endswith("depot_tools")`.

Rename our checkout to `depot_tools` (without the leading `_`) so the
`ninja.py` proxy can exclude it when reinvoking `ninja`.
@richardlau richardlau added lts-watch-v18.x PRs that may need to be released in v18.x. lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x labels Mar 5, 2025
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. labels Mar 5, 2025
@richardlau

This comment was marked as outdated.

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 5, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 5, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 5, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 7, 2025
@nodejs-github-bot nodejs-github-bot merged commit abd73d8 into nodejs:main Mar 7, 2025
85 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in abd73d8

@richardlau richardlau deleted the depot_tools branch March 7, 2025 15:45
marco-ippolito pushed a commit that referenced this pull request Mar 7, 2025
Recent changes to `depot_tools`'s `ninja.py` proxy is causing infinite
recursion in our V8 CI builds as we checkout `depot_tools` into a
directory with a leading `_` (i.e. `_depot_tools`) and the proxy now
checks for an exact match (i.e. `== "depot_tools"`) instead of
`endswith("depot_tools")`.

Rename our checkout to `depot_tools` (without the leading `_`) so the
`ninja.py` proxy can exclude it when reinvoking `ninja`.

PR-URL: #57330
Fixes: nodejs/build#4027
Refs: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6259139
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
aduh95 pushed a commit that referenced this pull request Mar 9, 2025
Recent changes to `depot_tools`'s `ninja.py` proxy is causing infinite
recursion in our V8 CI builds as we checkout `depot_tools` into a
directory with a leading `_` (i.e. `_depot_tools`) and the proxy now
checks for an exact match (i.e. `== "depot_tools"`) instead of
`endswith("depot_tools")`.

Rename our checkout to `depot_tools` (without the leading `_`) so the
`ninja.py` proxy can exclude it when reinvoking `ninja`.

PR-URL: #57330
Fixes: nodejs/build#4027
Refs: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6259139
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
aduh95 pushed a commit that referenced this pull request Mar 9, 2025
Recent changes to `depot_tools`'s `ninja.py` proxy is causing infinite
recursion in our V8 CI builds as we checkout `depot_tools` into a
directory with a leading `_` (i.e. `_depot_tools`) and the proxy now
checks for an exact match (i.e. `== "depot_tools"`) instead of
`endswith("depot_tools")`.

Rename our checkout to `depot_tools` (without the leading `_`) so the
`ninja.py` proxy can exclude it when reinvoking `ninja`.

PR-URL: #57330
Fixes: nodejs/build#4027
Refs: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6259139
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lts-watch-v18.x PRs that may need to be released in v18.x. lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x64 V8 builds run out of memory
7 participants