Merge Makefile and shell script improvements from an internal projectwip/lantw/license-makefile-shell-scripts

The Makefile used in this project was based on the one used in an
internal project. Since the Makefile and shell scripts have been
refactored several times and become easier to use, it should be good to
bring these improvements to the project.

9 files changed, 150 insertions, 94 deletions

diff --git a/.travis.yml b/.travis.yml
index e6f62f5..6e08069 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -10,6 +10,7 @@ addons:
     - openssl
     - libssl-dev
     - libgmp-dev
+    - shellcheck
 script:
 - set -e
 - bin/install_tools.sh
diff --git a/GNUmakefile b/GNUmakefile
index bbf14bd..81e6f18 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -1,62 +1,104 @@
 # Makefile for DEXON Consensus Core
 
-GOPATH = $(CURDIR)/../../../../
-ifndef BINDIR
-BINDIR := $(CURDIR)/build
+# Commands
+DOCKER       ?= docker
+GO           ?= go
+GOFMT        ?= gofmt
+GOLINT       ?= $(GOBIN)/golint
+GREP         ?= grep
+INSTALL      ?= install -c
+MKDIR_P      ?= mkdir -p
+SHELLCHECK   ?= shellcheck
+
+# Paths
+ifndef BUILDDIR
+BUILDDIR     := $(CURDIR)/build
 else
-BINDIR := $(abspath $(BINDIR))
-endif
-PROJECT_ROOT=github.com/dexon-foundation/dexon-consensus
-BLS_REPO = dexonfoundation/bls-go-alpine
-BLS_LIB = vendor/github.com/dexon-foundation/bls/lib/libbls384.a
-BUILDER_REPO = dexonfoundation/dexon-alpine
-
-ifeq ($(DOCKER),true)
-GO_LDFLAGS += -linkmode external -extldflags \"-static\"
+BUILDDIR     := $(abspath $(BUILDDIR))
 endif
+FIRST_GOPATH := $(shell $(GO) env GOPATH | cut -d: -f1)
+GOBIN        ?= $(FIRST_GOPATH)/bin
+BLS_LIB       = vendor/github.com/dexon-foundation/bls/lib/libbls384.a
 
+# Automake-like silent rules
 V ?= 0
-AT_LOCAL_GO    = $(AT_LOCAL_GO_$(V))
-AT_LOCAL_GO_0  = @echo "  HOST GO    "$1;
-AT_LOCAL_GO_1  =
-AT_DOCKER_GO   = $(AT_DOCKER_GO_$(V))
-AT_DOCKER_GO_0 = @echo "  DOCKER GO  "$1;
-AT_DOCKER_GO_1 =
+AT_LOCAL_GO        = $(AT_LOCAL_GO_$(V))
+AT_LOCAL_GO_0      = @echo "  HOST GO      "$1;
+AT_LOCAL_GO_1      =
+AT_DOCKER_GO       = $(AT_DOCKER_GO_$(V))
+AT_DOCKER_GO_0     = @echo "  DOCKER GO    "$1;
+AT_DOCKER_GO_1     =
+AT_RUN             = $(AT_RUN_$(V))
+AT_RUN_0           = @echo "  RUN          "$@;
+AT_RUN_1           =
+
+# Functions
+include functions.mk
+
+# Go build variables
+GO_IMPORT_PATH          = github.com/dexon-foundation/dexon-consensus
+# Handle -tags safely
+GO_TAGS                ?=
+GO_TAGS_SAFE            = $(call SAFE_STRING,$(GO_TAGS))
+GO_TAGS_FLAGS           = -tags $(GO_TAGS_SAFE)
+# Handle -ldflags safely
+GO_LDFLAGS              =
+GO_LDFLAGS_SAFE         = $(call SAFE_STRING,$(GO_LDFLAGS))
+GO_LDFLAGS_FLAGS        = -ldflags $(GO_LDFLAGS_SAFE)
+# Handle -timeout
+GO_TEST_TIMEOUT        := 33m
+# Produce common flags
+GO_BUILD_COMMON_FLAGS   = $(GO_TAGS_FLAGS) $(GO_LDFLAGS_FLAGS)
+GO_VET_COMMON_FLAGS     = $(GO_TAGS_FLAGS)
+GO_TEST_COMMON_FLAGS    = -v -timeout $(GO_TEST_TIMEOUT)
+# Produce final flags
+GO_BUILD_ALL_FLAGS      = $(GO_BUILD_COMMON_FLAGS) $(GO_BUILD_FLAGS)
+GO_LIST_ALL_FLAGS       = $(GO_BUILD_COMMON_FLAGS) $(GO_BUILD_FLAGS)
+GO_VET_ALL_FLAGS        = $(GO_VET_COMMON_FLAGS) $(GO_VET_FLAGS)
+GO_TEST_ALL_FLAGS       = $(GO_TEST_COMMON_FLAGS) $(GO_BUILD_COMMON_FLAGS) $(GO_BUILD_FLAGS)
+
+# Builder images
+BUILDER_IMAGE           = dexonfoundation/dexon-alpine:latest
+BLS_IMAGE               = dexonfoundation/bls-go-alpine:latest
+
+ifdef BUILD_IN_DOCKER
+GO_TAGS                += static
+GO_LDFLAGS             += -linkmode external -extldflags -static
+GO_BUILD_COMMON_FLAGS  += -buildmode=pie
+endif
 
 define BUILD_RULE
 $1: pre-build
-ifeq ($(DOCKER),true)
-	$(AT_DOCKER_GO)docker run --rm \
+ifdef BUILD_IN_DOCKER
+	$(AT_DOCKER_GO)$(DOCKER) run --rm \
 		-v BLSDATA:/data/bls \
-		-v "$(GOPATH)":/go:z \
-		-v $(BINDIR):/artifacts:z \
-		-e "GOPATH=/go" \
-		-w /go/src/$(PROJECT_ROOT) \
-		$(BUILDER_REPO):latest sh -c "\
+		-v $(FIRST_GOPATH):/go:z \
+		-v $(BUILDDIR):/artifacts:z \
+		-e GOPATH=/go \
+		-w /go/src/$(GO_IMPORT_PATH) \
+		$(BUILDER_IMAGE) sh -c $(call SAFE_STRING,\
 			mv -f $(BLS_LIB) $(BLS_LIB).bak; \
-			cp /data/bls/libbls384.a $(BLS_LIB) ;\
-			go build -o /artifacts/$1 $(PROJECT_ROOT)/cmd/$1; \
-			mv -f $(BLS_LIB).bak $(BLS_LIB)"
+			cp /data/bls/libbls384.a $(BLS_LIB); \
+			go build -o /artifacts/$1 $(GO_BUILD_ALL_FLAGS) \
+				$(GO_IMPORT_PATH)/cmd/$1; \
+			mv -f $(BLS_LIB).bak $(BLS_LIB))
 else
-	@mkdir -p $(BINDIR)
-	$(AT_LOCAL_GO)go install -ldflags '$(GO_LDFLAGS)' $(PROJECT_ROOT)/cmd/$1
-	@install -c $(GOPATH)/bin/$1 $(BINDIR)
+	$(AT_LOCAL_GO)$(GO) build -o $(GOBIN)/$1 $(GO_BUILD_ALL_FLAGS) \
+		$(GO_IMPORT_PATH)/cmd/$1
+	@$(INSTALL) $(GOBIN)/$1 $(BUILDDIR)
 endif
 endef
 
-GO_TEST_TIMEOUT := 33m
-
-TEST_TARGET := go list ./... | grep -v 'vendor'
+TEST_TARGET := $(GO) list $(GO_LIST_ALL_FLAGS) ./... | $(GREP) -v vendor
 ifeq ($(NO_INTEGRATION_TEST), true)
 	GO_TEST_TIMEOUT := 25m
-	TEST_TARGET := $(TEST_TARGET) | grep -v 'integration_test'
+	TEST_TARGET := $(TEST_TARGET) | $(GREP) -v integration_test
 else ifeq ($(ONLY_INTEGRATION_TEST), true)
-	TEST_TARGET := $(TEST_TARGET) | grep 'integration_test'
+	TEST_TARGET := $(TEST_TARGET) | $(GREP) integration_test
 endif
 
-GO_TEST_FLAG := -v -timeout $(GO_TEST_TIMEOUT)
 ifneq ($(NO_TEST_RACE), true)
-	GO_TEST_FLAG := $(GO_TEST_FLAG) -race
+	GO_TEST_COMMON_FLAGS += -race
 endif
 
 COMPONENTS = \
@@ -68,39 +110,47 @@ COMPONENTS = \
 default: all
 
 all: $(COMPONENTS)
-ifeq ($(DOCKER),true)
-	@docker volume rm BLSDATA > /dev/null
+ifdef BUILD_IN_DOCKER
+	$(DOCKER) volume rm BLSDATA > /dev/null
 endif
 
 $(foreach component, $(COMPONENTS), $(eval $(call BUILD_RULE,$(component))))
 
 pre-build: dep docker-dep
+	@$(MKDIR_P) $(BUILDDIR)
 
-pre-submit: dep check-format lint vet check-security test
+pre-submit: dep check-format lint vet shellcheck check-security test
 
 dep:
-	@bin/install_eth_dep.sh
-	@bin/install_dkg_dep.sh
+	bin/install_eth_dep.sh
+	bin/install_dkg_dep.sh
 
 docker-dep:
-ifeq ($(DOCKER),true)
-	@docker run --rm -v BLSDATA:/data/bls $(BLS_REPO):latest \
-	sh -c "cp -f /usr/lib/libbls384.a /data/bls/"
+ifdef BUILD_IN_DOCKER
+	$(DOCKER) volume create BLSDATA > /dev/null
+	$(DOCKER) run --rm -v BLSDATA:/data/bls $(BLS_IMAGE) \
+		cp -f /usr/lib/libbls384.a /data/bls/
 endif
 
 format:
-	@go fmt `go list ./... | grep -v 'vendor'`
+	$(AT_RUN)$(GO) fmt \
+		$$($(GO) list $(GO_LIST_ALL_FLAGS) ./... | $(GREP) -v vendor)
 
 lint:
-	@$(GOPATH)/bin/golint -set_exit_status `go list ./... | grep -v 'vendor'`
+	$(AT_RUN)$(GOBIN)/golint -set_exit_status \
+		$$($(GO) list $(GO_LIST_ALL_FLAGS) ./... | $(GREP) -v vendor)
 
 vet:
-	@go vet `go list ./... | grep -v 'vendor'`
+	$(AT_RUN)$(GO) vet $(GO_VET_ALL_FLAGS) \
+		$$($(GO) list $(GO_LIST_ALL_FLAGS) ./... | $(GREP) -v vendor)
+
+shellcheck:
+	$(AT_RUN)$(SHELLCHECK) */*.sh */*/*.sh
 
 check-security:
 	@rm -f gosec.log
-	@gosec -quiet -out gosec.log ./... || true
-	@if [ -a gosec.log ]; then \
+	$(AT_RUN)$(GOBIN)/gosec -quiet -out gosec.log ./... || true
+	@if [ -e gosec.log ]; then \
 		cat gosec.log; \
 		echo 'Error: security issue found'; \
 		exit 1; \
@@ -108,31 +158,31 @@ check-security:
 
 
 test-short:
-	@for pkg in `$(TEST_TARGET)`; do \
-		if ! go test -short $(GO_TEST_FLAG) $$pkg; then \
+	$(AT_RUN)for pkg in $$($(TEST_TARGET)); do \
+		if ! $(GO) test -short $(GO_TEST_ALL_FLAGS) "$$pkg"; then \
 			echo 'Some test failed, abort'; \
 			exit 1; \
 		fi; \
 	done
 
 test:
-	@for pkg in `$(TEST_TARGET)`; do \
-		if ! go test $(GO_TEST_FLAG) $$pkg; then \
+	$(AT_RUN)for pkg in $$($(TEST_TARGET)); do \
+		if ! $(GO) test $(GO_TEST_ALL_FLAGS) "$$pkg"; then \
 			echo 'Some test failed, abort'; \
 			exit 1; \
 		fi; \
 	done
 
 bench:
-	@for pkg in `go list ./... | grep -v 'vendor'`; do \
-		if ! go test -bench=. -run=^$$ $$pkg; then \
+	$(AT_RUN)for pkg in $$($(GO) list $(GO_LIST_ALL_FLAGS) ./... | $(GREP) -v vendor); do \
+		if ! $(GO) test -bench=. -run=^$$ $(GO_TEST_nALL_FLAGS) "$$pkg"; then \
 			echo 'Some test failed, abort'; \
 			exit 1; \
 		fi; \
 	done
 
 check-format:
-	@if gofmt -l `go list -f '{{.Dir}}' ./...` | grep -q go; then \
+	$(AT_RUN)if $(GOFMT) -l $$($(GO) list -f '{{.Dir}}' $(GO_LIST_ALL_FLAGS) ./...) | $(GREP) -q go; then \
 		echo 'Error: source code not formatted'; \
 		exit 1; \
 	fi
@@ -144,6 +194,6 @@ test-sim: all
 	@cp test_config/test.toml build/test-sim/
 	@cd build/test-sim ; ../dexcon-simulation-peer-server -config test.toml >& server.log &
 	@cd build/test-sim ; ../dexcon-simulation -config test.toml >& /dev/null
-	@if grep "error" build/test-sim/server.log -q -i; then \
+	@if $(GREP) "error" build/test-sim/server.log -q -i; then \
 		exit 1; \
 	fi
diff --git a/bin/env.sh b/bin/env.sh
index d1939a5..d3863d6 100644
--- a/bin/env.sh
+++ b/bin/env.sh
@@ -1,4 +1,5 @@
 # Environment variables for the project.
+# shellcheck shell=dash disable=SC2155
 
 export GITROOT=$(git rev-parse --show-toplevel)
 
diff --git a/bin/install_dkg_dep.sh b/bin/install_dkg_dep.sh
index 7d02437..b87e3e4 100755
--- a/bin/install_dkg_dep.sh
+++ b/bin/install_dkg_dep.sh
@@ -1,4 +1,6 @@
-#!/bin/bash
+#!/bin/sh
+
+: "${MAKE:="make"}"
 
 if [ -e .dep/dkg ]; then
   exit 0
@@ -6,13 +8,14 @@ fi
 
 if [ ! -d .dep/dkg ]; then
   mkdir -p .dep/dkg
-  cd .dep/dkg
-  git clone --depth 1 -b master git://github.com/dexon-foundation/bls.git &
-  git clone --depth 1 -b master git://github.com/dexon-foundation/mcl.git &
-  wait
-  cd bls
-  make test_go -j MCL_USE_OPENSSL=0
-  cd ../../../
+  (
+    cd .dep/dkg || exit
+    git clone --depth 1 -b master git://github.com/dexon-foundation/bls.git &
+    git clone --depth 1 -b master git://github.com/dexon-foundation/mcl.git &
+    wait
+    cd bls || exit
+    eval "${MAKE} test_go -j MCL_USE_OPENSSL=0"
+  ) || exit
 fi
 cd vendor/github.com/dexon-foundation && rm -rf bls mcl
 ln -s ../../../.dep/dkg/* .
diff --git a/bin/install_eth_dep.sh b/bin/install_eth_dep.sh
index 79ecd0e..ea15348 100755
--- a/bin/install_eth_dep.sh
+++ b/bin/install_eth_dep.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 
 if [ -e .dep/libsecp256k1 ]; then
   exit 0
@@ -7,14 +7,10 @@ fi
 rm -rf vendor/github.com/ethereum/go-ethereum/crypto/secp256k1/libsecp256k1
 if [ ! -d .dep/libsecp256k1 ]; then
   git init .dep/libsecp256k1
-  cd .dep/libsecp256k1
-  git remote add origin https://github.com/ethereum/go-ethereum.git
-  git config core.sparsecheckout true
-  echo "crypto/secp256k1/libsecp256k1/*" >> .git/info/sparse-checkout
-  cd ../../
+  git -C .dep/libsecp256k1 remote add origin https://github.com/ethereum/go-ethereum.git
+  git -C .dep/libsecp256k1 config core.sparsecheckout true
+  echo 'crypto/secp256k1/libsecp256k1/*' >> .dep/libsecp256k1/.git/info/sparse-checkout
 fi
-cd .dep/libsecp256k1
-git pull --depth=1 origin master
-cd ../../
+git -C .dep/libsecp256k1 pull --depth=1 origin master
 cp -r .dep/libsecp256k1/crypto/secp256k1/libsecp256k1 \
   vendor/github.com/ethereum/go-ethereum/crypto/secp256k1
diff --git a/bin/install_tools.sh b/bin/install_tools.sh
index bef77f7..857d447 100755
--- a/bin/install_tools.sh
+++ b/bin/install_tools.sh
@@ -1,11 +1,13 @@
 #!/bin/sh
 
-if ! which dep >/dev/null 2>&1; then
-  go get -u github.com/golang/dep/cmd/dep
+: "${GO:="go"}"
+
+if ! command -v dep >/dev/null 2>&1; then
+  eval "${GO} get -u ${GO_BUILD_FLAGS} github.com/golang/dep/cmd/dep"
 fi
-if ! which golint >/dev/null 2>&1; then
-  go get -u golang.org/x/lint/golint
+if ! command -v golint >/dev/null 2>&1; then
+  eval "${GO} get -u ${GO_BUILD_FLAGS} golang.org/x/lint/golint"
 fi
-if ! which gosec >/dev/null 2>&1; then
-  go get github.com/securego/gosec/cmd/gosec/...
+if ! command -v gosec >/dev/null 2>&1; then
+  eval "${GO} get -u ${GO_BUILD_FLAGS} github.com/securego/gosec/cmd/gosec/..."
 fi
diff --git a/functions.mk b/functions.mk
new file mode 100644
index 0000000..3c44151
--- /dev/null
+++ b/functions.mk
@@ -0,0 +1,3 @@
+# This function is put in a separate file to avoid breaking Vim syntax
+# highlighting.
+SAFE_STRING = '$(subst ','"'"',$1)'
diff --git a/simulation/kubernetes/run_scheduler.sh b/simulation/kubernetes/run_scheduler.sh
index 8b45eb5..d6e7ced 100755
--- a/simulation/kubernetes/run_scheduler.sh
+++ b/simulation/kubernetes/run_scheduler.sh
@@ -3,13 +3,13 @@
 IMAGE_TAG=asia.gcr.io/cobinhood/dexcon-simulation:latest
 
 build_binary() {
-  make DOCKER=true -C ../..
+  make -C ../.. BUILD_IN_DOCKER=true
   cp -r ../../build .
 }
 
 build_docker_image() {
-  docker build -t ${IMAGE_TAG} .
-  docker push ${IMAGE_TAG}
+  docker build -t "${IMAGE_TAG}" .
+  docker push "${IMAGE_TAG}"
 }
 
 start_simulation() {
@@ -23,11 +23,11 @@ main() {
     local num_nodes=$1
     local num_cpus=$2
 
-    if [ "$num_nodes" == "" ]; then
-      num_nodes=31
+    if [ "$num_nodes" = "" ]; then
+        num_nodes=31
     fi
 
-    if [ "$num_cpus" == "" ]; then
+    if [ "$num_cpus" = "" ]; then
         num_cpus=2
     fi
 
@@ -41,4 +41,4 @@ main() {
     start_simulation
 }
 
-main $*
+main "$@"
diff --git a/simulation/kubernetes/run_simulation.sh b/simulation/kubernetes/run_simulation.sh
index 2537eb6..d3bf28b 100755
--- a/simulation/kubernetes/run_simulation.sh
+++ b/simulation/kubernetes/run_simulation.sh
@@ -4,13 +4,13 @@ IMAGE_TAG=asia.gcr.io/dexon-dev/dexcon-simulation:latest
 
 
 build_binary() {
-  make DOCKER=true -C ../..
+  make -C ../.. BUILD_IN_DOCKER=true
   cp -r ../../build .
 }
 
 build_docker_image() {
-  docker build -t ${IMAGE_TAG} .
-  docker push ${IMAGE_TAG}
+  docker build -t "${IMAGE_TAG}" .
+  docker push "${IMAGE_TAG}"
 }
 
 start_simulation() {
@@ -34,7 +34,7 @@ start_simulation() {
 main() {
   local num_nodes=$1
 
-  if [ "$num_nodes" == "" ]; then
+  if [ "$num_nodes" = "" ]; then
     num_nodes=7
   fi
 
@@ -47,4 +47,4 @@ main() {
   start_simulation
 }
 
-main $*
+main "$@"