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.

Sign up for GitHub

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

Fix minor bugs in shell scripts #10584

Merged
merged 6 commits into from
Dec 14, 2020
Merged

Fix minor bugs in shell scripts #10584

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1f2ffa9
cmdlineTests.sh: Fix test_solc_assembly_output including garbage in t…
cameel Dec 12, 2020
ac12274
cmdlineTests.sh: Fix the script failing to detect that --ast option d…
cameel Dec 12, 2020
81668eb
soltest_all: Fix an error being printed to a file called `2` rather t…
cameel Dec 11, 2020
f712662
ASTImportTest.sh: Fix the script ignoring all but the first file from…
cameel Dec 12, 2020
893a5b6
Remove unused variables
cameel Dec 11, 2020
4a16b13
get_version.sh: Quote the ? wildcard that could otherwise break the r…
cameel Dec 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .circleci/soltest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ set -e

OPTIMIZE=${OPTIMIZE:-"0"}
EVM=${EVM:-"invalid"}
WORKDIR=${CIRCLE_WORKING_DIRECTORY:-.}
REPODIR="$(realpath $(dirname $0)/..)"

source "${REPODIR}/scripts/common.sh"
Expand Down
2 changes: 1 addition & 1 deletion .circleci/soltest_all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,6 @@ done

if (($STEP != $STEPS + 1))
then
echo "Step counter not properly adjusted!" >2
echo "Step counter not properly adjusted!" >&2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>2 means "write to a file called 2", not "write to stderr".

exit 1
fi
2 changes: 1 addition & 1 deletion scripts/ASTImportTest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ do
NSOURCES=$((NSOURCES - 1))
for i in $OUTPUT;
do
testImportExportEquivalence $i $OUTPUT
testImportExportEquivalence "$i" "$OUTPUT"
Copy link
Member Author

@cameel cameel Dec 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

NSOURCES=$((NSOURCES + 1))
done
elif [ ${SPLITSOURCES_RC} == 1 ]
Expand Down
1 change: 0 additions & 1 deletion scripts/common_cmdline.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ function compileFull()
fi

local files="$*"
local output

local stderr_path=$(mktemp)

Expand Down
1 change: 0 additions & 1 deletion scripts/install_cmake.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ if test -f $BIN/cmake && ($BIN/cmake --version | grep -q "$VERSION"); then
else
FILE=cmake-$VERSION-$OS-x86_64.tar.gz
URL=https://cmake.org/files/v$VERSION_MAJOR.$VERSION_MINOR/$FILE
ERROR=0
TMPFILE=$(mktemp --tmpdir cmake-$VERSION-$OS-x86_64.XXXXXXXX.tar.gz)
echo "Downloading CMake ($URL)..."
wget "$URL" -O "$TMPFILE" -nv
Expand Down
1 change: 0 additions & 1 deletion scripts/release_ppa.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ wget -O ./solc/deps/downloads/jsoncpp-1.9.3.tar.gz https://github.com/open-sourc
cd solc
version=$($(dirname "$0")/get_version.sh)
commithash=$(git rev-parse --short=8 HEAD)
committimestamp=$(git show --format=%ci HEAD | head -n 1)
commitdate=$(git show --format=%ci HEAD | head -n 1 | cut - -b1-10 | sed -e 's/-0?/./' | sed -e 's/-0?/./')

echo "$commithash" > commit_hash.txt
Expand Down
4 changes: 2 additions & 2 deletions scripts/tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ do
then
if [ -n "$optimize" ]
then
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
Comment on lines -112 to +114
Copy link
Member Author

@cameel cameel Dec 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

fi
fi

Expand Down
7 changes: 4 additions & 3 deletions scripts/wasm-rebuild/docker-scripts/patch.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#!/bin/bash

TAG="$1"
SOLJSON_JS="$2"

# If we ever want to patch the binaries e.g. for compatibility with older solc-js versions,
# we can do that here.
#
# This script gets the following parameters:
# - TAG
# - SOLJSON_JS
9 changes: 6 additions & 3 deletions test/cmdlineTests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if [ -z "$empty" ]
then
printError "Incorrect assembly output. Expected: "
Expand Down Expand Up @@ -436,15 +436,18 @@ SOLTMPDIR=$(mktemp -d)
# The contract should be compiled
if [[ "$result" != 0 ]]
then
printError "Failed to compile a simple contract from standard input"
exit 1
fi

# This should not fail
set +e
output=$(echo '' | "$SOLC" --ast - 2>/dev/null)
output=$(echo '' | "$SOLC" --ast-json - 2>/dev/null)
result=$?
set -e
if [[ $? != 0 ]]
if [[ $result != 0 ]]
Comment on lines -444 to +448
Copy link
Member Author

@cameel cameel Dec 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

then
printError "Incorrect response to --ast-json option with empty stdin"
exit 1
fi
)
Expand Down