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

Jump to bottom

x/mod: expose API to do GOPRIVATE matching #38725

Closed
mvdan opened this issue Apr 28, 2020 · 9 comments
Closed

x/mod: expose API to do GOPRIVATE matching #38725

mvdan opened this issue Apr 28, 2020 · 9 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@mvdan
Copy link
Member

mvdan commented Apr 28, 2020 •
edited

That is:

go/src/cmd/go/internal/str/path.go

Lines 54 to 96 in 863424b

// GlobsMatchPath reports whether any path prefix of target
// matches one of the glob patterns (as defined by path.Match)
// in the comma-separated globs list.
// It ignores any empty or malformed patterns in the list.
func GlobsMatchPath(globs, target string) bool {
for globs != "" {
// Extract next non-empty glob in comma-separated list.
var glob string
if i := strings.Index(globs, ","); i >= 0 {
glob, globs = globs[:i], globs[i+1:]
} else {
glob, globs = globs, ""
}
if glob == "" {
continue
}
// A glob with N+1 path elements (N slashes) needs to be matched
// against the first N+1 path elements of target,
// which end just before the N+1'th slash.
n := strings.Count(glob, "/")
prefix := target
// Walk target, counting slashes, truncating at the N+1'th slash.
for i := 0; i < len(target); i++ {
if target[i] == '/' {
if n == 0 {
prefix = target[:i]
break
}
n--
}
}
if n > 0 {
// Not enough prefix elements.
continue
}
matched, _ := path.Match(glob, prefix)
if matched {
return true
}
}
return false
}

go help module-private does briefly mention the semantics:

The variable is a comma-separated list of glob patterns (in the syntax of Go's path.Match) of module path prefixes.

However, I don't think that's quite detailed enough to implement this from scratch, and it's still pretty tricky and prone to errors to do.

I realise that package pattern matching wasn't exposed as a library on purpose, to encourage tool authors to use go list instead. However, I don't think this falls under the same bucket since there's no way to do this via the go tool, and because GOPRIVATE matching should be a very cheap operation - it doesn't involve any disk or network I/O for example, unlike patterns like cmd/....
@mvdan mvdan added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. modules Tools This label describes issues relating to any tools in the x/tools repository. labels Apr 28, 2020
@gopherbot gopherbot added this to the Unreleased milestone Apr 28, 2020
@bcmills
Copy link
Contributor

bcmills commented Apr 28, 2020

CC @jayconrod @matloob

@jayconrod
Copy link
Contributor

Seems like a good thing to have. Maybe in module we could add:

func MatchPatterns(path, patterns string) bool

where path is a module path and patterns is a string in the format expected by GOPRIVATE and friends?

@bcmills
Copy link
Contributor

bcmills commented Apr 28, 2020

I might be inclined to use a different name, since these patterns are looser than our usual “module path patterns”: they implicitly apply to prefixes, and they support the full glob syntax instead of just ....

Maybe module.MatchPrefixPatterns?

@mvdan
Copy link
Member Author

mvdan commented Apr 30, 2020

My only thought on the matter is that the "patterns" parameter should go first, to match what people are used to from the standard library - look at path.Match, for example.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label May 13, 2020
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 13, 2020
@stamblerre
Copy link
Contributor

Just came across this issue and wanted to add my support for this. @findleyr already added IsGoPrivatePath functionality to gopls as part of #36998, so it would be really nice to move that logic into x/mod. What's the next step for this issue? Can we mail a CL?

@jayconrod
Copy link
Contributor

Sure, CLs welcome. It should go into golang.org/x/mod/module with the signature:

func MatchPrefixPatterns(patterns, path string) bool

That can land immediately, and we can tag a prerelease version if that's useful. We should update cmd/go to use the logic in x/mod after 1.16 development is open.

@findleyr
Copy link
Contributor

In retrospect I probably should have made an effort to put this in x/mod before copying...

I can send a CL.

@gopherbot
Copy link

Change https://golang.org/cl/239797 mentions this issue: module: add a MatchPrefixPatterns function, for matching GOPRIVATE, etc.

@gopherbot
Copy link

Change https://golang.org/cl/240061 mentions this issue: cmd/go: migrate to module.MatchPrefixPatterns

gopherbot pushed a commit that referenced this issue Aug 14, 2020
cmd/go: migrate to module.MatchPrefixPatterns
8766f96 
In CL 239797, str.GlobsMatchPath was copied to golang.org/x/mod/module
as MatchPrefixPatterns. This CL updates x/mod, switches calls to use
the new function, and deletes the old function.

For #38725

Change-Id: I7241032228b574aa539426a92d2f5aad9ee001e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/240061
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@golang golang locked and limited conversation to collaborators Jun 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@jayconrod @mvdan @bcmills @stamblerre @gopherbot @findleyr