Golang upgrade, buildkite integration, Windows docker toolbox support, …

…fixes #55, fixes #34 (#56)

.buildkite/macosdockerformac.yml

@@ -0,0 +1,7 @@
+  - command: ".buildkite/test.sh"
+    agents:
+      - "os=macos"
+      - "dockertype=dockerformac"
+    env:
+      BUILDKITE_CLEAN_CHECKOUT: true
+    parallelism: 1

.buildkite/sanetestbot.sh

@@ -0,0 +1,22 @@
+#!/bin/bash
+
+# Check a testbot or test environment to make sure it's likely to be sane.
+# We should add to this script whenever a testbot fails and we can figure out why.
+
+set -o errexit
+set -o pipefail
+set -o nounset
+
+
+# Check that required commands are available.
+for command in mysql git go make; do
+    command -v $command >/dev/null || ( echo "Did not find command installed '$command'" && exit 2 )
+done
+docker run -t busybox ls
+
+if [ "$(go env GOOS)" = "windows"  -a "$(git config core.autocrlf)" != "false" ] ; then
+ echo "git config core.autocrlf is not set to false on windows"
+ exit 3
+fi
+
+echo "--- testbot $HOSTNAME seems to be set up OK"

.buildkite/test.cmd

@@ -0,0 +1,9 @@
+@echo "Building using bash and build.sh"
+"C:\Program Files\git\bin\bash" .buildkite/test.sh
+
+if %ERRORLEVEL% EQU 0 (
+   @echo Successful build
+) else (
+   @echo Failure Reason Given is %errorlevel%
+   exit /b %errorlevel%
+)

.buildkite/test.sh

@@ -0,0 +1,30 @@
+#!/bin/bash
+
+# This script is used to build drud/build-tools using buildkite
+
+# Manufacture a $GOPATH environment that can mount on docker (when buildkite build)
+export GOPATH=~/tmp/buildkite-fake-gopath/$BUILDKITE_JOB_ID
+DRUDSRC=$GOPATH/src/github.com/drud
+mkdir -p $DRUDSRC
+ln -s $PWD $DRUDSRC/build-tools
+cd $DRUDSRC/build-tools
+BUILD_OS=$(go env GOOS)
+echo "--- buildkite building $BUILDKITE_JOB_ID at $(date) on $HOSTNAME for OS=$(go env GOOS) in $PWD GOPATH=$GOPATH"
+
+set -o errexit
+set -o pipefail
+set -o nounset
+set -x
+
+# Our testbot should now be sane, run the testbot checker to make sure.
+echo "--- Checking for sane testbot"
+./.buildkite/sanetestbot.sh
+
+echo "--- make $BUILD_OS"
+cd tests
+time make $BUILD_OS
+echo "--- make test"
+time make test
+RV=$?
+echo "--- build.sh completed with status=$RV"
+exit $RV

.buildkite/windows10dockerforwindows.yml

@@ -0,0 +1,7 @@
+  - command: ".buildkite/test.cmd"
+    agents:
+      - "os=windows"
+      - "dockertype=dockerforwindows"
+    env:
+      BUILDKITE_CLEAN_CHECKOUT: true
+    parallelism: 1

.buildkite/windows10dockertoolbox.yml

@@ -0,0 +1,7 @@
+  - command: ".buildkite/test.cmd"
+    agents:
+      - "os=windows"
+      - "dockertype=toolbox"
+    env:
+      BUILDKITE_CLEAN_CHECKOUT: true
+    parallelism: 1

build_update.sh

@@ -40,7 +40,7 @@ tar -xf $local_file
 rm -rf build-tools/*
 cp -r $internal_name/ build-tools/
 rm -rf $internal_name/
-rm -rf build-tools/{tests,circle.yml,.circleci,.github,.appveyor.yml}
+rm -rf build-tools/{tests,circle.yml,.circleci,.github,.appveyor.yml,.buildkite}
 touch build-tools/build-tools-VERSION-$tag.txt
 git add build-tools
 echo "Updated build-tools to $tag

makefile_components/base_build_go.mak

@@ -5,7 +5,7 @@
 ##### comment about what you did and why.
 
 
-.PHONY: all build test push clean container-clean bin-clean version static govendor gofmt govet golint
+.PHONY: all build test push clean container-clean bin-clean version static govendor gofmt govet golint golangci-lint container
 GOTMP=.gotmp
 
 SHELL = /bin/bash
@@ -14,7 +14,7 @@ GOFILES = $(shell find $(SRC_DIRS) -name "*.go")
 
 BUILD_OS = $(shell go env GOHOSTOS)
 
-BUILD_IMAGE ?= drud/golang-build-container:v1.10.2
+BUILD_IMAGE ?= drud/golang-build-container:v1.10.3
 
 BUILD_BASE_DIR ?= $$PWD
 
@@ -23,6 +23,7 @@ SRC_AND_UNDER = $(patsubst %,./%/...,$(SRC_DIRS))
 
 GOMETALINTER_ARGS ?= --vendored-linters --disable-all --enable=gofmt --enable=vet --enable=vetshadow --enable=golint --enable=errcheck --enable=staticcheck --enable=ineffassign --enable=varcheck --enable=deadcode --deadline=2m
 
+GOLANGCI_LINT_ARGS ?= --out-format=line-number --disable-all --enable=gofmt --enable=govet --enable=golint --enable=errcheck --enable=staticcheck --enable=ineffassign --enable=varcheck --enable=deadcode
 
 COMMIT := $(shell git describe --tags --always --dirty)
 BUILDINFO = $(shell echo Built $$(date) $$(whoami)@$$(hostname) $(BUILD_IMAGE) )
@@ -35,26 +36,28 @@ LDFLAGS := -extldflags -static $(VERSION_LDFLAGS)
 DOCKERMOUNTFLAG := :delegated
 
 PWD=$(shell pwd)
+S =
 ifeq ($(BUILD_OS),windows)
-    TMPPWD=$(shell cmd /C echo %cd%)
-    PWD=$(shell echo "$(TMPPWD)" | awk '{gsub("\\\\", "/"); print}' )
+    # On Windows docker toolbox, volume mounts oddly need a // at the beginning for things to work out, so
+    # add that extra slash only on Windows.
+    S=/
 endif
 
-build: linux darwin
+build: $(BUILD_OS)
 
 linux darwin windows: $(GOFILES)
 	@echo "building $@ from $(SRC_AND_UNDER)"
 	@$(shell rm -f VERSION.txt)
 	@$(shell mkdir -p bin/$@ $(GOTMP)/{std/$@,bin,src/$(PKG)})
-	@docker run -t --rm -u $(shell id -u):$(shell id -g)                    \
-	    -v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                   \
-	    -v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                 \
-	    -v $(PWD)/bin/$@:/go/bin$(DOCKERMOUNTFLAG)                         \
-	    -v $(PWD)/bin/$@:/go/bin/$@$(DOCKERMOUNTFLAG)                 \
-	    -v $(PWD)/$(GOTMP)/std/$@:/usr/local/go/pkg/$@_amd64_static$(DOCKERMOUNTFLAG)  \
+	docker run -t --rm -u $(shell id -u):$(shell id -g)                    \
+	    -v "$(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)"                                \
+	    -v "$(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)"                              \
+	    -v "$(S)$$PWD/bin/$@:/go/bin$(DOCKERMOUNTFLAG)"                        \
+	    -v "$(S)$$PWD/bin/$@:/go/bin/$@$(DOCKERMOUNTFLAG)"             \
+	    -v "$(S)$$PWD/$(GOTMP)/std/$@:/usr/local/go/pkg/$@_amd64_static$(DOCKERMOUNTFLAG)"  \
 	    -e CGO_ENABLED=0                  \
 	    -e GOOS=$@						  \
-	    -w /go/src/$(PKG)                 \
+	    -w $(S)/go/src/$(PKG)                 \
 	    $(BUILD_IMAGE)                    \
         go install -installsuffix static -ldflags ' $(LDFLAGS) ' $(SRC_AND_UNDER)
 	@$(shell touch $@)
@@ -63,111 +66,121 @@ linux darwin windows: $(GOFILES)
 govendor:
 	@echo -n "Using govendor to check for missing dependencies and unused dependencies: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                    \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                   \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                  \
-		-w /go/src/$(PKG)                                         \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                   \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                  \
+		-w $(S)/go/src/$(PKG)                                         \
 		$(BUILD_IMAGE)                                                     \
 		bash -c 'OUT=$$(govendor list +missing +unused); if [ -n "$$OUT" ]; then echo "$$OUT"; exit 1; fi'
 
 gofmt:
 	@echo "Checking gofmt: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                    \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		bash -c 'export OUT=$$(gofmt -l $(SRC_DIRS))  && if [ -n "$$OUT" ]; then echo "These files need gofmt -w: $$OUT"; exit 1; fi'
 
 govet:
 	@echo "Checking go vet: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                         \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $S/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		bash -c 'go vet $(SRC_AND_UNDER)'
 
 golint:
 	@echo "Checking golint: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                   \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		bash -c 'export OUT=$$(golint $(SRC_AND_UNDER)) && if [ -n "$$OUT" ]; then echo "Golint problems discovered: $$OUT"; exit 1; fi'
 
 errcheck:
 	@echo "Checking errcheck: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                   \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		errcheck $(SRC_AND_UNDER)
 
 staticcheck:
 	@echo "Checking staticcheck: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                         \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		staticcheck $(SRC_AND_UNDER)
 
 unused:
 	@echo "Checking unused variables and functions: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                         \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		unused $(SRC_AND_UNDER)
 
 codecoroner:
 	@echo "Checking codecoroner for unused functions: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                         \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE) \
 		bash -c 'OUT=$$(codecoroner -tests -ignore vendor funcs $(SRC_AND_UNDER)); if [ -n "$$OUT" ]; then echo "$$OUT"; exit 1; fi'                                             \
 
 
 varcheck:
 	@echo "Checking unused globals and struct members: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                         \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		varcheck $(SRC_AND_UNDER) && structcheck $(SRC_AND_UNDER)
 
 misspell:
 	@echo "Checking for misspellings: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                         \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
 		misspell $(SRC_DIRS)
 
 gometalinter:
 	@echo "gometalinter: "
 	@docker run -t --rm -u $(shell id -u):$(shell id -g)                         \
-		-v $(PWD)/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
-		-v $(PWD):/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
-		-w /go/src/$(PKG)                                                  \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                                                 \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                                          \
+		-w $(S)/go/src/$(PKG)                                                  \
 		$(BUILD_IMAGE)                                                     \
-		gometalinter $(GOMETALINTER_ARGS) $(SRC_AND_UNDER)
+		time gometalinter $(GOMETALINTER_ARGS) $(SRC_AND_UNDER)
+
+golangci-lint:
+	@echo "golangci-lint: "
+	@docker run -t --rm -u $(shell id -u):$(shell id -g)                       \
+		-v $(S)$$PWD/$(GOTMP):/go$(DOCKERMOUNTFLAG)                            \
+		-v $(S)$$PWD:/go/src/$(PKG)$(DOCKERMOUNTFLAG)                          \
+		-w $(S)/go/src/$(PKG)                                                  \
+		$(BUILD_IMAGE)                                                         \
+		time bash -c "golangci-lint run $(GOLANGCI_LINT_ARGS) $(SRC_AND_UNDER)"
 
 version:
 	@echo VERSION:$(VERSION)
 
 clean: container-clean bin-clean
-	go clean -cache || echo "You're not running latest golang locally" # Make sure the local go cache is clean for testing
+	@go clean -cache || echo "You're not running latest golang locally" # Make sure the local go cache is clean for testing
 
 container-clean:
-	$(shell rm -rf .container-* .dockerfile* .push-* linux darwin windows container VERSION.txt .docker_image)
+	@if docker image inspect $(DOCKER_REPO):$(VERSION) >/dev/null 2>&1; then docker rmi -f $(DOCKER_REPO):$(VERSION); fi
+	@rm -rf .container-* .dockerfile* .push-* linux darwin windows container VERSION.txt .docker_image
 
 bin-clean:
 	$(shell rm -rf $(GOTMP) bin .tmp)

makefile_components/base_container.mak

@@ -8,14 +8,17 @@ SANITIZED_DOCKER_REPO = $(subst /,_,$(DOCKER_REPO))
 
 DOTFILE_IMAGE = $(subst /,_,$(IMAGE))-$(VERSION)
 
-container: linux .container-$(DOTFILE_IMAGE) container-name
-
-.container-$(DOTFILE_IMAGE): linux Dockerfile.in
-	@sed -e 's|UPSTREAM_REPO|$(UPSTREAM_REPO)|g' Dockerfile.in > .dockerfile
+container: $(wildcard Dockerfile*)
+    # UPSTREAM_REPO in the Dockerfile.in will be changed to the value from Makefile; this is deprecated.
+    # There's no reason not to just use Dockerfile now.
+	@if [ -f Dockerfile.in ]; then sed -e 's|UPSTREAM_REPO|$(UPSTREAM_REPO)|g' Dockerfile.in > .dockerfile; else cp Dockerfile .dockerfile; fi
+	# Add information about the commit into .docker_image, to be added to the build.
 	@echo "$(DOCKER_REPO):$(VERSION) commit=$(shell git describe --tags --always)"  >.docker_image
+	# Add the .docker_image into the build so it's easy to figure out where a docker image came from.
 	@echo "ADD .docker_image /$(SANITIZED_DOCKER_REPO)_VERSION_INFO.txt" >>.dockerfile
 	docker build -t $(DOCKER_REPO):$(VERSION) $(DOCKER_ARGS) -f .dockerfile .
-	@docker images -q $(DOCKER_REPO):$(VERSION) > $@
+	@docker images -q $(DOCKER_REPO):$(VERSION) >/dev/null
+
 
 container-name:
 	@echo "container: $(DOCKER_REPO):$(VERSION)"

tests/pkg/clean/build_tools_test.go

@@ -263,6 +263,27 @@ func TestGoMetalinter(t *testing.T) {
 	a.Contains(string(v), "this value of err is never used (SA4006) (staticcheck)")
 
 	// Test "make SRC_DIRS=pkg/clean codecoroner" to limit to just clean directories
-	_, err = exec.Command("make", "gometalinter", "SRC_DIRS=pkg/clean").Output()
-	a.NoError(err) // Should have no complaints in clean package
+	out, err := exec.Command("make", "gometalinter", "SRC_DIRS=pkg/clean").Output()
+	a.NoError(err, "Failed to get clean result for gometalinter: %v (output=%s)", err, out) // Should have no complaints in clean package
+}
+
+// Test golangci-lint.
+func TestGolangciLint(t *testing.T) {
+	a := assert.New(t)
+	if runtime.GOOS == "windows" {
+		t.Skip("Skipping TestGolangciLint on Windows; golangci-lint fails with dockertoolbox, see https://github.com/golangci/golangci-worker/blob/caca2738602c324b1a1d6633ad959aa6d883f2df/app/analyze/executors/temp_dir_shell.go#L27")
+	}
+
+	// Test "make gometalinter"
+	v, err := exec.Command("make", "golangci-lint").Output()
+	a.Error(err) // Should complain about pretty much everything in the dirtyComplex package.
+	a.Contains(string(v), "don't use MixedCaps in package name; dirtyComplex should be dirtycomplex")
+	a.Contains(string(v), "don't use underscores in Go names; func DummyExported_function should be DummyExportedFunction (golint)")
+	a.Contains(string(v), "File is not gofmt-ed with -s (gofmt)")
+	a.Contains(string(v), "ineffectual assignment to `num` (ineffassign)")
+	a.Contains(string(v), "yetAnotherExportedFunction` is unused (deadcode)")
+	a.Contains(string(v), "Error return value of `os.Chown` is not checked (errcheck)")
+
+	out, err := exec.Command("make", "golangci-lint", "SRC_DIRS=pkg/clean").Output()
+	a.NoError(err, "Failed to get clean result for golangci-lint: %v (output=%s)", err, out) // Should have no complaints in clean package
 }