fix check_go_path when it has spaces in it

[Stebalien]

No description provided.

[Stebalien]

So, circleci doesn't have realpath... Is that really necessary?

[magik6k]

So, circleci doesn't have realpath... Is that really necessary?

Doesn't pwd always return full normalized paths? (if we want to resolve symlinks we can use pwd -P)

[magik6k]

CI is unhappy about this, probably want to switch to #!/usr/bin/env bash

[djdv]

This brings back an old issue #2833
Reference: http://www.mingw.org/wiki/Posix_path_conversion

make build
bin/check_go_version 1.10
GOPATH="C:\Users\Dominic Della Valle\Projects\Go" bin/check_go_path github.com/ipfs/go-ipfs
go-ipfs must be built from within your $GOPATH directory.
expected within 'C:\Users\Dominic Della Valle\Projects\Go' but got '/c/Users/Dominic Della Valle/Projects/Go/src/github.com/ipfs/go-ipfs'
make: *** [mk/golang.mk:69: check_go_path] Error 1

go env returns a Windows native path, while pwd returns a posix path, so they can't easily be compared.

[djdv]

We can have the platform do the path conversion for us and store it. I'm not sure if there's a less hacky way to handle it.

diff --git a/bin/check_go_path b/bin/check_go_path
index 4f8c0025f..9ddc8f29d 100755
--- a/bin/check_go_path
+++ b/bin/check_go_path
@@ -6,6 +6,14 @@ PKG="$1"

 DIR="$(pwd -P)"
 GOPATH="$(go env GOPATH)"
+PLATFORM=$(uname -o)
+
+#Win <-> POSIX path conversion
+if [ "$PLATFORM" = "Msys" ] || [ "$PLATFORM" = "Cygwin" ] ; then
+    pushd "$GOPATH"
+    GOPATH="$(pwd -P)"
+    popd
+fi

 while read -d ':' p; do
     if ! cd "$p/src/$PKG" 2>/dev/null; then

[Stebalien]

@djdv That's just a bad error message. In that while loop, it cds into each gopath directory and uses pwd -P. Hoewver, I'm guessing that the cd 'C:\Users\Dominic Della Valle\Projects\Go/src/github.com/ipfs/go-ipfs' is throwing things off. Does switching path separators like that cause problems?

[Stebalien]

@djdv does the latest change fix it on windows?

[djdv]

@Stebalien
Negative

>make build
bin/check_go_version 1.10
GOPATH="C:\Users\Dominic Della Valle\Projects\Go" bin/check_go_path github.com/ipfs/go-ipfs
bin/check_go_path: line 11: cd: C: No such file or directory
bin/check_go_path: line 11: cd: UsersDominic Della ValleProjectsGo: No such file or directory
go-ipfs must be built from within your $GOPATH directory.
expected within 'C:\Users\Dominic Della Valle\Projects\Go' but got '/c/Users/Dominic Della Valle/Projects/Go/src/github.com/ipfs/go-ipfs'
make: *** [mk/golang.mk:69: check_go_path] Error 1

I should mention the patch I supplied, does resolve the issue for me.

[djdv]

Does switching path separators like that cause problems?

It shouldn't for things like cd, both in cmd.exe and Msys's bash. They get normalized to 1 or the other depending on the shell. cmd:\, bash:/

>cd "C:/Users\Dominic Della Valle"
>cd
C:\Users\Dominic Della Valle

>pwd
/c/Users/Dominic Della Valle

[Stebalien]

Oh. I forgot that windows uses ; as the path separator.

[Stebalien]

The problem with your patch is it won't work if GOPATH has multiple entries (which it technically can). Could you try again?

[djdv]

With the latest

make build
bin/check_go_version 1.10
GOPATH="C:\Users\Dominic Della Valle\Projects\Go" bin/check_go_path github.com/ipfs/go-ipfs
go-ipfs must be built from within your $GOPATH directory.
expected within 'C:\Users\Dominic Della Valle\Projects\Go' but got '/c/Users/Dominic Della Valle/Projects/Go/src/github.com/ipfs/go-ipfs'
make: *** [mk/golang.mk:69: check_go_path] Error 1

GOPATH has multiple entries

Ahh yes.

I suppose we could do translation inside the loop, for each element in GOPATH.
The cygpath tool seems appropriate for this and comes with the base system for Cygwin and Msys.
https://cygwin.com/cygwin-ug-net/cygpath.html

i.e.
cygpath "$(go env GOPATH)" would translate C:\Users\Dominic Della Valle\Projects\Go to /c/Users/Dominic Della Valle/Projects/Go which should match when we compare with the output of pwd.
So we can just call cygpath on each path during the compare.

[Stebalien]

I suppose we could do translation inside the loop, for each element in GOPATH.

I'm doing that by by cding into the directory in question and using pwd -P.

Try again (force pull)?

[Stebalien]

@djdv I'm reassigning this to you as this is turning into debug by trial end error. The only requirements are:

1. Works with GOPATHs with multiple entries.
2. Works on windows with or without cygwin (e.g., from git bash (?)).
3. Works on linux.

[djdv]

@Stebalien
I was a bit scatterbrained yesterday, mixing up bash and make semantics.
The issue was around \ acting as an escape character in the call to read, as well as a hardcoded :.
Does the current patch seem acceptable?

[Stebalien]

LGTM. @djdv, can you sign off? I can't sign of on my own PR.