Skip to content

Commit

Permalink
fix(run): Fix custom shell execution (yarnpkg#5851)
Browse files Browse the repository at this point in the history
**Summary**

A previous change removed the -c option that was passed to the shell, causing script like "echo foo"
to fail if script-shell was set to bash. This change cleans up the way custom shell is specified,
passing the shell to node's child process spawner instead of trying to make it part of our command
string. Doing it this way seems to account for the previously required -c flag.

fixes yarnpkg#5699 

**Test plan**

No test was added because our tests for the `yarn run` command use a mocked `execute-lifecycle-script`. Plus these tests would get run on windows too, so making a test that specified a config of `script-shell bash` would fail if it was unmocked.

Can be tested manually by creating a `.yarnrc` that contains:

```
script-shell bash
```

and adding a script like:

```
  "scripts": {
    "foo": "echo foo; echo bar; echo $SHELL"
  }
```

which should produce the output

```
$ yarn run foo
yarn run v1.6.0
$ echo foo; echo bar; echo $SHELL
foo
bar
/bin/bash
✨  Done in 0.15s.
```
  • Loading branch information
rally25rs authored and BYK committed May 22, 2018
1 parent cde7566 commit c506764
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions src/util/execute-lifecycle-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,8 @@ export async function executeLifecycleScript({
detached = false;
}

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

return {cwd, command: cmd, stdout};
}
Expand Down

0 comments on commit c506764

Please sign in to comment.