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.

Sign up for GitHub

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

Jump to bottom

NODE_OPTIONS do not work properly anymore #22526

Closed
BridgeAR opened this issue Aug 25, 2018 · 2 comments
Closed

NODE_OPTIONS do not work properly anymore #22526

BridgeAR opened this issue Aug 25, 2018 · 2 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. cli Issues and PRs related to the Node.js command line interface.

Comments

@BridgeAR
Copy link
Member

Seems like #22392 broke NODE_OPTIONS parsing.

Example:

> NODE_OPTIONS='-r logger.js --trace-events-enabled --trace-event-categories node.async_hooks' ./node

./node: --trace-events-enabled --trace-even is not allowed in NODE_OPTIONS

> NODE_OPTIONS='--trace-events-enabled --trace-event-categories node.async_hooks' ./node

./node: --trace-event-categories node.async_hooks is not allowed in NODE_OPTIONS

cc @addaleax
@BridgeAR BridgeAR added the c++ Issues and PRs that require attention from people who are familiar with C++. label Aug 25, 2018
@mscdex
Copy link
Contributor

mscdex commented Aug 25, 2018

We don't have tests for this?

@refack refack added the cli Issues and PRs related to the Node.js command line interface. label Aug 25, 2018
addaleax added a commit to addaleax/node that referenced this issue Aug 26, 2018
@addaleax
src: fix NODE_OPTIONS parsing bug
c533499 
I, uhm, might have messed up by using a `substr(start, end)`
signature when `std::string` actually uses `substr(start, len)`.
Fix that.

Fixes: nodejs#22526
Refs: nodejs#22392
@addaleax addaleax mentioned this issue Aug 26, 2018
Closed
3 tasks
@addaleax
Copy link
Member

@BridgeAR Fix + tests in #22529, would be cool if you could take a look? :)

addaleax added a commit that referenced this issue Aug 28, 2018
@addaleax
src: fix NODE_OPTIONS parsing bug
7e3ceb1 
I, uhm, might have messed up by using a `substr(start, end)`
signature when `std::string` actually uses `substr(start, len)`.
Fix that.

Fixes: #22526
Refs: #22392

PR-URL: #22529
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this issue Sep 3, 2018
@addaleax @targos
src: fix NODE_OPTIONS parsing bug
2c5dfef 
I, uhm, might have messed up by using a `substr(start, end)`
signature when `std::string` actually uses `substr(start, len)`.
Fix that.

Fixes: #22526
Refs: #22392

PR-URL: #22529
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this issue Sep 6, 2018
@addaleax @targos
src: fix NODE_OPTIONS parsing bug
7282768 
I, uhm, might have messed up by using a `substr(start, end)`
signature when `std::string` actually uses `substr(start, len)`.
Fix that.

Fixes: #22526
Refs: #22392

PR-URL: #22529
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. cli Issues and PRs related to the Node.js command line interface.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

src: fix NODE_OPTIONS parsing bug addaleax/node
4 participants
@mscdex @refack @addaleax @BridgeAR