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

End of CLI options -- is now part of process.execArgv (regression in Node 10.10) #24647

Closed
demurgos opened this issue Nov 25, 2018 · 2 comments
Closed
Assignees
Labels
cli Issues and PRs related to the Node.js command line interface. regression Issues related to regressions.

Comments

@demurgos
Copy link
Contributor

demurgos commented Nov 25, 2018

  • Version: 11.2.0
  • Platform: Linux 64 bit
  • Subsystem: src

I noticed a regression regarding -- following the refactoring in 8fd55fffee (#22392).

  • Node < 10.10:

    $ echo "console.log(process.execArgv)" > test.js
    $ node -- test.js
    []
    
  • Node >= 10.10

    $ echo "console.log(process.execArgv)" > test.js
    $ node -- test.js
    [ '--' ]
    

The double dash indicating the end of the exec args is now part of process.execArgv while it previously was neither in process.execArgv nor process.argv. This prevents from safely spawning a Node process while escaping the main module name. The following pattern to respawn itself no longer works:

const args = [...process.exevArgv, "--", ...process.argv.slice(1)];
cp.spawn(process.execPath, args);

Calling node -- foo.js results in node -- -- foo.js in Node > 10.10 (it worked fine previously).

I caught it in CI when working on spawn-wrap, a lib that heavily depends on how the node processes are spawned: https://travis-ci.org/demurgos/spawn-wrap/jobs/459508703

@targos targos added cli Issues and PRs related to the Node.js command line interface. regression Issues related to regressions. labels Nov 25, 2018
@targos
Copy link
Member

targos commented Nov 25, 2018

/cc @addaleax

demurgos added a commit to demurgos/spawn-wrap that referenced this issue Nov 25, 2018
addaleax added a commit to addaleax/node that referenced this issue Nov 26, 2018
This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

Fixes: nodejs#24647
@addaleax addaleax self-assigned this Nov 26, 2018
Trott pushed a commit to Trott/io.js that referenced this issue Nov 29, 2018
This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

Fixes: nodejs#24647

PR-URL: nodejs#24654
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@Trott
Copy link
Member

Trott commented Nov 29, 2018

Fixed in 83d6cb9

@Trott Trott closed this as completed Nov 29, 2018
targos pushed a commit that referenced this issue Nov 29, 2018
This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

Fixes: #24647

PR-URL: #24654
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

Fixes: nodejs#24647

PR-URL: nodejs#24654
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

Fixes: #24647

PR-URL: #24654
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 20, 2019
This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

Fixes: #24647

PR-URL: #24654
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

Fixes: #24647

PR-URL: #24654
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 9, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 9, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 9, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 9, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 10, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 10, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 10, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 10, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 10, 2019
demurgos added a commit to demurgos/spawn-wrap that referenced this issue May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Issues and PRs related to the Node.js command line interface. regression Issues related to regressions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants