Commit 9d9df70
committed
Merge pull request #6128 from koalaman:master
Hi, I came across [this shell function](https://github.com/microsoft/ChakraCore/blob/68d19b73e4c796540d2d44cc17ac2320c7576cd0/test/native-tests/test_native.sh#L30) in `test/native-tests/test_native.sh`:
```
SAFE_RUN() {
local SF_RETURN_VALUE=$($1 2>&1)
if [[ $? != 0 ]]; then
>&2 echo $SF_RETURN_VALUE
exit 1
fi
}
```
invoked as e.g.
```
SAFE_RUN `cd $TEST_PATH; ${CH_DIR} Platform.js > Makefile`
```
There are multiple issues that prevent it from living up to its name:
1. The code to run is executed by the backticks before the function ever starts:
```
$ test_run() { true; }
$ test_run `ls /doesnotexist`
ls: cannot access '/doesnotexist': No such file or directory
```
2. If the invocation is changed to pass the command directly, the invocation fails for metacharacters like `;` and `>`. Here they are both treated as literal text:
```
$ test_run() { local var=$($1 >&2); echo "$var"; }
$ test_run "echo Hello; ls > filelist"
Hello; ls > filelist
```
3. If passed a valid invocation, `local` [masks all exit codes](https://github.com/koalaman/shellcheck/wiki/SC2155) and returns success, even when the command fails:
```
$ test_run() { local foo=$($1); echo "$1 returned $?"; }
$ test_run true
true returned 0
$ test_run false
false returned 0
```
The suggested commits fixes all these issues.
I was unable to verify the fix locally as the tests appeared to hang when run overnight with and without these fixes. I'm not quite sure how they're used, but there is some possibility that this commit uncovers failures that were previously hidden.
1 file changed
+8
-7
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
| 31 | + | |
| 32 | + | |
32 | 33 | | |
33 | 34 | | |
34 | | - | |
| 35 | + | |
35 | 36 | | |
36 | 37 | | |
37 | 38 | | |
| |||
57 | 58 | | |
58 | 59 | | |
59 | 60 | | |
60 | | - | |
| 61 | + | |
61 | 62 | | |
62 | 63 | | |
63 | 64 | | |
64 | 65 | | |
65 | 66 | | |
66 | | - | |
| 67 | + | |
67 | 68 | | |
68 | 69 | | |
69 | | - | |
| 70 | + | |
70 | 71 | | |
71 | 72 | | |
72 | 73 | | |
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
77 | | - | |
| 78 | + | |
78 | 79 | | |
79 | 80 | | |
80 | 81 | | |
| |||
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
103 | | - | |
| 104 | + | |
0 commit comments