Skip to content

Commit

Permalink
build: use repository-local GOBIN for less finicky builds
Browse files Browse the repository at this point in the history
This commit adjusts our Makefile to set GOBIN to $(REPO_ROOT)/bin before
embarking upon a build. This has two benefits:

    1. Previously, users who had set GOBIN to something other than the
       default $GOPATH/bin would have a broken build. We'd extend the
       PATH to include $GOPATH/bin, but `go install` would put binaries
       in $GOBIN, and later build steps would be unable to find the
       built tools. By forcing GOBIN to a known value in the Makefile,
       users can no longer break the build by having a non-default GOBIN
       set in their environment.

    2. Since Go has a single global namespace for installed binaries
       (i.e., the GOBIN directory), if the user had installed a
       different version of one of our dependencies, or a different tool
       with a binary of the same name, we'd blow away that binary during
       our install process. Even worse, later re-installs of the other
       tool could blow away the version we installed without our build
       process noticing. Using a repository-local GOBIN solves this.

Fixes cockroachdb#6886.
  • Loading branch information
benesch committed Apr 5, 2017
1 parent 29cf4d9 commit 4fa0433
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 5 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# git config --global core.excludesfile ~/.gitignore_global)

artifacts
/bin
.bootstrap
.buildinfo
/cockroach
Expand Down
5 changes: 3 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,14 @@ install: override LINKFLAGS += \
-X "github.com/cockroachdb/cockroach/pkg/build.utcTime=$(shell date -u '+%Y/%m/%d %H:%M:%S')" \
-X "github.com/cockroachdb/cockroach/pkg/build.rev=$(shell cat .buildinfo/rev)"

# Special case: we install the cockroach binary to the user-specified GOBIN
# (usually GOPATH/bin) rather than our repository-local GOBIN.
install: GOBIN := $(ORIGINAL_GOBIN)
# Note: We pass `-v` to `go build` and `go test -i` so that warnings
# from the linker aren't suppressed. The usage of `-v` also shows when
# dependencies are rebuilt which is useful when switching between
# normal and race test builds.
install: .buildinfo/tag .buildinfo/rev
@echo "GOPATH set to $$GOPATH"
@echo "$$GOPATH/bin added to PATH"
$(GO) $(BUILDMODE) -v $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' $(BUILDTARGET)

# Build, but do not run the tests.
Expand Down
15 changes: 14 additions & 1 deletion build/common.mk
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,19 @@ export GOPATH := $(realpath $(ORG_ROOT)/../../..)
# | |~ GOPATH/src
# |~ GOPATH/src/github.com

# Avoid printing twice if Make restarts (because a Makefile was changed) or is
# called recursively from another Makefile.
ifeq ($(MAKE_RESTARTS)$(MAKELEVEL),0)
$(info GOPATH set to $(GOPATH))
endif

# We install our vendored tools to a directory within this repository to avoid
# overwriting any user-installed binaries of the same name in the default GOBIN.
# We must remember the original value of GOBIN so we can install the cockroach
# binary to the user's GOBIN instead of our repository-local GOBIN.
ORIGINAL_GOBIN := $(GOBIN)
export GOBIN := $(abspath $(REPO_ROOT)/bin)

# Prefer tools we've installed with go install and Yarn to those elsewhere on
# the PATH.
#
Expand All @@ -46,7 +59,7 @@ export GOPATH := $(realpath $(ORG_ROOT)/../../..)
# ORG_ROOT. It's much simpler to add the Yarn executable-installation directory
# to the PATH than have protobuf.mk adjust its paths to work in both ORG_ROOT
# and UI_ROOT.
export PATH := $(GOPATH)/bin:$(UI_ROOT)/node_modules/.bin:$(PATH)
export PATH := $(GOBIN):$(UI_ROOT)/node_modules/.bin:$(PATH)

# HACK: Make has a fast path and a slow path for command execution,
# but the fast path uses the PATH variable from when make was started,
Expand Down
4 changes: 2 additions & 2 deletions build/protobuf.mk
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ GOGO_PROTOBUF_TYPES_PACKAGE := $(GOGO_PROTOBUF_PACKAGE)/types
GOGO_PROTOBUF_PATH := $(GITHUB_ROOT)/../$(GOGO_PROTOBUF_PACKAGE)
PROTOBUF_PATH := $(GOGO_PROTOBUF_PATH)/protobuf

PROTOC := $(GOPATH)/bin/protoc
PROTOC := $(GOBIN)/protoc
PLUGIN_SUFFIX := gogoroach
PROTOC_PLUGIN := $(GOPATH)/bin/protoc-gen-$(PLUGIN_SUFFIX)
PROTOC_PLUGIN := $(GOBIN)/protoc-gen-$(PLUGIN_SUFFIX)
GOGOPROTO_PROTO := $(GOGO_PROTOBUF_PATH)/gogoproto/gogo.proto

COREOS_PATH := $(GITHUB_ROOT)/coreos
Expand Down
2 changes: 2 additions & 0 deletions build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ define VALID_VARS
GITHOOKSDIR
GIT_DIR
GO
GOBIN
GOFLAGS
GOPATH
GORACE
GOVERS
LINKFLAGS
MACOSX_DEPLOYMENT_TARGET
ORG_ROOT
ORIGINAL_GOBIN
PATH
PKG
PKG_ROOT
Expand Down

0 comments on commit 4fa0433

Please sign in to comment.