Merge pull request #1967 from mtrmac/golangci-lint

Rework hack/make*, and switch from golint to golangci-lint

.cirrus.yml

@@ -52,7 +52,9 @@ validate_task:
         image: '${SKOPEO_CIDEV_CONTAINER_FQIN}'
         cpu: 4
         memory: 8
-    script: |
+    setup_script: |
+        make tools
+    test_script: |
         make validate-local
         make vendor && hack/tree_status.sh
 
@@ -91,7 +93,7 @@ osx_task:
         export PATH=$GOPATH/bin:$PATH
         brew update
         brew install gpgme go go-md2man
-        go install golang.org/x/lint/golint@latest
+        make tools
     test_script: |
         export PATH=$GOPATH/bin:$PATH
         go version

Makefile

@@ -38,13 +38,6 @@ endif
 export CONTAINER_RUNTIME ?= $(if $(shell command -v podman ;),podman,docker)
 GOMD2MAN ?= $(if $(shell command -v go-md2man ;),go-md2man,$(GOBIN)/go-md2man)
 
-# Go module support: set `-mod=vendor` to use the vendored sources.
-# See also hack/make.sh.
-ifeq ($(shell go help mod >/dev/null 2>&1 && echo true), true)
-  GO:=GO111MODULE=on $(GO)
-  MOD_VENDOR=-mod=vendor
-endif
-
 ifeq ($(DEBUG), 1)
   override GOGCFLAGS += -N -l
 endif
@@ -133,9 +126,9 @@ binary: cmd/skopeo
 # Build w/o using containers
 .PHONY: bin/skopeo
 bin/skopeo:
-	$(GO) build $(MOD_VENDOR) ${GO_DYN_FLAGS} ${SKOPEO_LDFLAGS} -gcflags "$(GOGCFLAGS)" -tags "$(BUILDTAGS)" -o $@ ./cmd/skopeo
+	$(GO) build ${GO_DYN_FLAGS} ${SKOPEO_LDFLAGS} -gcflags "$(GOGCFLAGS)" -tags "$(BUILDTAGS)" -o $@ ./cmd/skopeo
 bin/skopeo.%:
-	GOOS=$(word 2,$(subst ., ,$@)) GOARCH=$(word 3,$(subst ., ,$@)) $(GO) build $(MOD_VENDOR) ${SKOPEO_LDFLAGS} -tags "containers_image_openpgp $(BUILDTAGS)" -o $@ ./cmd/skopeo
+	GOOS=$(word 2,$(subst ., ,$@)) GOARCH=$(word 3,$(subst ., ,$@)) $(GO) build ${SKOPEO_LDFLAGS} -tags "containers_image_openpgp $(BUILDTAGS)" -o $@ ./cmd/skopeo
 local-cross: bin/skopeo.darwin.amd64 bin/skopeo.linux.arm bin/skopeo.linux.arm64 bin/skopeo.windows.386.exe bin/skopeo.windows.amd64.exe
 
 $(MANPAGES): %: %.md
@@ -188,6 +181,11 @@ install-completions: completions
 shell:
 	$(CONTAINER_RUN) bash
 
+tools:
+	if [ ! -x "$(GOBIN)/golangci-lint" ]; then \
+		curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOBIN) v1.52.2 ; \
+	fi
+
 check: validate test-unit test-integration test-system
 
 test-integration:
@@ -200,7 +198,8 @@ test-integration:
 
 # Intended for CI, assumed to be running in quay.io/libpod/skopeo_cidev container.
 test-integration-local: bin/skopeo
-	hack/make.sh test-integration
+	hack/warn-destructive-tests.sh
+	hack/test-integration.sh
 
 # complicated set of options needed to run podman-in-podman
 test-system:
@@ -216,7 +215,8 @@ test-system:
 
 # Intended for CI, assumed to already be running in quay.io/libpod/skopeo_cidev container.
 test-system-local: bin/skopeo
-	hack/make.sh test-system
+	hack/warn-destructive-tests.sh
+	hack/test-system.sh
 
 test-unit:
 	# Just call (make test unit-local) here instead of worrying about environment differences
@@ -230,16 +230,19 @@ test-all-local: validate-local validate-docs test-unit-local
 
 .PHONY: validate-local
 validate-local:
-	BUILDTAGS="${BUILDTAGS}" hack/make.sh validate-git-marks validate-gofmt validate-lint validate-vet
+	hack/validate-git-marks.sh
+	hack/validate-gofmt.sh
+	GOBIN=$(GOBIN) hack/validate-lint.sh
+	BUILDTAGS="${BUILDTAGS}" hack/validate-vet.sh
 
 # This invokes bin/skopeo, hence cannot be run as part of validate-local
 .PHONY: validate-docs
-validate-docs:
+validate-docs: bin/skopeo
 	hack/man-page-checker
 	hack/xref-helpmsgs-manpages
 
-test-unit-local: bin/skopeo
+test-unit-local:
-	$(GO) test $(MOD_VENDOR) -tags "$(BUILDTAGS)" $$($(GO) list $(MOD_VENDOR) -tags "$(BUILDTAGS)" -e ./... | grep -v '^github\.com/containers/skopeo/\(integration\|vendor/.*\)$$')
+	$(GO) test -tags "$(BUILDTAGS)" $$($(GO) list -tags "$(BUILDTAGS)" -e ./... | grep -v '^github\.com/containers/skopeo/\(integration\|vendor/.*\)$$')
 
 vendor:
 	$(GO) mod tidy

hack/make.sh

@@ -1,92 +0,0 @@
-#!/usr/bin/env bash
-set -e
-
-# This script builds various binary from a checkout of the skopeo
-# source code. DO NOT CALL THIS SCRIPT DIRECTLY.
-#
-# Requirements:
-# - The current directory should be a checkout of the skopeo source code
-#   (https://github.com/containers/skopeo). Whatever version is checked out
-#   will be built.
-# - The script is intended to be run inside the container specified
-#   in the output of hack/get_fqin.sh
-# - The right way to call this script is to invoke "make" from
-#   your checkout of the skopeo repository.
-#   the Makefile will do a "docker build -t skopeo ." and then
-#   "docker run hack/make.sh" in the resulting image.
-#
-
-set -o pipefail
-
-export SKOPEO_PKG='github.com/containers/skopeo'
-export SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-export MAKEDIR="$SCRIPTDIR/make"
-
-# Set this to 1 to enable installation/modification of environment/services
-export SKOPEO_CONTAINER_TESTS=${SKOPEO_CONTAINER_TESTS:-0}
-
-if [[ "$SKOPEO_CONTAINER_TESTS" == "0" ]] && [[ "$CI" != "true" ]]; then
-    (
-    echo "***************************************************************"
-    echo "WARNING: Executing tests directly on the local development"
-    echo "         host is highly discouraged.  Many important items"
-    echo "         will be skipped.  For manual execution, please utilize"
-    echo "         the Makefile targets WITHOUT the '-local' suffix."
-    echo "***************************************************************"
-    ) > /dev/stderr
-    sleep 5
-fi
-
-echo
-
-# List of bundles to create when no argument is passed
-# TODO(runcom): these are the one left from Docker...for now
-# test-unit
-# validate-dco
-# cover
-DEFAULT_BUNDLES=(
-	validate-gofmt
-	validate-lint
-	validate-vet
-	validate-git-marks
-
-	test-integration
-)
-
-# Go module support: set `-mod=vendor` to use the vendored sources
-# See also the top-level Makefile.
-mod_vendor=
-if go help mod >/dev/null 2>&1; then
-  export GO111MODULE=on
-  mod_vendor='-mod=vendor'
-fi
-
-go_test_dir() {
-	dir=$1
-	(
-		echo '+ go test' $mod_vendor $TESTFLAGS ${BUILDTAGS:+-tags "$BUILDTAGS"} "${SKOPEO_PKG}${dir#.}"
-		cd "$dir"
-		export DEST="$ABS_DEST" # we're in a subshell, so this is safe -- our integration-cli tests need DEST, and "cd" screws it up
-		go test $mod_vendor $TESTFLAGS ${BUILDTAGS:+-tags "$BUILDTAGS"}
-	)
-}
-
-bundle() {
-	local bundle="$1"; shift
-	echo "---> Making bundle: $(basename "$bundle")"
-	source "$SCRIPTDIR/make/$bundle" "$@"
-}
-
-main() {
-	if [ $# -lt 1 ]; then
-		bundles=(${DEFAULT_BUNDLES[@]})
-	else
-		bundles=($@)
-	fi
-	for bundle in ${bundles[@]}; do
-		bundle "$bundle"
-		echo
-	done
-}
-
-main "$@"

hack/make/.validate

@@ -1,31 +0,0 @@
-#!/bin/bash
-
-if [ -z "$VALIDATE_UPSTREAM" ]; then
-	# this is kind of an expensive check, so let's not do this twice if we
-	# are running more than one validate bundlescript
-	
-	VALIDATE_REPO='https://github.com/containers/skopeo.git'
-	VALIDATE_BRANCH='main'
-	
-	if [ "$TRAVIS" = 'true' -a "$TRAVIS_PULL_REQUEST" != 'false' ]; then
-		VALIDATE_REPO="https://github.com/${TRAVIS_REPO_SLUG}.git"
-		VALIDATE_BRANCH="${TRAVIS_BRANCH}"
-	fi
-	
-	VALIDATE_HEAD="$(git rev-parse --verify HEAD)"
-	
-	git fetch -q "$VALIDATE_REPO" "refs/heads/$VALIDATE_BRANCH"
-	VALIDATE_UPSTREAM="$(git rev-parse --verify FETCH_HEAD)"
-	
-	VALIDATE_COMMIT_LOG="$VALIDATE_UPSTREAM..$VALIDATE_HEAD"
-	VALIDATE_COMMIT_DIFF="$VALIDATE_UPSTREAM...$VALIDATE_HEAD"
-	
-	validate_diff() {
-		git diff "$VALIDATE_UPSTREAM" "$@"
-	}
-	validate_log() {
-		if [ "$VALIDATE_UPSTREAM" != "$VALIDATE_HEAD" ]; then
-			git log "$VALIDATE_COMMIT_LOG" "$@"
-		fi
-	}
-fi

hack/make/test-integration

@@ -1,12 +0,0 @@
-#!/bin/bash
-set -e
-
-bundle_test_integration() {
-	go_test_dir ./integration
-}
-
-# subshell so that we can export PATH without breaking other things
-(
-	make PREFIX=/usr install
-	bundle_test_integration
-) 2>&1

hack/make/validate-git-marks

@@ -1,44 +0,0 @@
-#!/usr/bin/env bash
-
-source "$(dirname "$BASH_SOURCE")/.validate"
-
-# folders=$(find * -type d | egrep -v '^Godeps|bundles|.git')
-
-IFS=$'\n'
-files=( $(validate_diff --diff-filter=ACMR --name-only -- '*' | grep -v '^vendor/' || true) )
-unset IFS
-
-badFiles=()
-for f in "${files[@]}"; do
-    if [ $(grep -r "^<<<<<<<" $f) ]; then
-        badFiles+=( "$f" )
-        continue
-    fi
-
-    if [ $(grep -r "^>>>>>>>" $f) ]; then
-        badFiles+=( "$f" )
-        continue
-    fi
-
-    if [ $(grep -r "^=======$" $f) ]; then
-        badFiles+=( "$f" )
-        continue
-    fi
-    set -e
-done
-
-
-if [ ${#badFiles[@]} -eq 0 ]; then
-	echo 'Congratulations!  There is no conflict.'
-else
-	{
-		echo "There is trace of conflict(s) in the following files :"
-		for f in "${badFiles[@]}"; do
-			echo " - $f"
-		done
-		echo
-		echo 'Please fix the conflict(s) commit the result.'
-		echo
-	} >&2
-	false
-fi

hack/make/validate-lint

@@ -1,33 +0,0 @@
-#!/bin/bash
-
-source "$(dirname "$BASH_SOURCE")/.validate"
-
-# We will eventually get to the point where packages should be the complete list
-# of subpackages, vendoring excluded, as given by:
-#
-IFS=$'\n'
-files=( $(validate_diff --diff-filter=ACMR --name-only -- '*.go' | grep -v '^vendor/\|^integration' || true) )
-unset IFS
-
-errors=()
-for f in "${files[@]}"; do
-	failedLint=$(golint "$f")
-	if [ "$failedLint" ]; then
-		errors+=( "$failedLint" )
-	fi
-done
-
-if [ ${#errors[@]} -eq 0 ]; then
-	echo 'Congratulations!  All Go source files have been linted.'
-else
-	{
-		echo "Errors from golint:"
-		for err in "${errors[@]}"; do
-			echo "$err"
-		done
-		echo
-		echo 'Please fix the above errors. You can test via "golint" and commit the result.'
-		echo
-	} >&2
-	false
-fi

hack/test-integration.sh

@@ -0,0 +1,8 @@
+#!/bin/bash
+set -e
+
+make PREFIX=/usr install
+
+echo "cd ./integration;" go test $TESTFLAGS ${BUILDTAGS:+-tags "$BUILDTAGS"}
+cd ./integration
+go test $TESTFLAGS ${BUILDTAGS:+-tags "$BUILDTAGS"}

hack/validate-git-marks.sh

@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+IFS=$'\n'
+files=( $(git ls-tree -r HEAD --name-only | grep -v '^vendor/' || true) )
+unset IFS
+
+badFiles=()
+for f in "${files[@]}"; do
+    if [ $(grep -r "^\(<<<<<<<\|>>>>>>>\|^=======$\)" $f) ]; then
+        badFiles+=( "$f" )
+        continue
+    fi
+    set -e
+done
+
+
+if [ ${#badFiles[@]} -eq 0 ]; then
+	echo 'Congratulations!  There is no conflict.'
+else
+	{
+		echo "There is trace of conflict(s) in the following files :"
+		for f in "${badFiles[@]}"; do
+			echo " - $f"
+		done
+		echo
+		echo 'Please fix the conflict(s) commit the result.'
+		echo
+	} >&2
+	exit 1
+fi

hack/validate-gofmt.sh

@@ -1,9 +1,7 @@
 #!/bin/bash
 
-source "$(dirname "$BASH_SOURCE")/.validate"
-
 IFS=$'\n'
-files=( $(validate_diff --diff-filter=ACMR --name-only -- '*.go' | grep -v '^vendor/' || true) )
+files=( $(find . -name '*.go' | grep -v '^./vendor/' | sort || true) )
 unset IFS
 
 badFiles=()
@@ -25,5 +23,5 @@ else
 		echo 'Please reformat the above files using "gofmt -s -w" and commit the result.'
 		echo
 	} >&2
-	false
+	exit 1
 fi

hack/validate-lint.sh

@@ -0,0 +1,16 @@
+#!/bin/bash
+
+errors=$($GOBIN/golangci-lint run --build-tags "${BUILDTAGS}" 2>&1)
+
+if [ -z "$errors" ]; then
+	echo 'Congratulations!  All Go source files have been linted.'
+else
+	{
+		echo "Errors from golangci-lint:"
+		echo "$errors"
+		echo
+		echo 'Please fix the above errors. You can test via "golangci-lint" and commit the result.'
+		echo
+	} >&2
+	exit 1
+fi

hack/validate-vet.sh

@@ -1,6 +1,6 @@
 #!/bin/bash
 
-errors=$(go vet -tags="${BUILDTAGS}" $mod_vendor $(go list $mod_vendor -e ./...))
+errors=$(go vet -tags="${BUILDTAGS}" ./... 2>&1)
 
 if [ -z "$errors" ]; then
 	echo 'Congratulations!  All Go source files have been vetted.'
@@ -12,5 +12,5 @@ else
 		echo 'Please fix the above errors. You can test via "go vet" and commit the result.'
 		echo
 	} >&2
-	false
+	exit 1
 fi

hack/warn-destructive-tests.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+set -e
+
+# Set this to 1 to enable installation/modification of environment/services
+export SKOPEO_CONTAINER_TESTS=${SKOPEO_CONTAINER_TESTS:-0}
+
+if [[ "$SKOPEO_CONTAINER_TESTS" == "0" ]] && [[ "$CI" != "true" ]]; then
+    (
+    echo "***************************************************************"
+    echo "WARNING: Executing tests directly on the local development"
+    echo "         host is highly discouraged.  Many important items"
+    echo "         will be skipped.  For manual execution, please utilize"
+    echo "         the Makefile targets WITHOUT the '-local' suffix."
+    echo "***************************************************************"
+    ) > /dev/stderr
+    sleep 5
+fi