inspector: roll inspector_protocol to match v8's #56649

legendecas · 2025-01-17T23:57:45Z

tools: update inspector_protocol roller

Fix the inspector_protocol/roll.py to fit node source directory
structure.

roll.py reads the deps/v8/thrid_party/inspector_protocol/README.v8
to get the revision of the inspector_protocol that V8 depends on, and
updates the local version to match.

V8's copy of inspector_protocol modifies the namespace of crdtp
library and does not export the symbols. So it can not be used outside
of V8.

inspector: roll inspector_protocol

Roll the inspector_protocol library to match V8's inspector_protocol
revision.

Update the node inspector to use the new crdtp protocol library.

deps: move inspector_protocol to deps

The crdtp library in the inspector_protocol is compiled as a library
and linked to the node executable.

nodejs-github-bot · 2025-01-17T23:57:50Z

Review requested:

@nodejs/security-wg
@nodejs/inspector
@nodejs/cpp-reviewers

codecov · 2025-01-18T01:35:30Z

Codecov Report

Attention: Patch coverage is 60.65574% with 72 lines in your changes missing coverage. Please review.

Project coverage is 89.20%. Comparing base (d978610) to head (afcb230).
Report is 40 commits behind head on main.

Files with missing lines	Patch %	Lines
src/inspector/node_json.cc	48.95%	39 Missing and 10 partials ⚠️
src/inspector/node_string.cc	59.25%	10 Missing and 1 partial ⚠️
src/inspector/tracing_agent.cc	61.53%	4 Missing and 1 partial ⚠️
src/inspector_agent.cc	81.48%	4 Missing and 1 partial ⚠️
src/inspector/network_agent.cc	87.50%	0 Missing and 1 partial ⚠️
src/inspector/worker_agent.cc	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #56649      +/-   ##
==========================================
- Coverage   89.21%   89.20%   -0.02%     
==========================================
  Files         662      663       +1     
  Lines      191945   192180     +235     
  Branches    36948    36988      +40     
==========================================
+ Hits       171238   171425     +187     
- Misses      13549    13573      +24     
- Partials     7158     7182      +24

Files with missing lines	Coverage Δ
src/inspector/node_string.h	`57.14% <100.00%> (-16.20%)`	⬇️
src/inspector/runtime_agent.cc	`100.00% <100.00%> (ø)`
src/inspector/network_agent.cc	`94.89% <87.50%> (+0.21%)`	⬆️
src/inspector/worker_agent.cc	`90.36% <80.00%> (ø)`
src/inspector/tracing_agent.cc	`89.47% <61.53%> (-1.44%)`	⬇️
src/inspector_agent.cc	`79.74% <81.48%> (-0.40%)`	⬇️
src/inspector/node_string.cc	`60.00% <59.25%> (+4.82%)`	⬆️
src/inspector/node_json.cc	`48.95% <48.95%> (ø)`

... and 52 files with indirect coverage changes

Fix the inspector_protocol/roll.py to fit node source directory structure. `roll.py` reads the `deps/v8/thrid_party/inspector_protocol/README.v8` to get the revision of the inspector_protocol that V8 depends on, and updates the local version to match. V8's copy of inspector_protocol modifies the namespace of `crdtp` library and does not export the symbols. So it can not be used outside of V8.

nodejs-github-bot · 2025-01-23T15:51:45Z

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

nodejs-github-bot · 2025-01-23T22:31:18Z

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

Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library.

The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable.

nodejs-github-bot · 2025-01-25T16:12:26Z

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

nodejs-github-bot · 2025-01-25T19:08:14Z

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

nodejs-github-bot · 2025-01-25T21:13:16Z

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

legendecas · 2025-01-25T21:38:47Z

Fixed CI failures on AIX/linuxone. @nodejs/inspector would you mind taking another review? Thank you!

src/inspector/node_string.cc

nodejs-github-bot · 2025-01-26T23:20:11Z

Landed in 43daa9b...0aa5874

Fix the inspector_protocol/roll.py to fit node source directory structure. `roll.py` reads the `deps/v8/thrid_party/inspector_protocol/README.v8` to get the revision of the inspector_protocol that V8 depends on, and updates the local version to match. V8's copy of inspector_protocol modifies the namespace of `crdtp` library and does not export the symbols. So it can not be used outside of V8. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Fix the inspector_protocol/roll.py to fit node source directory structure. `roll.py` reads the `deps/v8/thrid_party/inspector_protocol/README.v8` to get the revision of the inspector_protocol that V8 depends on, and updates the local version to match. V8's copy of inspector_protocol modifies the namespace of `crdtp` library and does not export the symbols. So it can not be used outside of V8. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Refs: nodejs#56649

Refs: #56649 PR-URL: #56814 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>

Fix the inspector_protocol/roll.py to fit node source directory structure. `roll.py` reads the `deps/v8/thrid_party/inspector_protocol/README.v8` to get the revision of the inspector_protocol that V8 depends on, and updates the local version to match. V8's copy of inspector_protocol modifies the namespace of `crdtp` library and does not export the symbols. So it can not be used outside of V8. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Fix the inspector_protocol/roll.py to fit node source directory structure. `roll.py` reads the `deps/v8/thrid_party/inspector_protocol/README.v8` to get the revision of the inspector_protocol that V8 depends on, and updates the local version to match. V8's copy of inspector_protocol modifies the namespace of `crdtp` library and does not export the symbols. So it can not be used outside of V8. PR-URL: nodejs#56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library. PR-URL: nodejs#56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable. PR-URL: nodejs#56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Fix the inspector_protocol/roll.py to fit node source directory structure. `roll.py` reads the `deps/v8/thrid_party/inspector_protocol/README.v8` to get the revision of the inspector_protocol that V8 depends on, and updates the local version to match. V8's copy of inspector_protocol modifies the namespace of `crdtp` library and does not export the symbols. So it can not be used outside of V8. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable. PR-URL: #56649 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Refs: #56649 PR-URL: #56814 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>

nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Jan 17, 2025

legendecas added the inspector Issues and PRs related to the V8 inspector protocol label Jan 17, 2025

legendecas marked this pull request as ready for review January 17, 2025 23:58

legendecas force-pushed the inspector/roll-protocol branch 3 times, most recently from a9069ea to be85fe3 Compare January 18, 2025 00:19

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 18, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 21, 2025

This comment was marked as outdated.

Sign in to view

legendecas force-pushed the inspector/roll-protocol branch from be85fe3 to 601a566 Compare January 23, 2025 09:48

jasnell approved these changes Jan 23, 2025

View reviewed changes

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 23, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 23, 2025

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 23, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 23, 2025

legendecas and others added 2 commits January 25, 2025 16:02

inspector: roll inspector_protocol

416b4e5

Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library.

deps: move inspector_protocol to deps

afcb230

The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable.

legendecas force-pushed the inspector/roll-protocol branch from ab3d03c to afcb230 Compare January 25, 2025 16:02

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2025

anonrig approved these changes Jan 26, 2025

View reviewed changes

src/inspector/node_string.cc Show resolved Hide resolved

src/inspector/node_string.cc Show resolved Hide resolved

legendecas added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Jan 26, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 26, 2025

nodejs-github-bot closed this Jan 26, 2025

legendecas deleted the inspector/roll-protocol branch January 26, 2025 23:48

github-actions bot mentioned this pull request Jan 28, 2025

2025-01-30, Version 23.7.0 (Current) #56800

Merged

targos mentioned this pull request Jan 29, 2025

tools: fix license-builder for inspector_protocol #56814

Merged

targos added a commit to targos/node that referenced this pull request Jan 29, 2025

tools: fix license-builder for inspector_protocol

ca83bae

Refs: nodejs#56649

nodejs-github-bot pushed a commit that referenced this pull request Jan 29, 2025

tools: fix license-builder for inspector_protocol

e46f2c1

Refs: #56649 PR-URL: #56814 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>

legendecas mentioned this pull request Jan 29, 2025

NodeWorker.sendMessageToWorker fails with multi-byte code points in JSON #50516

Closed

targos added a commit that referenced this pull request Feb 2, 2025

tools: fix license-builder for inspector_protocol

fe00411

Refs: #56649 PR-URL: #56814 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

inspector: roll inspector_protocol to match v8's #56649

inspector: roll inspector_protocol to match v8's #56649

legendecas commented Jan 17, 2025 •

edited

Loading

nodejs-github-bot commented Jan 17, 2025 •

edited by legendecas

Loading

codecov bot commented Jan 18, 2025 •

edited

Loading

This comment was marked as outdated.

nodejs-github-bot commented Jan 23, 2025

nodejs-github-bot commented Jan 23, 2025

nodejs-github-bot commented Jan 25, 2025

nodejs-github-bot commented Jan 25, 2025

nodejs-github-bot commented Jan 25, 2025

legendecas commented Jan 25, 2025

nodejs-github-bot commented Jan 26, 2025

inspector: roll inspector_protocol to match v8's #56649

inspector: roll inspector_protocol to match v8's #56649

Conversation

legendecas commented Jan 17, 2025 • edited Loading

tools: update inspector_protocol roller

inspector: roll inspector_protocol

deps: move inspector_protocol to deps

nodejs-github-bot commented Jan 17, 2025 • edited by legendecas Loading

codecov bot commented Jan 18, 2025 • edited Loading

Codecov Report

This comment was marked as outdated.

nodejs-github-bot commented Jan 23, 2025

nodejs-github-bot commented Jan 23, 2025

nodejs-github-bot commented Jan 25, 2025

nodejs-github-bot commented Jan 25, 2025

nodejs-github-bot commented Jan 25, 2025

legendecas commented Jan 25, 2025

nodejs-github-bot commented Jan 26, 2025

legendecas commented Jan 17, 2025 •

edited

Loading

nodejs-github-bot commented Jan 17, 2025 •

edited by legendecas

Loading

codecov bot commented Jan 18, 2025 •

edited

Loading