review: handle hosting Gerrit in a sub-path
The git-review module is useful for other gerrit sites. Some
host gerrit under a sub-path. Add tests for both googlesource.com
repos and non-googlesource.com repos.
Change-Id: If2848128d6957db61ac50cbab7e6927da431002b
Reviewed-on: https://go-review.googlesource.com/20553
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/git-codereview/api.go b/git-codereview/api.go
index 6fbdd48..786ac60 100644
--- a/git-codereview/api.go
+++ b/git-codereview/api.go
@@ -11,6 +11,7 @@
"io"
"io/ioutil"
"net/http"
+ "net/url"
"os"
"sort"
"strings"
@@ -43,38 +44,80 @@
// Gerrit must be set, either explicitly via the code review config or
// implicitly as Git's origin remote.
origin := config()["gerrit"]
- if origin == "" {
- origin = trim(cmdOutput("git", "config", "remote.origin.url"))
- }
+ originUrl := trim(cmdOutput("git", "config", "remote.origin.url"))
+ err := loadGerritOriginInternal(origin, originUrl)
+ if err != nil {
+ dief("failed to load Gerrit origin: %v", err)
+ }
+}
+
+// loadGerritOriginInternal does the work of loadGerritOrigin, just extracted out
+// for easier testing.
+func loadGerritOriginInternal(origin, remoteOrigin string) error {
+ originUrl, err := url.Parse(remoteOrigin)
+ if err != nil {
+ return fmt.Errorf("failed to parse git's remote.origin.url %q as a URL: %v", remoteOrigin, err)
+ } else {
+ originUrl.User = nil
+ remoteOrigin = originUrl.String()
+ }
+ hasGerritConfig := true
+ if origin == "" {
+ hasGerritConfig = false
+ origin = remoteOrigin
+ }
if strings.Contains(origin, "github.com") {
- dief("git origin must be a Gerrit host, not GitHub: %s", origin)
+ return fmt.Errorf("git origin must be a Gerrit host, not GitHub: %s", origin)
}
if !strings.HasPrefix(origin, "https://") {
- dief("git origin must be an https:// URL: %s", origin)
+ return fmt.Errorf("git origin must be an https:// URL: %s", origin)
}
- // https:// prefix and then one slash between host and top-level name
- if strings.Count(origin, "/") != 3 {
- dief("git origin is malformed: %s", origin)
- }
- host := origin[len("https://"):strings.LastIndex(origin, "/")]
- // In the case of Google's Gerrit, host is go.googlesource.com
- // and apiURL uses go-review.googlesource.com, but the Gerrit
- // setup instructions do not write down a cookie explicitly for
- // go-review.googlesource.com, so we look for the non-review
- // host name instead.
- url := origin
- if i := strings.Index(url, ".googlesource.com"); i >= 0 {
+ if googlesourceIndex := strings.Index(origin, ".googlesource.com"); googlesourceIndex >= 0 {
+ // https:// prefix and then one slash between host and top-level name
+ if strings.Count(origin, "/") != 3 {
+ return fmt.Errorf("git origin is malformed: %s", origin)
+ }
+ host := origin[len("https://"):strings.LastIndex(origin, "/")]
+
+ // In the case of Google's Gerrit, host is go.googlesource.com
+ // and apiURL uses go-review.googlesource.com, but the Gerrit
+ // setup instructions do not write down a cookie explicitly for
+ // go-review.googlesource.com, so we look for the non-review
+ // host name instead.
+ url := origin
+ i := googlesourceIndex
url = url[:i] + "-review" + url[i:]
- }
- i := strings.LastIndex(url, "/")
- url, project := url[:i], url[i+1:]
- auth.host = host
- auth.url = url
- auth.project = project
+ i = strings.LastIndex(url, "/")
+ url, project := url[:i], url[i+1:]
+
+ auth.host = host
+ auth.url = url
+ auth.project = project
+ return nil
+ }
+
+ // Origin is not *.googlesource.com.
+ //
+ // If the Gerrit origin is set from the codereview.cfg file than we handle it
+ // differently to allow for sub-path hosted Gerrit.
+ auth.host = originUrl.Host
+ if hasGerritConfig {
+ if !strings.HasPrefix(remoteOrigin, origin) {
+ return fmt.Errorf("Gerrit origin %q from %q different then git origin url %q", origin, configRef, originUrl)
+ }
+
+ auth.project = strings.Trim(strings.TrimPrefix(remoteOrigin, origin), "/")
+ auth.url = origin
+ } else {
+ auth.project = strings.Trim(originUrl.Path, "/")
+ auth.url = strings.TrimSuffix(remoteOrigin, originUrl.Path)
+ }
+
+ return nil
}
// loadAuth loads the authentication tokens for making API calls to
diff --git a/git-codereview/api_test.go b/git-codereview/api_test.go
index 410be5a..279a76f 100644
--- a/git-codereview/api_test.go
+++ b/git-codereview/api_test.go
@@ -114,3 +114,71 @@
}
}
}
+
+func TestLoadGerritOrigin(t *testing.T) {
+ list := []struct {
+ origin, originUrl string
+
+ fail bool
+ host, url, project string
+ }{
+ {
+ // *.googlesource.com
+ origin: "",
+ originUrl: "https://go.googlesource.com/crypto",
+ host: "go.googlesource.com",
+ url: "https://go-review.googlesource.com",
+ project: "crypto",
+ },
+ {
+ // Gerrit origin is set.
+ // Gerrit is hosted on a sub-domain.
+ origin: "https://gerrit.mysite.com",
+ originUrl: "https://gerrit.mysite.com/projectA",
+ host: "gerrit.mysite.com",
+ url: "https://gerrit.mysite.com",
+ project: "projectA",
+ },
+ {
+ // Gerrit origin is set.
+ // Gerrit is hosted under sub-path under "/gerrit".
+ origin: "https://mysite.com/gerrit",
+ originUrl: "https://mysite.com/gerrit/projectA",
+ host: "mysite.com",
+ url: "https://mysite.com/gerrit",
+ project: "projectA",
+ },
+ {
+ // Gerrit origin is set.
+ // Gerrit is hosted under sub-path under "/gerrit" and repo is under
+ // sub-folder.
+ origin: "https://mysite.com/gerrit",
+ originUrl: "https://mysite.com/gerrit/sub/projectA",
+ host: "mysite.com",
+ url: "https://mysite.com/gerrit",
+ project: "sub/projectA",
+ },
+ }
+
+ for _, item := range list {
+ auth.host = ""
+ auth.url = ""
+ auth.project = ""
+ err := loadGerritOriginInternal(item.origin, item.originUrl)
+ if err != nil && !item.fail {
+ t.Errorf("unexpected error from item %q %q: %v", item.origin, item.originUrl, err)
+ continue
+ }
+ if auth.host != item.host || auth.url != item.url || auth.project != item.project {
+ t.Errorf("want %q %q %q, got %q %q %q", item.host, item.url, item.project, auth.host, auth.url, auth.project)
+ continue
+ }
+ if item.fail {
+ continue
+ }
+ have := haveGerritInternal(item.origin, item.originUrl)
+ if !have {
+ t.Errorf("for %q %q expect haveGerrit() == true, but got false", item.origin, item.originUrl)
+ }
+ }
+}
diff --git a/git-codereview/config.go b/git-codereview/config.go
index 739c4f5..9843fcf 100644
--- a/git-codereview/config.go
+++ b/git-codereview/config.go
@@ -36,15 +36,25 @@
return cachedConfig
}
+// haveGerrit returns true if gerrit should be used.
+// To enable gerrit, codereview.cfg must be present with "gerrit" property set to
+// the gerrit https URL or the git origin must be to
+// "https://<project>.googlesource.com/<repo>".
func haveGerrit() bool {
gerrit := config()["gerrit"]
+ origin := trim(cmdOutput("git", "config", "remote.origin.url"))
+ return haveGerritInternal(gerrit, origin)
+}
+
+// haveGerritInternal is the same as haveGerrit but factored out
+// for testing.
+func haveGerritInternal(gerrit, origin string) bool {
if gerrit == "off" {
return false
}
if gerrit != "" {
return true
}
- origin := trim(cmdOutput("git", "config", "remote.origin.url"))
if strings.Contains(origin, "github.com") {
return false
}
