Makefile: Respect GOBIN

And use 'go env GOBIN' to detect the user's existing preference.  From
[1]:

> The bin directory holds compiled commands.  Each command is named
> for its source directory, but only the final element, not the entire
> path.  That is, the command with source in DIR/src/foo/quux is
> installed into DIR/bin/quux, not DIR/bin/foo/quux.  The "foo/"
> prefix is stripped so that you can add DIR/bin to your PATH to get
> at the installed commands.  If the GOBIN environment variable is
> set, commands are installed to the directory it names instead of
> DIR/bin.  GOBIN must be an absolute path.
> ...
> Go searches each directory listed in GOPATH to find source code, but
> new packages are always downloaded into the first directory in the
> list.

So if GOBIN is set, it will be non-empty, and we can use $(GOBIN)/...

If GOBIN is unset, 'go env GOBIN' will return an empty string (as it
does on Travis [2]).  In that case, I'm assuming that the package in
question is in the first directory in GOPATH and using the new
FIRST_GOPATH (firstword and subst are documented in [3]).  That's
probably fairly safe, since our previous GOPATH handling assumed it
only contained a single path, and nobody was complaining about that.

Using ?= allows us to skip the 'dirname' call if we end up not needing
GOPKGBASEDIR [4] (e.g. for the 'help' target).  The recursive
expansion could cause an issue if the result of the shell expansions
included a '$', but those seem unlikely in GOPKGBASEDIR, GOMD2MAN, or
the manpage paths.  I haven't used ?= for GOBIN, because we'll always
need the expanded value for the if check.

Using GOMD2MAN allows us to collapse old ||-based recipe into a less
confusing invocation.  And using a static pattern rule [5] for
$(MANPAGES) lets us write a single rule to handle both section 1 and
section 5.

While I was updating the GOPATH handling, I moved .gopathok from the
possibly-shared $(GOPATH)/.gopathok to the
definitely-specific-to-this-project .gopathok.  That may cause some
issues if you rebuild after changing your GOPATH without calling
'clean', but I don't expect folks to change their GOPATH frequently.
And the old approach would fail if different consumers were also using
the same flag path to mean something else (as CRI-O does [6]).

As part of cleaning up .gopathok, I've also collapsed clean's rm calls
into a single invocation.  That will give us the same results with
less process setup/teardown penalties.

[1]: https://golang.org/cmd/go/#hdr-GOPATH_environment_variable
[2]: https://travis-ci.org/projectatomic/libpod/jobs/379345071#L459
[3]: https://www.gnu.org/software/make/manual/html_node/Text-Functions.html
[4]: https://www.gnu.org/software/make/manual/html_node/Setting.html
[5]: https://www.gnu.org/software/make/manual/html_node/Static-Usage.html
[6]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.1/Makefile#L62

Signed-off-by: W. Trevor King <wking@tremily.us>

Closes: #774
Approved by: mheon

1 files changed, 35 insertions, 34 deletions

diff --git a/Makefile b/Makefile
index e878c834c..8c05d3a75 100644
--- a/Makefile
+++ b/Makefile
@@ -36,11 +36,17 @@ ifeq ($(GOPATH),)
 export GOPATH := $(CURDIR)/_output
 unexport GOBIN
 endif
-GOPKGDIR := $(GOPATH)/src/$(PROJECT)
-GOPKGBASEDIR := $(shell dirname "$(GOPKGDIR)")
+FIRST_GOPATH := $(firstword $(subst :, ,$(GOPATH)))
+GOPKGDIR := $(FIRST_GOPATH)/src/$(PROJECT)
+GOPKGBASEDIR ?= $(shell dirname "$(GOPKGDIR)")
+
+GOBIN := $(shell go env GOBIN)
+ifeq ($(GOBIN),)
+GOBIN := $(FIRST_GOPATH)/bin
+endif
+
+GOMD2MAN ?= $(shell command -v go-md2man || echo '$(GOBIN)/go-md2man')
 
-# Update VPATH so make finds .gopathok
-VPATH := $(VPATH):$(GOPATH)
 BASE_LDFLAGS := -X main.gitCommit=${GIT_COMMIT} -X main.buildInfo=${BUILD_INFO}
 LDFLAGS := -ldflags '${BASE_LDFLAGS}'
 LDFLAGS_PODMAN := -ldflags '${BASE_LDFLAGS}'
@@ -66,7 +72,7 @@ ifeq ("$(wildcard $(GOPKGDIR))","")
 	mkdir -p "$(GOPKGBASEDIR)"
 	ln -s "$(CURDIR)" "$(GOPKGBASEDIR)"
 endif
-	touch "$(GOPATH)/.gopathok"
+	touch $@
 
 lint: .gopathok varlink_generate
 	@echo "checking lint"
@@ -94,22 +100,19 @@ python-podman:
 	$(MAKE) -C contrib/python python-podman
 
 clean:
-ifneq ($(GOPATH),)
-	rm -f "$(GOPATH)/.gopathok"
-endif
-	rm -rf _output
-	rm -f docs/*.1
-	rm -f docs/*.5
-	rm -fr test/testdata/redis-image
+	rm -rf \
+		.gopathok \
+		_output \
+		bin/podman \
+		build \
+		test/bin2img/bin2img \
+		test/checkseccomp/checkseccomp \
+		test/copyimg/copyimg \
+		test/testdata/redis-image \
+		$(MANPAGES)
+	$(MAKE) -C contrib/python clean
 	find . -name \*~ -delete
 	find . -name \#\* -delete
-	rm -f bin/podman
-	rm -f test/bin2img/bin2img
-	rm -f test/copyimg/copyimg
-	rm -f test/checkseccomp/checkseccomp
-	rm -fr build/
-	$(MAKE) -C contrib/python clean
-
 
 libpodimage:
 	docker build -t ${LIBPOD_IMAGE} .
@@ -154,14 +157,11 @@ binaries: varlink_generate podman python-podman
 
 test-binaries: test/bin2img/bin2img test/copyimg/copyimg test/checkseccomp/checkseccomp
 
-MANPAGES_MD := $(wildcard docs/*.md)
-MANPAGES    := $(MANPAGES_MD:%.md=%)
-
-docs/%.1: docs/%.1.md .gopathok
-	(go-md2man -in $< -out $@.tmp && touch $@.tmp && mv $@.tmp $@) || ($(GOPATH)/bin/go-md2man -in $< -out $@.tmp && touch $@.tmp && mv $@.tmp $@)
+MANPAGES_MD ?= $(wildcard docs/*.md)
+MANPAGES ?= $(MANPAGES_MD:%.md=%)
 
-docs/%.5: docs/%.5.md .gopathok
-	(go-md2man -in $< -out $@.tmp && touch $@.tmp && mv $@.tmp $@) || ($(GOPATH)/bin/go-md2man -in $< -out $@.tmp && touch $@.tmp && mv $@.tmp $@)
+$(MANPAGES): %: %.md .gopathok
+	$(GOMD2MAN) -in $< -out $@
 
 docs: $(MANPAGES)
 
@@ -222,35 +222,35 @@ uninstall:
 
 .PHONY: .gitvalidation
 .gitvalidation: .gopathok
-	GIT_CHECK_EXCLUDE="./vendor" $(GOPATH)/bin/git-validation -v -run DCO,short-subject,dangling-whitespace -range $(EPOCH_TEST_COMMIT)..$(HEAD)
+	GIT_CHECK_EXCLUDE="./vendor" $(GOBIN)/git-validation -v -run DCO,short-subject,dangling-whitespace -range $(EPOCH_TEST_COMMIT)..$(HEAD)
 
 .PHONY: install.tools
 
 install.tools: .install.gitvalidation .install.gometalinter .install.md2man
 
 .install.gitvalidation: .gopathok
-	if [ ! -x "$(GOPATH)/bin/git-validation" ]; then \
+	if [ ! -x "$(GOBIN)/git-validation" ]; then \
 		go get -u github.com/vbatts/git-validation; \
 	fi
 
 .install.gometalinter: .gopathok
-	if [ ! -x "$(GOPATH)/bin/gometalinter" ]; then \
+	if [ ! -x "$(GOBIN)/gometalinter" ]; then \
 		go get -u github.com/alecthomas/gometalinter; \
-		cd $(GOPATH)/src/github.com/alecthomas/gometalinter; \
+		cd $(FIRST_GOPATH)/src/github.com/alecthomas/gometalinter; \
 		git checkout 23261fa046586808612c61da7a81d75a658e0814; \
 		go install github.com/alecthomas/gometalinter; \
-		$(GOPATH)/bin/gometalinter --install; \
+		$(GOBIN)/gometalinter --install; \
 	fi
 
 .install.md2man: .gopathok
-	if [ ! -x "$(GOPATH)/bin/go-md2man" ]; then \
+	if [ ! -x "$(GOBIN)/go-md2man" ]; then \
 		   go get -u github.com/cpuguy83/go-md2man; \
 	fi
 
 .install.ostree: .gopathok
 	if ! pkg-config ostree-1 2> /dev/null ; then \
-		git clone https://github.com/ostreedev/ostree $(GOPATH)/src/github.com/ostreedev/ostree ; \
-		cd $(GOPATH)/src/github.com/ostreedev/ostree ; \
+		git clone https://github.com/ostreedev/ostree $(FIRST_GOPATH)/src/github.com/ostreedev/ostree ; \
+		cd $(FIRST_GOPATH)src/github.com/ostreedev/ostree ; \
 		./autogen.sh --prefix=/usr/local; \
 		make all install; \
 	fi
@@ -274,6 +274,7 @@ API.md: cmd/podman/varlink/io.projectatomic.podman.varlink
 validate: gofmt .gitvalidation
 
 .PHONY: \
+	.gopathok \
 	binaries \
 	clean \
 	default \