Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:You can indicate your approval by writing |
| fi | ||
|
|
||
| which dep >/dev/null || go get -u github.com/golang/dep/cmd/dep | ||
| which ${GOPATH}/bin/dep >/dev/null || go get -u github.com/golang/dep/cmd/dep |
There was a problem hiding this comment.
you mean $GOROOT? $GOPATH has many paths.
There was a problem hiding this comment.
Thanks Kuat, GOPATH/bin/dep is being used everywhere, I guess we should change all of them
There was a problem hiding this comment.
Is there a problem with just which dep and installing it if necessary?
There was a problem hiding this comment.
The following is simpler and should be sufficient:
[ -f ${GOPATH}/bin/dep ] || go get -u github.com/golang/dep/cmd/dep
GOPATH won' t have many paths given the "export GOPATH=$TOP" that happens a few lines above.
There was a problem hiding this comment.
whoa, why do we override GOPATH? that seems counter to go practice. it's a standard practice to use system vendored libraries to make local changes instead of using dep.
There was a problem hiding this comment.
The top-level Makefile does it too. It seems like a reasonable thing to do from a perspective of build hermetics and reproducibility.
There was a problem hiding this comment.
We can isolate the host OS with a container or a VM and use standard go practices.
Doing peculiar things with go tool seems to fall into the bazel trap of being overly opinionated about the build environments.
There was a problem hiding this comment.
I see GOPATH set in Makefile
# Make sure GOPATH is set based on the executing Makefile and workspace.
# Will override GOPATH from the env.
export GOPATH= $(shell cd ../../..; pwd)
check for dep in GOPATH