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

Does not work with Windows + Yarn #387

Closed
ehmicky opened this issue Oct 28, 2019 · 4 comments · Fixed by #388
Closed

Does not work with Windows + Yarn #387

ehmicky opened this issue Oct 28, 2019 · 4 comments · Fixed by #388
Assignees
Labels
type: bug code to address defects in shipped code

Comments

@ehmicky
Copy link
Contributor

ehmicky commented Oct 28, 2019

See #318.

Environment: Windows 10, either cmd.exe or Git Bash.

Example repository: https://github.com/talves/netlify-plugin-debug

Running yarn run netlify-build prints EBADF errors.

Running npx netlify-build works though.

@ehmicky
Copy link
Contributor Author

ehmicky commented Oct 28, 2019

Ok so this is probably one of the hardest bugs I've had to debug in a long time. This is my current progress of the understanding of that bug.

Background

@netlify/build is running plugins in child processes. Child processes communicate with parent over std3. We use std3 instead of stdout/stderr so that we can separate plugin outputs from plugin IPC.

In essence, the parent does this:

const {spawn} = require('child_process')
spawn('node', ['child.js'], {stdio: ['inherit', 'inherit', 'inherit', 'pipe']})

The child does this:

const {write} = require('fs')
write(3, 'response', console.log)

This works.

Bug with cmd.exe

However when using yarn run on Windows, the parent ends up doing this:

const {spawn} = require('child_process')
spawn('cmd.exe', ['/c', 'node', 'child.js'], {stdio: ['inherit', 'inherit', 'inherit', 'pipe']})

This results in the following error:

[Error: EBADF: bad file descriptor, write] {
  errno: -4083,
  code: 'EBADF',
  syscall: 'write'
}

This is probably because cmd.exe only forwards stdin, stdout and stderr but not other file descriptors.

Issue with cross-spawn

The reason why yarn run on Windows does this is because we use execa, which itself uses cross-spawn. cross-spawn allows running shebang files on Windows. It does it by prepending cmd.exe /c in front of any non-executable command.

cross-spawn first runs a Node.js port of the which Unix utlity (which itself is basically a hack) on the command. Here the command is node so it usually resolves to C:\Program files\nodejs\node.exe. Since .exe is executable, cross-spawn usually does not need to prepend cmd.exe.

Bug with yarn

However yarn run creates a small node.cmd file in C:\users\me\AppData\Local\Temp\yarn-ID-ID\. That file simplify forwards to the real node.exe:

@"C:\Program files\nodejs\node.exe" %*

That file exists because of Yarn Plug'n'Play and it's basically a hack as well.

When calling yarn run, this small file is run instead of C:\Program files\nodejs\node.exe. It is then prepended to the PATH environment variable so that child processes use that node wrapper as well. However since that file is a .cmd file, cross-spawn needs to prepend cmd.exe /c, which does not work with std3.

I am still investigating this issue, just wanted to report my progress.

@ehmicky
Copy link
Contributor Author

ehmicky commented Oct 28, 2019

I fixed this problem by using process.execPath which is the absolute path to node instead of the 'node' string. This short-circuits node-which and solves this bug.

Fixed at #388.

This is released by @netlify/build 0.0.35, which will be included in the next release of @netlify/cli (not done yet).

@ehmicky ehmicky reopened this Oct 28, 2019
@ehmicky
Copy link
Contributor Author

ehmicky commented Oct 29, 2019

This is fixed in the latest version of @netlify/cli (2.20.1).

@ehmicky ehmicky closed this as completed Oct 29, 2019
@talves
Copy link

talves commented Oct 29, 2019

Woot! @ehmicky all working great on my system now! Confirmed that there is no hanging any longer, also.

Great job on getting this hammered out. Nice to see this fixed in the netlify-cli also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
No open projects
Netlify Build
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants