Fix minor bugs in shell scripts #10584
Conversation
… a multi-source test when testing input equivalence
| @@ -80,6 +80,6 @@ done | |||
|
|
|||
| if (($STEP != $STEPS + 1)) | |||
| then | |||
| echo "Step counter not properly adjusted!" >2 | |||
| echo "Step counter not properly adjusted!" >&2 | |||
There was a problem hiding this comment.
>2 means "write to a file called 2", not "write to stderr".
| @@ -98,7 +98,7 @@ do | |||
| NSOURCES=$((NSOURCES - 1)) | |||
| for i in $OUTPUT; | |||
| do | |||
| testImportExportEquivalence $i $OUTPUT | |||
| testImportExportEquivalence "$i" "$OUTPUT" | |||
There was a problem hiding this comment.
If $OUTPUT is not quoted, testImportExportEquivalence() gets source names as multiple separate arguments. But the function assumes that its second argument is a big string consisting of concatenated names and does not access any other arguments.
This means that we were never really testing the equivalence.
| log=--logger=JUNIT,error,$log_directory/opt_$vm.xml $testargs | ||
| log=--logger=JUNIT,error,$log_directory/opt_$vm.xml | ||
| else | ||
| log=--logger=JUNIT,error,$log_directory/noopt_$vm.xml $testargs_no_opt | ||
| log=--logger=JUNIT,error,$log_directory/noopt_$vm.xml |
There was a problem hiding this comment.
These variables were at some point present in the file and contained the --logger part. Looks like they were left behind in this command by mistake when their definitions were removed and it wasn't noticed because the command still worked (they simply evaluate to nothing now).
| @@ -206,7 +206,7 @@ function test_solc_assembly_output() | |||
| local expected_object="object \"object\" { code "${expected}" }" | |||
|
|
|||
| output=$(echo "${input}" | "$SOLC" - ${solc_args} 2>/dev/null) | |||
| empty=$(echo $output | sed -ne '/'"${expected_object}"'/p') | |||
| empty=$(echo "$output" | tr '\n' ' ' | tr -s ' ' | sed -ne "/${expected_object}/p") | |||
There was a problem hiding this comment.
This one is interesting. $output looks like this in one of the cases:
Pretty printed source:
object "object" {
code { let x := 0 }
}
Binary representation:
600050
Text representation:
/* "<stdin>":16:22 */
0x00
/* "<stdin>":0:24 */
pop
Not quoting it means that any * characters are evaluated as shell globs. And as you can see the output contains multi-line comments... The opening part (/*) expands to the content of your root directory while the closing one is replaced by the content of the current directory :) This means that part of the output is just garbage but the regex still finds the expected part because it is in there somewhere among that garbage :)
Quoting it fixes the problem but also breaks the regex because a convenient side-effect of not quoting it was that it got squashed into a single line with multiple spaces replaced by a single one. I added the tr commands to restore that behavior in a more explicit way.
| output=$(echo '' | "$SOLC" --ast - 2>/dev/null) | ||
| output=$(echo '' | "$SOLC" --ast-json - 2>/dev/null) | ||
| result=$? | ||
| set -e | ||
| if [[ $? != 0 ]] | ||
| if [[ $result != 0 ]] |
There was a problem hiding this comment.
There's no --ast option (and I think there never was?) but this was not detected because $? in the if is the result of the set command, not the compiler run.
I suspect this was supposed to be either --ast-compact-json or --ast-json. Or maybe --combined-json ast?
…egex in the presence of a single-letter file name
| @@ -28,4 +28,4 @@ | |||
|
|
|||
| set -e | |||
|
|
|||
| grep -oP "PROJECT_VERSION \"?\K[0-9.]+(?=\")"? $(dirname "$0")/../CMakeLists.txt | |||
| grep -oP "PROJECT_VERSION \"?\K[0-9.]+(?=\")?" $(dirname "$0")/../CMakeLists.txt | |||
There was a problem hiding this comment.
The ? character is meant to make the closing " in PROJECT_VERSION "..." optional and does - but only as long as the current directory does not contain a single-letter file name that would match it. In that case Bash would replace it with the file name and break the regex.
Putting it inside quotes fixes the problem.
There was a problem hiding this comment.
Are you sure this is correct? The ? now seems to refer to the (?-expression preceding it, and not to a quote.
There was a problem hiding this comment.
Yeah. The quote is just Bash syntax and not a part of the regex. I think that we do want ? to apply to the (?-expression (which is the part of the regex that will match a quote in the text). And it applies to it in both cases but if it's outside the quotes, it's interpreted as Bash wildcard syntax (unless nothing matches and then it falls back to being just ?).
Related to #10583.
This is a bunch of minor bugs I discovered in our shell scripts while fixing warnings reported by shellcheck.