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: --without-ssl and --without-intl imply new limitation (no debugger) #12758

Closed
refack opened this issue Apr 30, 2017 · 16 comments
Closed
Labels
build Issues and PRs related to build files or the CI. inspector Issues and PRs related to the V8 inspector protocol openssl Issues and PRs related to the OpenSSL dependency. regression Issues related to regressions.

Comments

@refack
Copy link
Contributor

refack commented Apr 30, 2017

  • Version: > 6
  • Platform: All
  • Subsystem: inspector, ssl

Since #12197 (i.e. node8) when node is built --without-ssl or --without-intl inspector is not available. As a consequence node is built with no debugger interface. configure does not warn about this (new) consequence.

Since in previous versions these switches did not remove all debugger interface, this is a regression in the build behaviour.

Ref: #12757

@refack refack added inspector Issues and PRs related to the V8 inspector protocol regression Issues related to regressions. labels Apr 30, 2017
@refack
Copy link
Contributor Author

refack commented Apr 30, 2017

Marked regression since when node8 is built --without-ssl it will not have any kind of debugger interface. and this a new regressive behaviour for the --without-ssl switch

@refack
Copy link
Contributor Author

refack commented Apr 30, 2017

@addaleax correct me if I'm wrong but the debug protocol was independent of the ssl capabilities of the binary, and AFAIK inspect is. Thus, no ssl -> no debugger -> regression.

@addaleax
Copy link
Member

@refack Yeah, I see where you’re coming from, but I wouldn’t say it is a regression. This is related to features we’re intentionally adding and removing, there’s just no way we can avoid any incompatibilities.

@refack
Copy link
Contributor Author

refack commented Apr 30, 2017

could we build a proxy?

@refack refack added regression Issues related to regressions. and removed regression Issues related to regressions. labels Apr 30, 2017
@refack
Copy link
Contributor Author

refack commented Apr 30, 2017

I thought about it, and then again, since this is an unintended consequence of the --without-ssl switch, IMHO even a strongly worded warning would solve this issue.

@addaleax you are right in that this not an inspector bug but IMHO it is a build regression.

@refack refack changed the title inspector: unavailable when built --without-ssl build: --without-ssl has unintended consequences (no debugger) Apr 30, 2017
@refack refack added build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. labels Apr 30, 2017
refack added a commit to refack/node that referenced this issue May 1, 2017
* To fix nodejs#12758, added an assertion that if
  `--without-ssl` or `--without-intl` are set, --without-debugger must
  also be explicitly set.

* Added `--without-debugger` is an alias to `--without-inspector`, since
  we removed the old TCP `debug` protocol the V8 `inspect` protocol is the
  only debugger available. Hence disabling `inspector` means disabling
  any kind on JS debugger.

Fixes: nodejs#12758
@sam-github
Copy link
Contributor

Doesn't sound like a bug to me. Lots of stuff disappears when SSL isn't built in (crypto, https, ...). Building without OpenSSL is a fringe build, for special purposes.

@refack
Copy link
Contributor Author

refack commented May 7, 2017

Doesn't sound like a bug to me. Lots of stuff disappears when SSL isn't built in (crypto, https, ...). Building without OpenSSL is a fringe build, for special purposes.

This is a regression in the build behaviour, since it's a new consequence of a feature that disappears.
My solution was just to require explicitly to configure --without-debugger #12768

@sam-github
Copy link
Contributor

@refack I don't really follow, sorry.

Are you saying that --without-ssl used to build node with the Inspector, and now it doesn't?

Or are you saying that --without-ssl disables the Inspector and leave the Debugger so node could be generally said to "support debugging", and now that there is no Debugger, node can generally be said to "not support debugging"?

@refack
Copy link
Contributor Author

refack commented May 8, 2017

Or are you saying that --without-ssl disables the Inspector and leave the Debugger so node could be generally said to "support debugging", and now that there is no Debugger, node can generally be said to "not support debugging"?

Yes.
i.e. if you build v7 --without-ssl you can still debug, but ever since #12197 if you build --without-ssl you have no debugger interface (neither debug or inspect protocols). That's a new loss of an ability.

My proposed solution is to require the builder to additionally explicitly specify --without-debugger (or --without-inspector) when calling ./configure

@refack refack changed the title build: --without-ssl has unintended consequences (no debugger) build: --without-ssl and --without-intl imply new limitation (no debugger) May 8, 2017
@refack
Copy link
Contributor Author

refack commented May 8, 2017

Apparently --without-intl has the same effect. Title and first comment updated.

@sam-github
Copy link
Contributor

I don't see the regression. Losing the Debugger is obviously backwards incompatible/semver-major (and was treated as such). Also, forcing people to type two flags instead of one is an annoyance rather than being helpful.

Unless you are saying that its possible to not have SSL, but to have the Inspector? You are decoupling that? If not, I'm not sure what problem is being solved here.

@refack
Copy link
Contributor Author

refack commented May 8, 2017

Unless you are saying that it's possible to not have SSL, but to have the Inspector? You are decoupling that? If not, I'm not sure what problem is being solved here.

I was hoping that's possible. And I'm looking into it, but that's a bigger issue.

Also, forcing people to type two flags instead of one is an annoyance rather than being helpful.

IMHO building a new (even semver-major) version and finding out you lost a previously available (important) ability, without any warning, is worse.
I see it a bit like the experimental (#12723) discussion. The user should explicitly opt-in to new behaviour, that way we minimize surprises.

Another solution would be to emit a warning, but I fear it'll be drowned in all sort of script outputs.

@sam-github
Copy link
Contributor

There was a fair amount of warning that the Debugger was going away. And since there is no way to opt-out of the new behaviour, talking about opt-in vs opt-out doesn't seem to apply. And failing to disable SSL when configuring explicitly --without-ssl is in itself a step backwards and a loss of a feature. IMO, the pain exceeds the arguable gain.

@refack
Copy link
Contributor Author

refack commented May 26, 2017

Addressed by #12978

@refack refack closed this as completed May 26, 2017
@sparkleholic
Copy link

Is there any reason why --without-intl disable debugger?

@bnoordhuis
Copy link
Member

@sparkleholic --without-intl disables ICU. The debugger uses ICU.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. inspector Issues and PRs related to the V8 inspector protocol openssl Issues and PRs related to the OpenSSL dependency. regression Issues related to regressions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants