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

Yarn run command doesn't wok in 1.6.0 and 1.7.0 #6086

Closed
shabith opened this issue Jul 11, 2018 · 14 comments
Closed

Yarn run command doesn't wok in 1.6.0 and 1.7.0 #6086

shabith opened this issue Jul 11, 2018 · 14 comments
Assignees
Labels

Comments

@shabith
Copy link

shabith commented Jul 11, 2018

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
yarn run {command} is not working properly in yarn version 1.6.0 and 1.7.0

If the current behavior is a bug, please provide the steps to reproduce.
I tried to execute yarn run script in last 3 version of yarn and realize that last two version gives 2 different errors and it only works in yarn 1.5.1

Please have a look at below screenshot to
Version 1.7.0 - not working.

Version 1.7.0

Version 1.6.0 - not working.

Version 1.6.0

Version 1.5.1 - working.

Version 1.5.1

repo to reproduce the error - https://github.com/shabith/yarn-run-bug

What is the expected behavior?
yarn run {command} to work in latest version.

Please mention your node.js, yarn and operating system version.
git version - 2.16.2.windows.1
node version - 8.11.2
OS - Windows 10
terminal - Git bash
Yarn installed via choco without node.js - choco install yarn --ignore-dependecies

@ghost ghost assigned kaylie-alexa Jul 11, 2018
@ghost ghost added the triaged label Jul 11, 2018
@tkamenoko
Copy link

I have same issue on yarn 1.7.0, using powershell as script-shell.
I got this error message when trying yarn run test (but npm run test works fine).

The argument '/d' is not recognized as the name of a script file. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

In my opinion, child.spawn causes this error.
First, execCommand is called in cli/commands/run.js

await execCommand({
stage,
config,
cmd: cmdWithArgs,
cwd: config.cwd,
isInteractive: true,
customShell: customShell ? String(customShell) : undefined,
});

...and execCommand comes from util/execute-lifecycle-script.js, and calls executeLifecycleScript inside.

export async function execCommand({
stage,
config,
cmd,
cwd,
isInteractive,
customShell,
}: {
stage: string,
config: Config,
cmd: string,
cwd: string,
isInteractive: boolean,
customShell?: string,
}): Promise<void> {
const {reporter} = config;
try {
reporter.command(cmd);
await executeLifecycleScript({stage, config, cwd, cmd, isInteractive, customShell});

Then, executeLifecycleScript calls child.spawn from util/child.js.

const stdout = await child.spawn(cmd, [], {cwd, env, stdio, detached, shell}, onProgress);

yarn/src/util/child.js

Lines 61 to 74 in 1711e72

export function spawn(
program: string,
args: Array<string>,
opts?: child_process$spawnOpts & {detached?: boolean, process?: ProcessFn} = {},
onData?: (chunk: Buffer | string) => void,
): Promise<string> {
const key = opts.cwd || String(++uid);
return queue.push(
key,
(): Promise<string> =>
new Promise((resolve, reject) => {
const proc = child.spawn(program, args, opts);
spawnedProcesses[key] = proc;

This spawn uses Node.js standard module child_process.spawn to call shell commands, but this function uses command line option /d /s /c on Windows.
/d /s /c works fine only on cmd.exe, so the error occurs.

@borekb
Copy link

borekb commented Jul 28, 2018

I just did the same digging as @tkamenoko and came to the same conclusion. Initially, I thought that Yarn is not working in Git Bash (or MSYS2 mingw shell in my case) but it turned out to be the problem with script-shell.

In my case:

# In .npmrc: script-shell="bash.exe"

$ yarn run example
yarn run v1.9.2
$ echo example
/d: /d: Is a directory
error Command failed with exit code 126.

I tried putting this into .npmrc:

script-shell="bash.exe -c"

which probably isn't how script-shell should be used but I wanted to try it anyway. The error then was:

$ yarn run example
yarn run v1.9.2
$ echo example
error Couldn't find the binary echo example

@shabith
Copy link
Author

shabith commented Jul 29, 2018

Thanks @tkamenoko and @borekb for investigating on this.

@tkamenoko But when i run in the cmd.exe I'm getting below error.

error Couldn't find the binary cowsay 'is it working??'

I check on one of my colleague machine and it's working fine on 1.7.0. I have a hunch that it is related to my profile name which has white space "Shabith Ishan" because that is the first noticeable different I saw with his setup.

@borekb
Copy link

borekb commented Jul 29, 2018

@shabith Do you mean you run the command in cmd.exe, or that cmd.exe is set as your script-shell? With cmd.exe as a script shell, all is fine on my computer with Yarn 1.9.2.

@shabith
Copy link
Author

shabith commented Jul 30, 2018

@borekb I meant once I run the command in cmd.exe. script-shell is bash all the time. Let me check with yarn 1.9.2

@shabith
Copy link
Author

shabith commented Jul 30, 2018

no luck with Yarn 1.9.2 as well 😞 Same behavior for me.

@rally25rs
Copy link
Contributor

rally25rs commented Sep 20, 2018

After some research, it seems that this is actually an issue with node itself and they way it tries to spawn a command on windows when specifying bash as the shell.

Yarn uses node's child_process.spawn

You can reproduce this without yarn at all by creating a file test.js

const { spawn } = require('child_process');
const ls = spawn('echo "hello"', [], {shell: 'bash'});

ls.stdout.on('data', (data) => {
  console.log(`stdout: ${data}`);
});

ls.stderr.on('data', (data) => {
  console.log(`stderr: ${data}`);
});

ls.on('close', (code) => {
  console.log(`child process exited with code ${code}`);
});

and run it while in mingw (git-bash) with:

$ node ./test.js
stderr: /d: /d: Is a directory

child process exited with code 126

I suspect this is because node is trying to run cmd because you are on windows, but you are actually in mingw which interprets the /d being passed to cmd to spawn bash as a directory.

You could just remove shell-script from .yarnrc and that should fix the issue.

The way we spawn scripts in yarn changed around 1.6.0 to fix various other issues on windows, so changing it back would reintroduce other issues. We hoped node would have had this kind of thing covered :(


edit:

This is the node.js source for spawn():

  if (options.shell) {
    const command = [file].concat(args).join(' ');

    if (process.platform === 'win32') {
      if (typeof options.shell === 'string')
        file = options.shell;
      else
        file = process.env.comspec || 'cmd.exe';
      args = ['/d', '/s', '/c', `"${command}"`];
      options.windowsVerbatimArguments = true;

So when there is a shell-script specified in .yarnrc, it passes that as the shell option to spawn.
It looks like whenever a shell is specified and process.platform is win32, it will always tack on ['/d', '/s', '/c', in the arguments.
There is probably no way for yarn to tell that it's on windows in a unix-style shell and shouldn't do that.

@shabith
Copy link
Author

shabith commented Sep 21, 2018

Thanks for investigation on this. I checked for yarn configuration using yarn config get script-shell and I got it as undefined then I checked with npm config get script-shell and got to know that bash was set over there. once I delete that it starts to work again in both cmd/gitbash using both npm and yarn. 😄 🍾

Like you have explained it seems like issue is with node itself. Once again thanks for help on this. 🙏

EDIT
It need to bescript-shell not shell-script 🤒

@borekb
Copy link

borekb commented Sep 21, 2018

The issue is probably fixed by nodejs/node#21943. As it's tagged semver-major, it will probably come with Node v11 which should be out 23th October according to this page. Can't wait :)

@shabith
Copy link
Author

shabith commented Sep 21, 2018

That's a good news @borekb
Thanks @tkamenoko for your hard work on this. 🙏 ✨

@ackvf
Copy link

ackvf commented Oct 8, 2018

@shabith BTW, shouldn't it be script-shell?? https://docs.npmjs.com/misc/config#script-shell

@shabith
Copy link
Author

shabith commented Oct 9, 2018

@ackvf you are right! sorry I got that mixed up because of the excitement. Let me edit that. thanks for point out @ackvf!

@whatsdis
Copy link

removed shell-script=bash from .npmrc and it works fine! thanks guys, im writing this comment in case I need to find this thread again.

@VukiMusician
Copy link

Couldn't find the binary yarn ts-node inquirer.ts ? Help :(

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

No branches or pull requests

8 participants