ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23k stars 5.7k forks source link

Stylecheck script fails on MacOS #13492

Open reubenr0d opened 2 years ago

reubenr0d commented 2 years ago

scripts/check_style.sh does not work on MacOS:

$ scripts/check_style.sh 
grep: repetition-operator operand invalid

$ sysctl kern.version
kern.version: Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:41 PST 2021; root:xnu-8019.61.5~1/RELEASE_ARM64_T6000
$ grep -V
grep (BSD grep, GNU compatible) 2.6.0-FreeBSD
nikola-matic commented 2 years ago

Eh, so it looks like this has been broken on OSX for a long time now; OSX uses FreeBSD grep, whereas Linux used GNU grep, and even though they claim GNU compatible, it's really not.

The easiest solution would be to require a brew install grep prerequisite on Macs, which will install GNU grep, but under the ggrep command, and then altering the check_style.sh script to check whether it's on Darwin, and use ggrep instead of grep.

Or, we can simply let it sit broken until we move to clang-format which ought to make the grep-ing approach redundant, but that's not likely to happen any time soon.

reubenr0d commented 2 years ago

I have something that I think works, but still need to test it:

diff --git a/scripts/check_style.sh b/scripts/check_style.sh
index d1ad6bb9e..dbfbecd85 100755
--- a/scripts/check_style.sh
+++ b/scripts/check_style.sh
@@ -4,6 +4,12 @@ set -eu

 ERROR_LOG="$(mktemp -t check_style_XXXXXX.log)"

+if [ "$(uname)" == "Darwin" ]; then
+    grepCommand=ggrep
+else
+    grepCommand=grep
+fi
+
 EXCLUDE_FILES=(
     "libsolutil/picosha2.h"
     "test/cmdlineTests/strict_asm_only_cr/input.yul"
@@ -24,7 +30,7 @@ REPO_ROOT="$(dirname "$0")"/..
 cd "$REPO_ROOT" || exit 1

 WHITESPACE=$(git grep -n -I -E "^.*[[:space:]]+$" |
-    grep -v "test/libsolidity/ASTJSON\|test/libsolidity/ASTRecoveryTests\|test/compilationTests/zeppelin/LICENSE\|${EXCLUDE_FILES_JOINED}" || true
+    ${grepCommand} -v "test/libsolidity/ASTJSON\|test/libsolidity/ASTRecoveryTests\|test/compilationTests/zeppelin/LICENSE\|${EXCLUDE_FILES_JOINED}" || true
 )

 if [[ "$WHITESPACE" != "" ]]
@@ -37,13 +43,13 @@ fi

 function preparedGrep
 {
-    git grep -nIE "$1" -- '*.h' '*.cpp' | grep -v "${EXCLUDE_FILES_JOINED}"
+    git grep -nIE "$1" -- '*.h' '*.cpp' | ${grepCommand} -v "${EXCLUDE_FILES_JOINED}"
     return $?
 }

 FORMATERROR=$(
 (
-    preparedGrep "#include \"" | grep -E -v -e "license.h" -e "BuildInfo.h"  # Use include with <> characters
+    preparedGrep "#include \"" | ${grepCommand} -E -v -e "license.h" -e "BuildInfo.h"  # Use include with <> characters
     preparedGrep "\<(if|for|while|switch)\(" # no space after "if", "for", "while" or "switch"
     preparedGrep "\<for\>\s*\([^=]*\>\s:\s.*\)" # no space before range based for-loop
     preparedGrep "\<if\>\s*\(.*\)\s*\{\s*$" # "{\n" on same line as "if"
@@ -51,12 +57,12 @@ FORMATERROR=$(
     preparedGrep "[,\(<]\s*const " # const on left side of type
     preparedGrep "^\s*(static)?\s*const " # const on left side of type (beginning of line)
     preparedGrep "^ [^*]|[^*]  |    [^*]" # uses spaces for indentation or mixes spaces and tabs
-    preparedGrep "[a-zA-Z0-9_]\s*[&][a-zA-Z_]" | grep -E -v "return [&]" # right-aligned reference ampersand (needs to exclude return)
+    preparedGrep "[a-zA-Z0-9_]\s*[&][a-zA-Z_]" | ${grepCommand} -E -v "return [&]" # right-aligned reference ampersand (needs to exclude return)
     # right-aligned reference pointer star (needs to exclude return and comments)
-    preparedGrep "[a-zA-Z0-9_]\s*[*][a-zA-Z_]" | grep -E -v -e "return [*]" -e "^* [*]" -e "^*//.*"
+    preparedGrep "[a-zA-Z0-9_]\s*[*][a-zA-Z_]" | ${grepCommand} -E -v -e "return [*]" -e "^* [*]" -e "^*//.*"
     # unqualified move check, i.e. make sure that std::move() is used instead of move()
-    preparedGrep "move\(.+\)" | grep -v "std::move" | grep -E "[^a-z]move"
-) | grep -E -v -e "^[a-zA-Z\./]*:[0-9]*:\s*\/(\/|\*)" -e "^test/" || true
+    preparedGrep "move\(.+\)" | ${grepCommand} -v "std::move" | ${grepCommand} -E "[^a-z]move"
+) | ${grepCommand} -E -v -e "^[a-zA-Z\./]*:[0-9]*:\s*\/(\/|\*)" -e "^test/" || true
 )

 if [[ "$FORMATERROR" != "" ]]

Although I get some warnings when I try to run it now, need to check if they're relevant:

$scripts/check_style.sh         
ggrep: warning: stray \ before /
ggrep: warning: stray \ before /
ggrep: warning: * at start of expression
ggrep: warning: * at start of expression
hiroshitashir commented 1 year ago

PR at https://github.com/ethereum/solidity/pull/14348