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

Bug fix for issue #114 #124

Merged
merged 1 commit into from Jan 24, 2017
Merged

Bug fix for issue #114 #124

merged 1 commit into from Jan 24, 2017

Conversation

sudo-suhas
Copy link
Collaborator

@sudo-suhas sudo-suhas commented Jan 23, 2017

On Windows, if the same execaOptions are re-used for running 'linters' in runScripts.js windowsVerbatimArguments: true options will be added which breaks the git add command if present with following error:

{ Error: spawn undefined ENOENT
    at notFoundError (E:\workspace\odyssey\node_modules\cross-spawn\lib\enoent.js:11:11)
    at verifyENOENT (E:\workspace\odyssey\node_modules\cross-spawn\lib\enoent.js:46:16)
    at ChildProcess.cp.emit (E:\workspace\odyssey\node_modules\cross-spawn\lib\enoent.js:33:19)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:215:12)
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn undefined',
  killed: false,
  stdout: '',
  stderr: 'git: \'Files\\Git\\cmd\\git.EXE\' is not a git command. See \'git --help\'.\n',
  failed: true,
  signal: null,
  cmd: 'C:\\Program Files\\Git\\cmd\\git.EXE add -- E:\\workspace\\odyssey\\build\\webpack.base.conf.js',
  timedOut: false }

Error is reproducible using this simple script:

'use strict';

const execa = require('execa');

const res = {
    bin: 'C:\\Program Files\\Git\\cmd\\git.EXE',
    args: [
        'add',
        '--',
        'E:\\workspace\\odyssey\\build\\webpack.base.conf.js'
    ]
};

execa(res.bin, res.args, {
        cwd: 'E:\\workspace\\odyssey',
        windowsVerbatimArguments: true
    })
    .then(() => {
        console.log('worked!');
    })
    .catch(err => {
        console.log(err);
    });

If the same `execaOptions` is used for running 'linters' in
[runScripts.js](/okonet/lint-staged/blob/master/src/runScript.js),
option `windowsVerbatimArguments: true` gets introduced which breaks the `git add` command if present
with following error:
```
{ Error: spawn undefined ENOENT
    at notFoundError (E:\workspace\odyssey\node_modules\cross-spawn\lib\enoent.js:11:11)
    at verifyENOENT (E:\workspace\odyssey\node_modules\cross-spawn\lib\enoent.js:46:16)
    at ChildProcess.cp.emit (E:\workspace\odyssey\node_modules\cross-spawn\lib\enoent.js:33:19)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:215:12)
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn undefined',
  killed: false,
  stdout: '',
  stderr: 'git: \'Files\\Git\\cmd\\git.EXE\' is not a git command. See \'git --help\'.\n',
  failed: true,
  signal: null,
  cmd: 'C:\\Program Files\\Git\\cmd\\git.EXE add -- E:\\workspace\\odyssey\\build\\webpack.base.conf.js',
  timedOut: false }
```
@okonet
Copy link
Collaborator

okonet commented Jan 23, 2017

Hey @sudo-suhas! Thanks for the effort and this PR. I'm still not sure how is re-using of execaOptions object can lead to windowsVerbatimArguments: true in it. Is this something OS-related?

@sudo-suhas
Copy link
Collaborator Author

@okonet I don't know enough about execa to answer that. Sorry. Since it says Windows in the option name, it must be due to windows OS. If there is some test you want me to run, I can.

@okonet
Copy link
Collaborator

okonet commented Jan 24, 2017

In the example you mentioned you've added windowsVerbatimArguments: true to the options manually. We don't do that in lint-staged. I'm wondering how is the fix works.

@sudo-suhas
Copy link
Collaborator Author

sudo-suhas commented Jan 24, 2017

I simply logged the execaOptions before each call. I saw that the windowsVerbatimArguments: true parameter was being added to the options. The obvious conclusion is that execa introduces it for internal use. But when we pass the same options to the git call, it breaks. Which was why I showed the test code with that argument.

@okonet okonet merged commit be8aa3f into lint-staged:master Jan 24, 2017
@okonet
Copy link
Collaborator

okonet commented Jan 24, 2017

Awesome! Thanks for the fix! It will be released automatically soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants