'unused' recipe in Makefile produces confusing messages for users

[LeGEC]

Host operating system: output of uname -a

Linux legec-laptop 5.4.0-74-generic #83-Ubuntu SMP Sat May 8 02:35:39 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

mysqld_exporter version: output of mysqld_exporter --version

mysqld_exporter, version 0.13.0 (branch: master, revision: ad2847c)
build date: 20210616-12:56:48
go version: go1.16.3
platform: linux/amd64

MySQL server version

n.a.

What did you do that produced an error?

make

What did you expect to see?

a successful build

What did you see instead?

>> running check for unused/missing packages in go.mod
GO111MODULE=on go mod tidy
...
<a long diff, see below>
...
make: *** [Makefile.common:214: common-unused] Error 1

---

The default make target includes unused :

mysqld_exporter/Makefile.common

Lines 120 to 121 in ad2847c

	.PHONY: common-all
	common-all: precheck style check_license lint unused build test

The unused target runs go mod tidy (when GO11MODULE is set, which is the default choice), and runs a git diff check on files go.mod, go.sum and vendor/ to confirm that nothing has changed :

mysqld_exporter/Makefile.common

Lines 205 to 222 in ad2847c

	.PHONY: common-unused
	common-unused: $(GOVENDOR)
	ifdef GOVENDOR
	@echo ">> running check for unused packages"
	@$(GOVENDOR) list +unused | grep . && exit 1 || echo 'No unused packages'
	else
	ifdef GO111MODULE
	@echo ">> running check for unused/missing packages in go.mod"
	GO111MODULE=$(GO111MODULE) $(GO) mod tidy
	ifeq (,$(wildcard vendor))
	@git diff --exit-code -- go.sum go.mod
	else
	@echo ">> running check for unused packages in vendor/"
	GO111MODULE=$(GO111MODULE) $(GO) mod vendor
	@git diff --exit-code -- go.sum go.mod vendor/
	endif
	endif
	endif

The issues are :

• there is no message explaining to the user what to make of this output
• this action does modify go.mod and go.sum, so running make build afterwards, for example, will not produce the same binary as what the end user would expect
• interestingly, this makes the make action depend on the presence of git (not a big dependency issue, but still), and this action will not work if you aren't within a git clone of the project (no git archive tarball for you)

About the git part : a colleague of mine had deleted the .git/ directory after cloning, which turned the command git diff --exit-code -- go.sum go.mod into a diff between the two files go.sum and go.mod :

GO111MODULE=on go mod tidy
diff --git a/go.sum b/go.mod
index 079bfb9..11c6d5e 100644
--- a/go.sum
+++ b/go.mod
@@ -1,444 +1,17 @@
-cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
-cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
-github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
...
-honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
-sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
-sourcegraph.com/sourcegraph/appdash v0.0.0-20190731080439-ebfcffb1b5c0/go.mod h1:hI742Nqp5OhwiqlzhgfbWU4mW4yO10fP+LoT9WOswdU=
+module github.com/prometheus/mysqld_exporter
+
+require (
+	github.com/DATA-DOG/go-sqlmock v1.5.0
+	github.com/go-kit/kit v0.10.0
+	github.com/go-sql-driver/mysql v1.6.0
+	github.com/prometheus/client_golang v1.10.0
+	github.com/prometheus/client_model v0.2.0
+	github.com/prometheus/common v0.24.0
+	github.com/prometheus/exporter-toolkit v0.5.1
+	github.com/satori/go.uuid v1.2.0
+	github.com/smartystreets/goconvey v1.6.4
+	gopkg.in/alecthomas/kingpin.v2 v2.2.6
+	gopkg.in/ini.v1 v1.62.0
+)
+
+go 1.13
make: *** [Makefile.common:214: common-unused] Error 1

which lead to some unhelpful head scratching before figuring out what it was.

From what I understand, this action is useful for maintainers of the project, but pretty unhelpful for users who just wish to clone and use it, and makes for a weird blocker.

Possible suggestions :

• remove unused from default make targets, add some other action to check for unused (another make target ? a script ? a pre-push hook ? ...)
• keep unused in default targets, but instruct regular users to run make build && make test in Readme.md

additionally :

• display a more helpful message when this action fails (non go users will have a hard time understanding what is displayed, and what to make out of it)

[LeGEC]

Some other issues show users confused by this message :

#534 - build failed on FreeBSD
part of his issue is to have run go mod tidy when no version of go was specified in the go.mod he had

#532 - Build failed on golang docker image
for some minor changes in go.sum

#431 - Build failed on centos7.6
interestingly, the only changes mentioned in the diff are not changes in the modules, but changes in the order of the lines within the vendor/modules.txt file