cmd/go: very difficult to work with private Github repositories #9697

Closed
zenazn opened this Issue Jan 27, 2015 · 14 comments

Projects

None yet

8 participants

@zenazn
Contributor
zenazn commented Jan 27, 2015

The new custom import path checking behavior introduced in Go 1.4 makes it extremely difficult to work with private Github repositories.

Before Go 1.4, the traditional way to work with a private Github repository was to run something similar the following:

git config --global url."git@github.com:".insteadOf "https://github.com/"

This would allow go get and friends to transparently work as expected, automatically rewriting https URLs to use SSH for auth. This worked both when pushing and pulling.

However, in Go 1.4 this setting causes fun things like this to happen:

~$ go get -u github.com/tools/godep
package github.com/tools/godep: github.com/tools/godep is a custom import path for https://github.com/tools/godep, but /Users/carl/go/src/github.com/tools/godep is checked out from git@github.com:tools/godep
~/go/src/github.com/tools/godep$ git config remote.origin.url
https://github.com/tools/godep

(while I've chosen a public repository as my example here, this occurs on private repos as well, meaning that it's not sufficient to use a non---global setting)

I've been loosely following some discussion (on go-nuts, I believe) that advocates for pushInsteadOf: while this works well for contributors for open-source repositories, it doesn't help in situations in which both push and pull access to a repository need authentication.

Is there a suggested course of action here?

@zenazn zenazn changed the title from cmd go: very difficult to work with private Github repositories to cmd/go: very difficult to work with private Github repositories Jan 27, 2015
@slimsag
slimsag commented Jan 27, 2015

It sounds to me like the go1.4 import path checking feature is looking at the git repository to determine where it was cloned from. I wonder if a viable alternative would be to have it instead:

  • Compare:
    • The import path comment // import "rsc.io/pdf"
    • Against the package directory inside $GOPATH $GOPATH/src/rsc.io/pdf

This would still have it block people from doing go get github.com/rsc/pdf -- as that would place it inside $GOPATH/src/github.com/rsc/pdf (not the same as the import comment).

I don't know enough about this to know if this is a good solution, or why testing against the git repo's remote URL was chosen (assuming I am correct on this).

@adg
Contributor
adg commented Jan 27, 2015

Just use go get -f to bypass the check.

@zenazn
Contributor
zenazn commented Jan 27, 2015

I'm aware I can bypass the check, and I've done so a great many times. It doesn't make the behavior any less frustrating, and it only helps when I'm running go get -u myself (e.g., any scripts I use now need to be modified to pass -f).

I think at least part of the problem is that I expect go get foo.com/a/brand/new/package to put the package in a sensible place that the rest of the Go toolchain (including go get -u) will be okay with. It's extremely surprising to me that this invariant has been broken in 1.4.

@nightlyone
Contributor

@adg it is really not practical (and I guess also not desirable) to fix all automation around go get out there to always use go get -f to ensure the tooling doesn't break for private repositories.

Would a fix for this accepted, if provided?

Please consider that github.com private repositories are a major use-case. Actually so major, that this is basically Githubs business model in a nutshell.

@adg
Contributor
adg commented Jan 27, 2015

What would the fix look like?

On 27 January 2015 at 22:30, Ingo Oeser notifications@github.com wrote:

@adg https://github.com/adg it is really not practical (and I guess
also not desirable) to fix all automation around go get out there to
always use go get -f to ensure the tooling doesn't break for private
repositories.

Would a fix for this accepted, if provided?

Please consider that github.com private repositories are a major
use-case. Actually so major, that this is basically Githubs business model
in a nutshell.


Reply to this email directly or view it on GitHub
#9697 (comment).

@nightlyone
Contributor

@adg just use git config remote.origin.url instead of git remote -v. This also halves the code, as we need no parsing, just a check against supported schemes.

diff --git a/src/cmd/go/vcs.go b/src/cmd/go/vcs.go
index 1cac613..302c990 100644
--- a/src/cmd/go/vcs.go
+++ b/src/cmd/go/vcs.go
@@ -122,32 +122,21 @@ var vcsGit = &vcsCmd{
 }

 func gitRemoteRepo(vcsGit *vcsCmd, rootDir string) (remoteRepo string, err error) {
-       outb, err := vcsGit.runOutput(rootDir, "remote -v")
+       cmd := "config remote.origin.url"
+       errParse := errors.New("unable to parse output of git " + cmd)
+
+       outb, err := vcsGit.runOutput(rootDir, cmd)
        if err != nil {
                return "", err
        }
-       out := string(outb)
-
-       // Expect:
-       // origin       https://github.com/rsc/pdf (fetch)
-       // origin       https://github.com/rsc/pdf (push)
-       // use first line only.
-
-       if !strings.HasPrefix(out, "origin\t") {
-               return "", fmt.Errorf("unable to parse output of git remote -v")
-       }
-       out = strings.TrimPrefix(out, "origin\t")
-       i := strings.Index(out, "\n")
-       if i < 0 {
-               return "", fmt.Errorf("unable to parse output of git remote -v")
-       }
-       out = out[:i]
-       i = strings.LastIndex(out, " ")
-       if i < 0 {
-               return "", fmt.Errorf("unable to parse output of git remote -v")
+       repoUrl := strings.TrimSpace(string(outb))
+       for _, s := range vcsGit.scheme {
+               if strings.HasPrefix(repoUrl, s) {
+                       return repoUrl, nil
+               }
        }
-       out = out[:i]
-       return strings.TrimSpace(string(out)), nil
+       return "", errParse
+
 }
@adg
Contributor
adg commented Jan 28, 2015

@zenazn I'm confused by this:

I think at least part of the problem is that I expect go get foo.com/a/brand/new/package to put the package in a sensible place that the rest of the Go toolchain (including go get -u) will be okay with. It's extremely surprising to me that this invariant has been broken in 1.4.

Are you saying that you can run "go get importpath" and then "go get -u importpath" and receive that error message? That shouldn't happen. I don't understand how it could happen. Or are you saying that if you run the first command with a pre-Go 1.4 go tool, and the latter with the Go 1.4 go tool, you receive the error?

@nightlyone that seems like a reasonable fix. What say you @rsc?

@zenazn
Contributor
zenazn commented Jan 29, 2015

@adg—yeah. Run the git config line from my first message, then go get any package you like on Github, then go get -u it. Running Go 1.4.1 the whole time.

@adg
Contributor
adg commented Jan 29, 2015

@nightlyone want to send that change as a CL?

@nightlyone
Contributor

@adg mailed out https://golang.org/cl/3504 for review

@adg adg added a commit that closed this issue Feb 20, 2015
@nightlyone @adg nightlyone + adg cmd/go: simplify/fix handling private github repos
Before Go 1.4, the traditional way to work with a private Github
repository was to run something similar the following:

```
git config --global url."git@github.com:".insteadOf "https://github.com/"
```

It would allow go get and friends to transparently work as expected,
automatically rewriting https URLs to use SSH for auth. This worked both
when pushing and pulling.

In Go 1.4 this broke, now requiring the use of `go get -f` instead of `go get`
in order to fetch private repositories. This seems neither intended nor
practical, as it requires changing a lot of tooling.

So just use `git config remote.origin.url` instead of `git remote -v` as
this reflects the actual substitution intended in the `insteadOf` config
directive.

Also remove now useless parsing.

Also add a check against supported schemes to avoid errors in later
commands using this URL and expecting such a scheme.

Fixes #9697

Change-Id: I907327f83504302288f913a68f8222a5c2d673ee
Reviewed-on: https://go-review.googlesource.com/3504
Reviewed-by: Andrew Gerrand <adg@golang.org>
668762c
@adg adg closed this in 668762c Feb 20, 2015
@christophercurrie

I believe that this fix is incomplete. With a .gitconfig file configured as stated in the issue, the first go get is successful, a second reports an error:

$ go get -u github.com/tools/godep
$ go get -u github.com/tools/godep
package github.com/tools/godep: github.com/tools/godep is a custom import path for https://github.com/tools/godep, but /Users/codemonkey/Projects/gopath/src/github.com/tools/godep is checked out from git@github.com:tools/godep

This creates challenges when, for example, running go get -u from a repository one is working in cause dependencies to be updated.

@ungerik
ungerik commented May 4, 2015

Please, just make it work like it did before 1.4. The change in CLI behavior breaks a lot of scripts.

@ianlancetaylor
Contributor

@christophercurrie @ungerik Please open a new issue to discuss the specific issue you want addressed. This issue has been fixed and closed. Thanks.

@christophercurrie

@ianlancetaylor The current fix is incomplete, which is why I continued the thread here, to retain that context. However, per your request I have created #10791 to track this continuing issue.

@dkulchenko dkulchenko referenced this issue in dkulchenko/bunch May 26, 2015
Closed

support for private github repos #21

@gopherbot gopherbot locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.