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 · 14 comments
Open

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

sjudeng opened this issue May 8, 2020 · 14 comments

Comments

@sjudeng
Copy link

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
@mrgleeco
Copy link

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
Contributor

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.

@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. modules labels May 14, 2020
@sjudeng
Copy link
Author

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 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 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 (old) Jun 10, 2020
@rsc
Copy link
Contributor

rsc commented Aug 18, 2021

Allowing users to override the tooling would also fragment the ecosystem, at least for public packages, because not everyone would know to override the tooling. (See also #38964 (comment).)

Is the .git extension not working?

@rsc
Copy link
Contributor

rsc commented Aug 18, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Aug 18, 2021
@sjudeng
Copy link
Author

sjudeng commented Aug 20, 2021

Hi thanks for picking this up and considering. The .git extension does work but it needs to be included in the module import path across all files and repos. That's consistent with docs so this would be a new feature. The goal is to provide an additional mechanism to signal the vcs system without requiring inclusion of the .git extension. Understand the portability concerns. The use case here is for development teams that are sharing private modules through internal repo servers and would choose such an alternate approach even if it meant requiring their developers to customize their environment to accommodate the configuration.

@rsc
Copy link
Contributor

rsc commented Aug 25, 2021

for development teams that are sharing private modules through internal repo servers and would choose such an alternate approach even if it meant requiring their developers to customize their environment to accommodate the configuration.

In this case it is probably even easier to set up an HTTP redirector serving the custom domain meta tags.

@tandr
Copy link

tandr commented Aug 27, 2021

In this case it is probably even easier to set up an HTTP redirector serving the custom domain meta tags.

Sorry @rsc - I think you overestimate capability of a smallish dev team to push through all the approvals and frictions in a not-dev-oriented enterprise to have anything like this done. Each interaction with IT in medium-large organizations is an uphill battle that nobody enjoys. Making a go.mod.local file (for example) or changing something in a environment is incomparable easier.

(maybe a bit out of scope for a current discussion, but please allow me to run with an idea - if Go's tooling would have a capability to have go.*.local files to augment/override corresponding parts in go.*' files, it might go a long way to help in a scenario like "checkout repo, have gitignore line for go.*.local, have go.*.local files produced by manually or by script, then commit changes without need to exclude your local env overrides every time.'. )

@rsc
Copy link
Contributor

rsc commented Sep 8, 2021

Talked to @jayconrod, @bcmills, @matloob. It sounds like this general need fits into the overall enterprise authentication question, and maybe a broader solution would be better. That said, we don't know what the broader solution is either.

The .git extension does work but it needs to be included in the module import path across all files and repos.

For what it's worth, you'd need to use the same import path across all files and repos no matter what. It's just that right now that path has to include a .git.

@rsc
Copy link
Contributor

rsc commented Sep 15, 2021

Putting on hold for the broader enterprise story. For now my suggestion is to keep using .git in the paths.

@rsc rsc moved this from Active to Hold in Proposals (old) Sep 15, 2021
@rsc
Copy link
Contributor

rsc commented Sep 15, 2021

Placed on hold.
— rsc for the proposal review group

@ekle

This comment was marked as off-topic.

@joedian joedian removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Hold
Development

No branches or pull requests

10 participants