yarn 1.6.0 breaks script-shell multiple commands

[aikar]

yarn config set script-shell bash

package.json:

"scripts": {
"foo": "echo foo; echo bar";
}

run yarn foo

expected:

yarn foo
yarn run v1.6.0
$ echo foo; echo bar
foo
bar
Done in 0.06s.

received:

yarn foo
yarn run v1.6.0
$ echo foo; echo bar
bash: echo foo; echo bar: No such file or directory
error Command failed with exit code 127.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

This worked pre 1.6.0 and helps keep stuff working consistently between windows and linux and needs to be supported.

[arcanis]

Which node version are you running ?

[aikar]

8.11.1

[arcanis]

Hm that's really weird - it seems like the command being run is "echo foo; echo bar" instead of "echo" "foo"; "echo" "bar". If it was really the case, our tests would be screaming like crazy (plus it would break even without multiple commands).

The only case I'm aware where this could happen is if you're running with an unsupported 4.X Node version (which doesn't support the shell: true option), but if that's not the case, then I have no clue ...

[sth]

That works for me as expected. Are you aware of anything that could be unusual in your setup? How did you install yarn?

No - I did test that wrong.

[aikar]

did you set the script shell to 'bash'?

Also worth noting, Linux Mint 18.3

[aikar]

and I had installed using the yarn installer script.

[rally25rs]

Has nothing to do with multiple commands. "foo": "echo foo" fails with the same error.
This looks like script-shell is just broken. _(insert rant on why script-shell is a terrible idea 😠)_

Yarn is trying to execute the command bash "echo foo" which causes the same error

~/Projects/yarn-test 🐒   bash "echo foo"
bash: echo foo: No such file or directory

This looks like it was caused by https://github.com/yarnpkg/yarn/pull/5484 @BYK

More specifically, shFlag is no longer passed to child.spawn()

-  const stdout = await child.spawn(sh, [shFlag, cmd], {cwd, env, stdio, windowsVerbatimArguments}, updateProgress);
+  const stdout = customShell
+    ? await child.spawn(customShell, [cmd], {cwd, env, stdio, windowsVerbatimArguments: true}, updateProgress)
+    : await child.spawn(cmd, [], {cwd, env, stdio, shell: true}, updateProgress);

[shFlag, cmd] became [cmd], and shFlag was set to -c. Adding that flag back fixes the behavior:

~/Projects/yarn-test 🐒   bash "echo foo; echo bar;"
bash: echo foo; echo bar;: No such file or directory

~/Projects/yarn-test 🐒   bash -c "echo foo; echo bar;"
foo
bar

---

_Edit:_

Actually, it seems to work by instead of specifying the custom shell as the command, pass it as the shell options to .spawn()

  const stdout = customShell
    ? await child.spawn(cmd, [], {cwd, env, stdio, windowsVerbatimArguments: true, shell: customShell}, updateProgress)
    : await child.spawn(cmd, [], {cwd, env, stdio, shell: true}, updateProgress);

The above code makes it work correctly for me, but only tested on OSX. I don't have access to my win machine right now to try it out in other environments with and without custom shells.

According to the node docs:

windowsVerbatimArguments <boolean> No quoting or escaping of arguments is done on Windows. Ignored on Unix. This is set to true automatically when shell is specified. Default: false.

so if specifying a shell sets it to true automatically, we may not have to specify it at all and get rid of the ternary, making the code just:

const shell = customShell || true;
await child.spawn(cmd, [], {cwd, env, stdio, shell}, updateProgress);

[BYK]

Thanks for the quick diagnosis @rally25rs!

I think we can just use the shell option. I don't know why I haven't done it that way when I was refactoring. I'll test in on Windows when I get to my computer.

[aikar]

@BYK sounds like solution is known, when will this be fixed?

[BYK]

@aikar when someone submits a PR? Would you like to be our hero today? 😉

[kostiako]

I can't make heads or tails out of yarn's codebase, otherwise would gladly fix this.
Since this is currently rendering my whole Windows env useless.

Anyone?

[rally25rs]

I created PR #5851 if you want to test it out.