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

src: allow inspector without v8 platform #30049

Closed
wants to merge 1 commit into from

Conversation

@codebytere
Copy link
Member

codebytere commented Oct 21, 2019

Related to #30029.

Embedders sometimes want to take control of V8 platform initialization themselves, so we wouldn't want to experience an error if NODE_USE_V8_PLATFORM was false necessarily. Instead, we can also gate it with HAVE_INSPECTOR to allow embedders to define that themselves and use Inspector without NODE_USE_V8_PLATFORM.

We currently patch this in ourselves, so this would help us further reduce patch surface. Additionally, if there are thoughts on a more robust way to handle this case I would welcome them!

cc @joyeecheung

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
@codebytere codebytere requested a review from joyeecheung Oct 21, 2019
src/node_options.cc Outdated Show resolved Hide resolved
@codebytere codebytere force-pushed the codebytere:no-error-inspector branch from 98561b3 to 179ab39 Oct 23, 2019
@nodejs-github-bot

This comment has been minimized.

codebytere added a commit that referenced this pull request Oct 25, 2019
Embedders sometimes want to take control of V8 platform initialization
themselves, so we wouldn't want to experience an error if
NODE_USE_V8_PLATFORM was false necessarily. Instead, we can also gate
it with HAVE_INSPECTOR to allow embedders to define that themselves
and use Inspector without NODE_USE_V8_PLATFORM.

PR-URL: #30049
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@codebytere

This comment has been minimized.

Copy link
Member Author

codebytere commented Oct 25, 2019

Landed in 11275dc

@codebytere codebytere closed this Oct 25, 2019
@codebytere codebytere deleted the codebytere:no-error-inspector branch Oct 25, 2019
targos added a commit that referenced this pull request Oct 28, 2019
Embedders sometimes want to take control of V8 platform initialization
themselves, so we wouldn't want to experience an error if
NODE_USE_V8_PLATFORM was false necessarily. Instead, we can also gate
it with HAVE_INSPECTOR to allow embedders to define that themselves
and use Inspector without NODE_USE_V8_PLATFORM.

PR-URL: #30049
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@targos targos mentioned this pull request Nov 5, 2019
targos added a commit that referenced this pull request Nov 8, 2019
Embedders sometimes want to take control of V8 platform initialization
themselves, so we wouldn't want to experience an error if
NODE_USE_V8_PLATFORM was false necessarily. Instead, we can also gate
it with HAVE_INSPECTOR to allow embedders to define that themselves
and use Inspector without NODE_USE_V8_PLATFORM.

PR-URL: #30049
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
targos added a commit that referenced this pull request Nov 10, 2019
Embedders sometimes want to take control of V8 platform initialization
themselves, so we wouldn't want to experience an error if
NODE_USE_V8_PLATFORM was false necessarily. Instead, we can also gate
it with HAVE_INSPECTOR to allow embedders to define that themselves
and use Inspector without NODE_USE_V8_PLATFORM.

PR-URL: #30049
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
targos added a commit that referenced this pull request Nov 10, 2019
Embedders sometimes want to take control of V8 platform initialization
themselves, so we wouldn't want to experience an error if
NODE_USE_V8_PLATFORM was false necessarily. Instead, we can also gate
it with HAVE_INSPECTOR to allow embedders to define that themselves
and use Inspector without NODE_USE_V8_PLATFORM.

PR-URL: #30049
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
targos added a commit that referenced this pull request Nov 11, 2019
Embedders sometimes want to take control of V8 platform initialization
themselves, so we wouldn't want to experience an error if
NODE_USE_V8_PLATFORM was false necessarily. Instead, we can also gate
it with HAVE_INSPECTOR to allow embedders to define that themselves
and use Inspector without NODE_USE_V8_PLATFORM.

PR-URL: #30049
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.