Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

Prefer --inspect-brk over --debug-brk #43

Merged
merged 1 commit into from
Apr 6, 2017
Merged

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Apr 6, 2017

Node 8.x no longer has --debug-brk and --inspect-brk is backwards
compatible with Node 7.

Ref: nodejs/node#12197.

I will be cherry-picking this on nodejs/node#11441.

lib/_inspect.js Outdated
@@ -98,7 +98,7 @@ function runScript(script, scriptArgs, inspectHost, inspectPort, childPrint) {
return new Promise((resolve) => {
const args = [
'--inspect',
Copy link
Collaborator

@jkrems jkrems Apr 6, 2017

Choose a reason for hiding this comment

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

If we make this change, should this just be --inspect-brk=${inspectPort}?

P.S.: What I mean is that we don't need to pass --inspect anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack.

@jkrems
Copy link
Collaborator

jkrems commented Apr 6, 2017

Just looked it up: This will drop support for node <7.6.0. Will publish as major for the release to npm.

@ofrobots
Copy link
Contributor Author

ofrobots commented Apr 6, 2017

Is it possible to take a dependency on semver for this module? If not, I can use regexes to avoid the major due to <7.6.0.

@jkrems
Copy link
Collaborator

jkrems commented Apr 6, 2017

I would prefer delaying the pain of getting 3rd party modules working in our bundled build (where we get away with just dumping the files into node right now). So 👍 for a regex solution even though it's not "perfectly clean".

Node 8.x no longer has --debug-brk.
@ofrobots
Copy link
Contributor Author

ofrobots commented Apr 6, 2017

Done. PTAL.

Copy link
Collaborator

@jkrems jkrems left a comment

Choose a reason for hiding this comment

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

🎉

@jkrems
Copy link
Collaborator

jkrems commented Apr 6, 2017

@jkrems jkrems changed the title prefer --inspect-brk over --debug-brk Prefer --inspect-brk over --debug-brk Apr 6, 2017
@jkrems
Copy link
Collaborator

jkrems commented Apr 6, 2017

That OSX failure looks scary but I'd blame the latest nightly rather than this change:

node::FatalException(v8::Isolate*, v8::Local<v8::Value>, v8::Local<v8::Message>) [/Users/iojs/build/workspace/node-inspect-continuous-integration/MACHINE/osx1010/node-v8.0.0-nightly20170405491d59da84-darwin-x64/bin/node]
3:  < v8::V8::ToLocalEmpty() [/Users/iojs/build/workspace/node-inspect-continuous-integration/MACHINE/osx1010/node-v8.0.0-nightly20170405491d59da84-darwin-x64/bin/node]
4: node::inspector::AgentImpl::CallAndPauseOnStart(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/iojs/build/workspace/node-inspect-continuous-integration/MACHINE/osx1010/node-v8.0.0-nightly20170405491d59da84-darwin-x64/bin/node]
5: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [/Users/iojs/build/workspace/node-inspect-continuous-integration/MACHINE/osx1010/node-v8.0.0-nightly20170405491d59da84-darwin-x64/bin/node]
6: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/iojs/build/workspace/node-inspect-continuous-integration/MACHINE/osx1010/node-v8.0.0-nightly20170405491d59da84-darwin-x64/bin/node]
7:  < v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/iojs/build/workspace/node-inspect-continuous-integration/MACHINE/osx1010/node-v8.0.0-nightly20170405491d59da84-darwin-x64/bin/node]

@jkrems jkrems merged commit 8ef696d into nodejs:master Apr 6, 2017
ofrobots added a commit to ofrobots/node that referenced this pull request Apr 12, 2017
Node 8.x no longer has --debug-brk.

Ref: nodejs/node-inspect#43
PR-URL: nodejs#11441
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: joshgav - Josh Gavant <josh.gavant@outlook.com>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants