build: don't put .node_modules/bin on the path

Since Yarn 0.25 (yarnpkg/yarn#3310), Yarn transitively symlinks binaries
into node_modules/.bin. (Previously, it would only sync top-level
binaries--i.e., binaries that we directly depend on.) The new behavior
results in adding all sorts of junk to the path--notably, a JavaScript
which reimplementation that shadows the system-provided which. Ew.

Avoid putting node_modules/.bin on the PATH by using full paths to the
binaries within, or using `yarn run` if within pkg/ui.

Author: benesch

build/common.mk

@@ -48,14 +48,7 @@ GO_INSTALL := GOBIN='$(LOCAL_BIN)' $(GO) install
 
 # Prefer tools we've installed with go install and Yarn to those elsewhere on
 # the PATH.
-#
-# Usually, we could use the yarn run command to avoid changing the PATH
-# globally. Unfortunately, yarn run must be executed in or beneath UI_ROOT, but
-# protobuf.mk, which depends on Yarn-installed executables, needs to execute in
-# 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 := $(LOCAL_BIN):$(UI_ROOT)/node_modules/.bin:$(PATH)
+export PATH := $(LOCAL_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,

build/protobuf.mk

@@ -51,6 +51,9 @@ GW_SOURCES := $(GW_PROTOS:%.proto=%.pb.gw.go)
 GO_PROTOS := $(addprefix $(REPO_ROOT)/, $(sort $(shell git -C $(REPO_ROOT) ls-files --exclude-standard --cached --others -- '*.proto')))
 GO_SOURCES := $(GO_PROTOS:%.proto=%.pb.go)
 
+PBJS := $(UI_ROOT)/node_modules/.bin/pbjs
+PBTS := $(UI_ROOT)/node_modules/.bin/pbts
+
 UI_JS := $(UI_ROOT)/src/js/protos.js
 UI_TS := $(UI_ROOT)/src/js/protos.d.ts
 UI_SOURCES := $(UI_JS) $(UI_TS)
@@ -101,15 +104,15 @@ $(CPP_SOURCES_TARGET): $(PROTOC) $(CPP_PROTOS)
 $(UI_JS): $(GO_PROTOS) $(COREOS_RAFT_PROTOS) $(YARN_INSTALLED_TARGET)
 	# Add comment recognized by reviewable.
 	echo '// GENERATED FILE DO NOT EDIT' > $@
-	pbjs -t static-module -w es6 --strict-long --keep-case --path $(ORG_ROOT) --path $(GOGO_PROTOBUF_PATH) --path $(COREOS_PATH) --path $(GRPC_GATEWAY_GOOGLEAPIS_PATH) $(GW_PROTOS) >> $@
+	$(PBJS) -t static-module -w es6 --strict-long --keep-case --path $(ORG_ROOT) --path $(GOGO_PROTOBUF_PATH) --path $(COREOS_PATH) --path $(GRPC_GATEWAY_GOOGLEAPIS_PATH) $(GW_PROTOS) >> $@
 
 $(UI_TS): $(UI_JS) $(YARN_INSTALLED_TARGET)
 	# Install a known-good version of jsdoc; see
 	# https://github.com/dcodeIO/protobuf.js/issues/716.
 	(cd $(UI_ROOT)/node_modules/protobufjs/cli && npm install --silent jsdoc@3.4.3)
 	# Add comment recognized by reviewable.
 	echo '// GENERATED FILE DO NOT EDIT' > $@
-	pbts $(UI_JS) >> $@
+	$(PBTS) $(UI_JS) >> $@
 
 .DEFAULT_GOAL := protos
 .PHONY: protos

pkg/ui/Makefile

@@ -32,20 +32,20 @@ generate: $(GOBINDATA_TARGET)
 
 .PHONY: lint
 lint: $(YARN_INSTALLED_TARGET) | protos
-	stylint -c .stylintrc styl
-	tslint -c tslint.json -p tsconfig.json --type-check
+	yarn run -- stylint -c .stylintrc styl
+	yarn run -- tslint -c tslint.json -p tsconfig.json --type-check
 
 .PHONY: test
 test: $(YARN_INSTALLED_TARGET) | protos
-	karma start
+	yarn run -- karma start
 
 .PHONY: test-watch
 test-watch: $(YARN_INSTALLED_TARGET) | protos
-	karma start --no-single-run --auto-watch
+	yarn run -- karma start --no-single-run --auto-watch
 
 $(GOBINDATA_TARGET): $(YARN_INSTALLED_TARGET) $(shell git ls-files | grep -vF $(GOBINDATA_TARGET)) | protos
 	rm -rf dist
-	webpack -p
+	yarn run -- webpack -p
 	go-bindata -nometadata -pkg ui -o $@ -prefix dist dist/...
 	# Add comment recognized by reviewable.
 	echo '// GENERATED FILE DO NOT EDIT' >> $@