Ver Fonte

Add shellcheck to web, server-pro, server-ce (#23823)

* Update shellcheck files to include non ".sh" files having the sh shebang

* Add shellcheck to root Makefile

* `make shellcheck_fix`

* Add shellcheck to server-ce and server-pro

* Exclude SC1091 (Not following) in SP/CE

* Fix errors in SP

* Fix errors in CE

* Update root shellcheck to ignore all failing rules. We can later remove rules one by one.

* Add shellcheck to web

* Add shellcheck step for server-ce and server-pro in cloudbuild.yaml

* Revert "Add shellcheck to root Makefile"

This reverts commit e0fa56f2

* Revert "`make shellcheck_fix`"

This reverts commit eb179245c109a9e742a7fdeeb75a4bdd03963587.

* `make shellcheck_fix` in server-ce

* Fix: Use $(...) notation instead of legacy backticked

```
In init_scripts/100_set_docker_host_ipaddress.sh line 5:
echo "`route -n | awk '/UG[ \t]/{print $2}'` dockerhost" >> /etc/hosts
      ^-- SC2006 (style): Use $(...) notation instead of legacy backticks `...`.

Did you mean:
echo "$(route -n | awk '/UG[ \t]/{print $2}') dockerhost" >> /etc/hosts

For more information:
  https://www.shellcheck.net/wiki/SC2006 -- Use $(...) notation instead of le...
```

* `make shellcheck_fix` in web

* Manual corrections on autofix

* Update SHELLCHECK_FILES to Perl regex so it matches at start of file

GitOrigin-RevId: 815d53f319a3792efa22703beb75570de5977450
Antoine Clausse há 1 ano atrás
pai
commit
fe03d8d7fd

+ 22 - 1
server-ce/Makefile

@@ -1,6 +1,7 @@
 # Makefile
 
 MONOREPO_ROOT := ../
+HERE=$(shell pwd)
 export MONOREPO_REVISION := $(shell git rev-parse HEAD)
 export BRANCH_NAME ?= $(shell git rev-parse --abbrev-ref HEAD)
 export OVERLEAF_BASE_BRANCH ?= sharelatex/sharelatex-base:$(BRANCH_NAME)
@@ -40,5 +41,25 @@ build-community:
 	  --tag $(OVERLEAF_BRANCH) \
 	  $(MONOREPO_ROOT)
 
+SHELLCHECK_OPTS = \
+	--shell=bash \
+	--external-sources \
+	--exclude=SC1091
+SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
-.PHONY: all build-base build-community
+shellcheck:
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
+
+shellcheck_fix:
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
+		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
+		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
+		elif [ -n "$$diff" ]; then echo "$$file"; \
+		else echo "\033[2m$$file\033[0m"; fi \
+	done
+
+.PHONY: all \
+	build-base build-community \
+	shellcheck shellcheck_fix

+ 1 - 1
server-ce/init_scripts/100_set_docker_host_ipaddress.sh

@@ -2,4 +2,4 @@
 set -e -o pipefail
 
 # See the bottom of http://stackoverflow.com/questions/24319662/from-inside-of-a-docker-container-how-do-i-connect-to-the-localhost-of-the-mach
-echo "`route -n | awk '/UG[ \t]/{print $2}'` dockerhost" >> /etc/hosts
+echo "$(route -n | awk '/UG[ \t]/{print $2}') dockerhost" >> /etc/hosts

+ 1 - 0
server-ce/init_scripts/200_nginx_config_template.sh

@@ -26,6 +26,7 @@ if [ -f "${nginx_template_file}" ]; then
   # Note the single-quotes, they are important.
   # This is a pass-list of env-vars that envsubst
   # should operate on.
+  # shellcheck disable=SC2016
   envsubst '
     ${NGINX_KEEPALIVE_TIMEOUT}
     ${NGINX_WORKER_CONNECTIONS}

+ 1 - 1
server-ce/runit/clsi-overleaf/run

@@ -11,7 +11,7 @@ fi
 if [ -e '/var/run/docker.sock' ]; then
   echo ">> Setting permissions on docker socket"
   DOCKER_GROUP=$(stat -c '%g' /var/run/docker.sock)
-  groupadd --non-unique --gid ${DOCKER_GROUP} dockeronhost
+  groupadd --non-unique --gid "${DOCKER_GROUP}" dockeronhost
   usermod -aG dockeronhost www-data
 fi
 

+ 3 - 2
services/chat/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/clsi/Makefile

@@ -44,13 +44,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/contacts/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/docstore/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/document-updater/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/filestore/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/history-v1/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/notifications/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/project-history/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 3 - 2
services/real-time/Makefile

@@ -43,13 +43,14 @@ SHELLCHECK_OPTS = \
 	--shell=bash \
 	--external-sources
 SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
 
 shellcheck:
-	find . -type f -name "*.sh" -print0 | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(HERE):/mnt -w /mnt \
 		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
 
 shellcheck_fix:
-	@for file in $$(find . -type f -name "*.sh"); do \
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
 		diff=$$(docker run --rm -v $(HERE):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
 		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
 		elif [ -n "$$diff" ]; then echo "$$file"; \

+ 20 - 0
services/web/Makefile

@@ -4,6 +4,7 @@ BUILD_NUMBER ?= local
 BRANCH_NAME ?= $(shell git rev-parse --abbrev-ref HEAD)
 PROJECT_NAME = web
 BUILD_DIR_NAME = $(shell pwd | xargs basename | tr -cd '[a-zA-Z0-9_.\-]')
+PWD = $(shell pwd)
 
 export OVERLEAF_CONFIG ?= /overleaf/services/web/test/acceptance/config/settings.test.saas.js
 export BASE_CONFIG ?= ${OVERLEAF_CONFIG}
@@ -448,6 +449,24 @@ format_styles_fix:
 format_in_docker:
 	$(RUN_LINT_FORMAT) make format -j2 --output-sync
 
+SHELLCHECK_OPTS = \
+	--shell=bash \
+	--external-sources
+SHELLCHECK_COLOR := $(if $(CI),--color=never,--color)
+SHELLCHECK_FILES := { git ls-files "*.sh" -z; git grep -Plz "\A\#\!.*bash"; } | sort -zu
+
+shellcheck:
+	@$(SHELLCHECK_FILES) | xargs -0 -r docker run --rm -v $(PWD):/mnt -w /mnt \
+		koalaman/shellcheck:stable $(SHELLCHECK_OPTS) $(SHELLCHECK_COLOR)
+
+shellcheck_fix:
+	@$(SHELLCHECK_FILES) | while IFS= read -r -d '' file; do \
+		diff=$$(docker run --rm -v $(PWD):/mnt -w /mnt koalaman/shellcheck:stable $(SHELLCHECK_OPTS) --format=diff "$$file" 2>/dev/null); \
+		if [ -n "$$diff" ] && ! echo "$$diff" | patch -p1 >/dev/null 2>&1; then echo "\033[31m$$file\033[0m"; \
+		elif [ -n "$$diff" ]; then echo "$$file"; \
+		else echo "\033[2m$$file\033[0m"; fi \
+	done
+
 #
 # Build & publish
 #
@@ -548,4 +567,5 @@ $(MODULE_TARGETS):
 	test_unit_modules test_unit_module test_frontend \
 	test_acceptance test_acceptance_app test_acceptance_modules \
 	test_acceptance_module ci format format_fix lint \
+	shellcheck shellcheck_fix \
 	build publish tar

+ 3 - 0
services/web/bin/cdn_upload_batch

@@ -20,6 +20,7 @@ fi
 # -x              Python regex for excluding files from the sync
 if [[ "$text_extension" == "-" || $(find /tmp/public -type f -name "*$text_extension" | wc -l) != "0" ]]; then
   # Potentially skip upload of non-compressed .js/.css files.
+  # shellcheck disable=SC2086
   gsutil \
     -h "Cache-Control:public, max-age=31536000" \
     ${content_type_options} \
@@ -31,6 +32,8 @@ if [[ "$text_extension" == "-" || $(find /tmp/public -type f -name "*$text_exten
     "/tmp/public/" \
     "${bucket}/public/"
 fi
+
+# shellcheck disable=SC2086
 gsutil \
   -h "Cache-Control:public, max-age=31536000" \
   -h "Content-Encoding:gzip" \

+ 1 - 1
services/web/bin/check_extracted_translations

@@ -4,7 +4,7 @@ set -e
 
 # Ensure all locales used in the frontend are tracked
 OUTPUT=data/dumpFolder/i18next-scanner
-trap "rm -rf $OUTPUT" EXIT
+trap 'rm -rf "$OUTPUT"' EXIT
 npx i18next-scanner --output "$OUTPUT"
 ACTUAL=frontend/extracted-translations.json
 EXPECTED="$OUTPUT/frontend/extracted-translations.json"

+ 1 - 1
services/web/bin/push-translations-changes.sh

@@ -7,7 +7,7 @@ WEB_DIR=$(dirname "$SCRIPT_DIR")
 
 cd "$WEB_DIR"
 
-if [[ `git status --porcelain=2 locales/` ]]; then
+if [[ $(git status --porcelain=2 locales/) ]]; then
   git add locales/*
   git commit -m "auto update translation"
   # Switch the cloudbuild clone from https to ssh authentication.

+ 2 - 2
services/web/bin/run

@@ -1,8 +1,8 @@
 #!/bin/bash
 
 pushd ..
-bin/run $*
+bin/run "$*"
 RV=$?
-popd
+popd || exit 1
 
 exit $RV

+ 1 - 1
services/web/transform/cjs-to-esm/transform-dir.sh

@@ -12,7 +12,7 @@ while true; do
   if [ -z "$FILES" ]; then
     break
   fi
-  node transform/cjs-to-esm/cjs-to-esm.mjs $FILES
+  node transform/cjs-to-esm/cjs-to-esm.mjs "$FILES"
 done
 
 make format_fix > /dev/null

+ 1 - 1
services/web/transform/o-error/transform.sh

@@ -3,7 +3,7 @@ npx jscodeshift \
   -t transform/o-error/transform.js \
   --ignore-pattern=frontend/js/libraries.js \
   --ignore-pattern=frontend/js/vendor \
-  $1
+  "$1"
 # replace blank lines in staged changed with token
 git diff --ignore-all-space --ignore-blank-lines | sed 's/^\+$/\+REMOVE_ME_IM_A_BLANK_LINE/g' | git apply --reject --cached --ignore-space-change
 # stage changes with token instead of blank line