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

proposal: cmd/go: configurable extra vcs paths #38964

Open
sjudeng opened this issue May 8, 2020 · 5 comments
Open

proposal: cmd/go: configurable extra vcs paths #38964

sjudeng opened this issue May 8, 2020 · 5 comments

Comments

@sjudeng
Copy link

@sjudeng sjudeng commented May 8, 2020

Go module vcs identification is currently based on an array of regular expressions (vcsPaths) for known hosting platforms.

var vcsPaths = []*vcsPath{
// Github
{
prefix: "github.com/",
regexp: lazyregexp.New(`^(?P<root>github\.com/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(/[\p{L}0-9_.\-]+)*$`),
vcs: "git",
repo: "https://{root}",
check: noVCSSuffix,
},
// Bitbucket
{
prefix: "bitbucket.org/",
regexp: lazyregexp.New(`^(?P<root>bitbucket\.org/(?P<bitname>[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+))(/[A-Za-z0-9_.\-]+)*$`),
repo: "https://{root}",
check: bitbucketVCS,
},
// IBM DevOps Services (JazzHub)
{
prefix: "hub.jazz.net/git/",
regexp: lazyregexp.New(`^(?P<root>hub\.jazz\.net/git/[a-z0-9]+/[A-Za-z0-9_.\-]+)(/[A-Za-z0-9_.\-]+)*$`),
vcs: "git",
repo: "https://{root}",
check: noVCSSuffix,
},
// Git at Apache
{
prefix: "git.apache.org/",
regexp: lazyregexp.New(`^(?P<root>git\.apache\.org/[a-z0-9_.\-]+\.git)(/[A-Za-z0-9_.\-]+)*$`),
vcs: "git",
repo: "https://{root}",
},
// Git at OpenStack
{
prefix: "git.openstack.org/",
regexp: lazyregexp.New(`^(?P<root>git\.openstack\.org/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(\.git)?(/[A-Za-z0-9_.\-]+)*$`),
vcs: "git",
repo: "https://{root}",
},
// chiselapp.com for fossil
{
prefix: "chiselapp.com/",
regexp: lazyregexp.New(`^(?P<root>chiselapp\.com/user/[A-Za-z0-9]+/repository/[A-Za-z0-9_.\-]+)$`),
vcs: "fossil",
repo: "https://{root}",
},
// General syntax for any server.
// Must be last.
{
regexp: lazyregexp.New(`(?P<root>(?P<repo>([a-z0-9.\-]+\.)+[a-z0-9.\-]+(:[0-9]+)?(/~?[A-Za-z0-9_.\-]+)+?)\.(?P<vcs>bzr|fossil|git|hg|svn))(/~?[A-Za-z0-9_.\-]+)*$`),
schemelessRepo: true,
},
}
There is a generic fallback regular expression that requires the vcs name be included as a suffix (.git) in the module path. Otherwise Go falls back to a dynamic check that fails if the server doesn't allow HTTPS access or else has non-standard connection/authentication requirements associated with HTTPS access that can't be negotiated.
func repoRootForImportDynamic(importPath string, mod ModuleMode, security web.SecurityMode) (*RepoRoot, error) {
url, err := urlForImportPath(importPath)
if err != nil {
return nil, err
}
resp, err := web.Get(security, url)
if err != nil {
msg := "https fetch: %v"
if security == web.Insecure {
msg = "http/" + msg
}
return nil, fmt.Errorf(msg, err)
}

This proposal is to add support for cmd/go configuration to provide extra vcsPaths for use in mapping servers to vcs. The goal is to provide more flexibility for using go modules with smaller/internal repository servers while adding minimal complexity to the go command. By providing users with an option to specify the vcs for a module Go can delegate all further module download operations to the relevant client (git) and bypass the dynamic fallback function that is problematic for SSH-only and some HTTPS repository server deployments.

Example: Current behavior

Module: some.internal.com/agroup/depmod
VCS: Git (prefer ssh)

Attempt 1

git config --global url."git@some.internal.com:".insteadOf "https://some.internal.com"
export GOPRIVATE="some.internal.com"
# if applicable, add the following to $HOME/.netrc
# machine some.internal.com login YOU password APIKEY
go get some.internal.com/agroup/depmod
  • repo is not identified as git through vcsPaths
  • dynamic fallback fails with "unrecognized import path...https fetch" if the server only allows SSH access or otherwise uses a more custom HTTPS access/authentication implementation

Attempt 2

git config --global url."git@some.internal.com:".insteadOf "https://some.internal.com"
export GOPRIVATE="some.internal.com"
# add login credentials to $HOME/.netrc, if applicable
go get some.internal.com/agroup/depmod.git
  • repo is identified as git through vcsPaths
  • all download requests are delegated to git client, which is configured to use SSH as above -> download succeeds
  • parsing fails unless module path also includes .git suffix

Attempt 3 (hack)

# configure git to map fake github.com/some-group prefix to git@some.internal.com
git config --global url."git@some.internal.com:".insteadOf "https://github.com/some-group"
export GOPRIVATE="github.com/some*"
# module path includes fake github.com/some-group prefix
go get github.com/some-group/depmod
  • repo is identified as git through vcsPaths
  • download requests delegated to git client -> download succeeds
  • parsing succeeds as long as module path is consistent but this requires using a fake path prefix

Example: Proposed

git config --global url."git@some.internal.com:".insteadOf "https://some.internal.com"
export GOPRIVATE="some.internal.com"
export GOVCSEXT="root:some.internal.com:vcs:git"
go get some.internal.com/agroup/depmod
  • repo is identified as git through vcsPaths
  • download requests delegated to git client -> download succeeds
  • no module path mismatch -> parsing succeeds

Alternatives

Local go get config

VCS as protocol prefix in GOPRIVATE

  • export GOPRIVATE="git://some.internal.com"
@gopherbot gopherbot added this to the Proposal milestone May 8, 2020
@gopherbot gopherbot added the Proposal label May 8, 2020
@mrgleeco
Copy link

@mrgleeco mrgleeco commented May 8, 2020

For any repo that is pure SSH and HTTP/S is not an option it would seem a step forward.

See also: gomods/athens#1450

@bcmills
Copy link
Member

@bcmills bcmills commented May 14, 2020

What exactly was the problem with attempt # 2? It's true that you need an explicit .git suffix in that case, but that support is documented (go help importpath) and it seems like a reasonable tradeoff in order to avoid an HTTPS resolver.

The idea is that the location of each path should be unambiguous without additional configuration. The proposed GOVCSEXT violates that property.

@sjudeng
Copy link
Author

@sjudeng sjudeng commented May 15, 2020

@bcmills Thanks for looking this over and for your reply. It's just a potential user experience issue. It might not be intuitive that the suffix is needed in these cases and some might opt for special configuration if it was available to avoid needing to include the suffix across all their modules. One reason for this would be to maintain more consistency between public and internal module paths, where having different host prefixes makes sense but needing to add the suffix when both are Git for example might not.

In some cases the module path without the suffix may be unambiguous, and consistent with how users access the repository in a browser or through their (git) client. In these cases I'm wondering if it's less about the ambiguity of the path and more about the current capabilities of the dynamic discovery implementation and the existing hints (common hosts, known suffixes) that Go uses to associate imports with the right backend client, and whether there's any opportunity for improvement without violating tooling/language principles.

Would using GOPRIVATE as above be any more appropriate since it's already being used during the import process?

@mtt0
Copy link

@mtt0 mtt0 commented May 19, 2020

If my vcs is using a specific port(see #38213), and ssh is unvailable, maybe I could add vcs path

// YourGit
{
  prefix: "yourgit.com/",
  regexp: lazyregexp.New(`^(?P<root>yourgit\.com)(/[\p{L}0-9_.\-]+)*$`),
  vcs:    "git",
  repo:   "https://{root}:19547",
  check:  noVCSSuffix,
},

to

var vcsPaths = []*vcsPath{
// Github
{
prefix: "github.com/",
regexp: lazyregexp.New(`^(?P<root>github\.com/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(/[\p{L}0-9_.\-]+)*$`),
vcs: "git",
repo: "https://{root}",
check: noVCSSuffix,
},

?

So, configurable extra vcs path maybe reasonable at this situation

@juicemia
Copy link

@juicemia juicemia commented Jun 1, 2020

I think this proposal is a great idea. I was actually about to open an issue about the same thing. As I was reading through this issue I managed to get it working thanks to trying different ways to add the .git extension to the import path.

However, I still think this would be a good idea.

VCS platforms like Gitlab have historically had trouble with go get and allowing users to override the tooling would make a lot of those issues disappear.

@rsc rsc added this to Incoming in Proposals Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.