Skip to content
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

Revert "Makefile, bin: Support multiple GOPATH components" #2862

Merged
merged 1 commit into from
Jun 17, 2016

Conversation

ghost
Copy link

@ghost ghost commented Jun 17, 2016

This reverts commit ff75bc0,
which broke building on windows. See #2833.

cc @karalabe @djdv @jefft0

@ghost ghost added the regression label Jun 17, 2016
This reverts commit ff75bc0,
which broke building on windows. See #2833.

License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
@ghost ghost force-pushed the revert-multi-gopath branch from f6c62c7 to ad461c6 Compare June 17, 2016 13:32
@Kubuxu
Copy link
Member

Kubuxu commented Jun 17, 2016

Random test failure 😃

@djdv
Copy link
Contributor

djdv commented Jun 17, 2016

Works on my machine. 👌

However, I don't know if this revert is necessary. The recommended ways of building on Windows (https://github.com/ipfs/go-ipfs/blob/master/docs/windows.md) are unaffected by ff75bc0, the only broken case is when using make outside of Cygwin or an msys terminal (i.e. using the native Windows command line).

Reverting would obviously break multipath support, so It may be better to keep that in for now and fix native Windows building later on top of that commit rather than reverting.

Thoughts on this?

@jefft0
Copy link
Contributor

jefft0 commented Jun 17, 2016

It was broken for me inside Cygwin, as I reported #2851 .

@Kubuxu
Copy link
Member

Kubuxu commented Jun 17, 2016

Does it only affect native terminal? If yes them I am also against it. cygwin tools make are supposed to be used in cygwin terminal and environment.

@jefft0
Copy link
Contributor

jefft0 commented Jun 17, 2016

It was broken inside a Cygwin terminal.

@djdv
Copy link
Contributor

djdv commented Jun 17, 2016

Disregard that, thanks @jefft0, I had not seen the breakage in Cygwin. I am in favor of this revert.

@whyrusleeping
Copy link
Member

@lgierth do we have consensus here? Is this good to merge?

@ghost
Copy link
Author

ghost commented Jun 17, 2016

Yes I think there's consensus

@whyrusleeping whyrusleeping merged commit 2a3bba3 into master Jun 17, 2016
@whyrusleeping whyrusleeping deleted the revert-multi-gopath branch June 17, 2016 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants