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

cmd/go: simplify scheme lookup in internal/get/vcs.go #26123

Closed
rsc opened this issue Jun 29, 2018 · 3 comments
Closed

cmd/go: simplify scheme lookup in internal/get/vcs.go #26123

rsc opened this issue Jun 29, 2018 · 3 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Jun 29, 2018

After CL 120998 is merged into the main tree, the vcs ping code will look like:

	if srv.ping {
		if scheme != "" {
			match["repo"] = scheme + "://" + match["repo"]
		} else {
			for _, scheme := range vcs.scheme {
				if security == web.Secure && !vcs.isSecureScheme(scheme) {
					continue
				}
				if vcs.pingCmd != "" && vcs.ping(scheme, match["repo"]) == nil {
					match["repo"] = scheme + "://" + match["repo"]
					goto Found
				}
			}
			// No scheme found. Fall back to the first one.
			match["repo"] = vcs.scheme[0] + "://" + match["repo"]
		Found:
		}
	}

I believe it can be simplified to:

	if srv.ping {
		scheme = vcs.scheme[0] // default to first scheme
		if vcs.pingCmd != "" {
			// If we know how to test schemes, scan to find one.
			for _, scm := range vcs.scheme {
			if security == web.Secure && !vcs.isSecureScheme(scm) {
				continue
			}
			if vcs.ping(scheme, match["repo"]) == nil {
				scheme = scm
				break
			}
		}
		match["repo"] = scheme + "://" + match["repo"]
	}

but I don't want to do that this late in the cycle. We should try the rewrite when the Go 1.12 cycle opens.

@rsc rsc added this to the Go1.12 milestone Jun 29, 2018
@rsc rsc added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Jun 29, 2018
@bcmills bcmills modified the milestones: Go1.12, Go1.13 Nov 15, 2018
@bcmills
Copy link
Contributor

bcmills commented Nov 15, 2018

This fell off my radar. Will put it on my list for early in 1.13.

@bcmills bcmills modified the milestones: Go1.13, Go1.14 May 9, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@bcmills bcmills modified the milestones: Backlog, Go1.14 Oct 10, 2019
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2019
@gopherbot
Copy link

Change https://golang.org/cl/200758 mentions this issue: cmd/go/internal/get: simplify scheme lookup

@bcmills
Copy link
Contributor

bcmills commented Oct 11, 2019

Will put it on my list for early in 1.13.

[One release later...]

Fixed!

@golang golang locked and limited conversation to collaborators Oct 10, 2020
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants