devapp/owners: use path prefixes instead of string prefixes for matching
Remove trailing slashes uniformly from table entries.
Previously, we used string prefixes, which could over-match if two
packages' final elements share a prefix, and used trailing slashes to
compensate, but it's remarkably easy to omit a trailing slash and end
up with a pattern that usually-but-not-always works.
Updates golang/go#27586
Change-Id: Id029a706653e2ba42e83d68f789b65fca34be623
Reviewed-on: https://go-review.googlesource.com/c/build/+/171242
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
diff --git a/devapp/owners/owners.go b/devapp/owners/owners.go
index c762740..9791fb5 100644
--- a/devapp/owners/owners.go
+++ b/devapp/owners/owners.go
@@ -10,7 +10,6 @@
"html/template"
"log"
"net/http"
- "strings"
)
type Owner struct {
@@ -43,13 +42,32 @@
func match(path string) *Entry {
var deepestPath string
for p := range entries {
- if strings.HasPrefix(path, p) && len(p) > len(deepestPath) {
+ if hasPathPrefix(path, p) && len(p) > len(deepestPath) {
deepestPath = p
}
}
return entries[deepestPath]
}
+// hasPathPrefix reports whether the slash-separated path s
+// begins with the elements in prefix.
+//
+// Copied from go/src/cmd/go/internal/str.HasPathPrefix.
+func hasPathPrefix(s, prefix string) bool {
+ if len(s) == len(prefix) {
+ return s == prefix
+ }
+ if prefix == "" {
+ return true
+ }
+ if len(s) > len(prefix) {
+ if prefix[len(prefix)-1] == '/' || s[len(prefix)] == '/' {
+ return s[:len(prefix)] == prefix
+ }
+ }
+ return false
+}
+
// Handler takes one or more paths and returns a map of each to a matching
// Entry struct. If no Entry is matched for the path, the value for the key
// is nil.
diff --git a/devapp/owners/owners_test.go b/devapp/owners/owners_test.go
index 2956ac2..56599a3 100644
--- a/devapp/owners/owners_test.go
+++ b/devapp/owners/owners_test.go
@@ -20,6 +20,12 @@
entry *Entry
}{
{
+ "sync",
+ &Entry{
+ Primary: []Owner{bcmills},
+ },
+ },
+ {
"crypto/chacha20poly1305/chacha20poly1305.go",
&Entry{
Primary: []Owner{filippo},
@@ -34,6 +40,13 @@
},
},
{
+ "go/src/cmd/compile",
+ &Entry{
+ Primary: []Owner{khr, gri},
+ Secondary: []Owner{josharian, mdempsky, martisch},
+ },
+ },
+ {
"go/path/with/no/owners",
&Entry{
Primary: []Owner{rsc, iant, bradfitz},
diff --git a/devapp/owners/table.go b/devapp/owners/table.go
index 8f9229e..283d943 100644
--- a/devapp/owners/table.go
+++ b/devapp/owners/table.go
@@ -74,20 +74,20 @@
// entries is a map of <repo name>/<path> to Owner entries.
// It should not be modified at runtime.
var entries = map[string]*Entry{
- "arch/": {
+ "arch": {
Primary: []Owner{cherryyz},
},
- "crypto/": {
+ "crypto": {
Primary: []Owner{filippo},
Secondary: []Owner{agl},
},
- "crypto/ssh/": {
+ "crypto/ssh": {
Primary: []Owner{hanwen},
Secondary: []Owner{filippo},
},
- "go/": {
+ "go": {
Primary: []Owner{rsc, iant, bradfitz},
},
"go/src/archive/tar": {
@@ -105,7 +105,7 @@
Primary: []Owner{},
Secondary: []Owner{bradfitz, iant},
},
- "go/src/cmd/compile/": {
+ "go/src/cmd/compile": {
Primary: []Owner{khr, gri},
Secondary: []Owner{josharian, mdempsky, martisch},
},
@@ -547,22 +547,22 @@
Secondary: []Owner{andybons},
},
- "gofrontend/": {
+ "gofrontend": {
Primary: []Owner{iant},
Secondary: []Owner{thanm},
},
- "gollvm/": {
+ "gollvm": {
Primary: []Owner{thanm},
Secondary: []Owner{cherryyz},
},
- "mobile/": {
+ "mobile": {
Primary: []Owner{eliasnaur},
Secondary: []Owner{hyangah},
},
- "net/": {
+ "net": {
Primary: []Owner{mikioh},
Secondary: []Owner{bradfitz, iant},
},
@@ -585,11 +585,11 @@
Primary: []Owner{mikioh, iant},
},
- "review/": {
+ "review": {
Secondary: []Owner{josharian, kevinburke},
},
- "sync/": {
+ "sync": {
Primary: []Owner{bcmills},
},
@@ -600,7 +600,7 @@
Primary: []Owner{alexbrainman, bradfitz},
},
- "text/": {
+ "text": {
Primary: []Owner{mpvl},
},
