-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Makefile, bin: Support multiple GOPATH components #2808
Conversation
I'm generally cool with this change, but we should be aware it introduces a build dependency on realpath. The previous version used make's built-in realpath. More thoughts? |
Yup, just noticed it myself too. Will try to work around it. |
@lgierth Moved Edit: The teamcity failure seems to be something unrelated. But please correct me if I'm wrong and actually broke something. |
You are right (just for tracking: #2809) |
License: MIT Signed-off-by: Péter Szilágyi <peterke@gmail.com>
Yup, force pushed to rebuild and all tests are green now. :D |
Cool you pulled it off :) I can't speak for your multiple-gopaths scenario, but I tested with my own symlinked-into-gopath scenario and it still works fine. LGTM |
@chriscool and @Kubuxu could you CR this? i want to always double check changes that affect the build system |
It looks good to me. I hope there isn't another GOPATH edge case waiting around the coroner 😄. |
@Kubuxu my sentiments exactly, lol. Its looks good to me, but theres always something tricky going on. its hard to be sure |
It LGTM too. Thanks @karalabe ! |
Go's
GOPATH
environment variable can contain multiple paths, separated by a colon:
similar to thePATH
env var. In such cases Go uses the first path element to install any packages, however the remaining paths are still used to find dependencies. Because of this it's quite a common setup to have multiple paths listed in GOPATH (e.g. one for development and another for appengine, which has its special installer).The current Makefile however erroneously assumes that
GOPATH
contains a single path element, and tries to locate go-ipfs within by appending a string toGOPATH
, which is obviously cause an invalid path, and hence fail thepath_check
rule in the Makefile, even though the required condition is met.This PR fixes it by moving path handling back intobin/check_go_path
, which iterates over all the path components ofGOPATH
, trying each one of them for the go-ipfs match and only failing if none matches the requirements. Further it was kind of weird to pass inPWD
as a parameter to a script, so I moved the first parameter's evaluation into the script as well.This PR fixes it by splitting up GOPATH along the colon characters, appending the go-ipfs suffix to each component and resolving them individually. The possibly multiple resulting paths are passed to the path verification script to iterate over and check individually.