From bbd51a736ff574c61e374b4c180e072576e98583 Mon Sep 17 00:00:00 2001 From: Bruno Meneguele Date: Fri, 11 Feb 2022 16:23:38 -0300 Subject: [PATCH] remote: support remotes referred by URL only When using git-push the user can set the upstream of a branch to a remote that is not present in the current repository by referring to its URL. In these cases, git checks for the validity of the URL itself instead of checking if a remote with that name exists locally. I.e.: $ git push --set-upstream This case affects lab mostly when we use the remote URL to lookup for it in GitLab: lab checks for the branch's remote name. `branch..remote`, then it gets the remote URL itself; however, in this situation, the `branch..remote` will be the URL already and any check for that remote in the local repository fails. The biggest problem is the amount of different URL formats supported by Git [1], which is bigger than the ones supported by the usual url.Parse() (net/url Go module). Because of that, I brought the git-urls module to help. This new module threat anything that isn't an external URL as a possible file URL, meaning that default remote names are also accepted, with that we can pass all our remote names (URL or not) to its Parse() function and just use the "path" component, which is pretty close to what we need for the GitLab lookup. [1] https://git-scm.com/docs/git-push#URLS Signed-off-by: Bruno Meneguele --- cmd/ci_run.go | 5 ++++- cmd/mr_create.go | 6 +++++- cmd/util.go | 33 +++++++++++++++++++++------------ cmd/util_test.go | 5 ++++- go.mod | 1 + go.sum | 2 ++ internal/git/git.go | 32 +++++++++++++++++--------------- internal/git/git_test.go | 4 ++-- 8 files changed, 56 insertions(+), 32 deletions(-) diff --git a/cmd/ci_run.go b/cmd/ci_run.go index 12bed67f0..11c5a98be 100644 --- a/cmd/ci_run.go +++ b/cmd/ci_run.go @@ -100,7 +100,10 @@ func getCIRunOptions(cmd *cobra.Command, args []string) (string, string, error) var pid string - remote := determineSourceRemote(branch) + remote, err := determineSourceRemote(branch) + if err != nil { + return "", "", err + } rn, err := git.PathWithNamespace(remote) if err != nil { return "", "", err diff --git a/cmd/mr_create.go b/cmd/mr_create.go index 734334a72..235e121d2 100644 --- a/cmd/mr_create.go +++ b/cmd/mr_create.go @@ -139,7 +139,11 @@ func runMRCreate(cmd *cobra.Command, args []string) { log.Fatal(err) } - sourceRemote := determineSourceRemote(localBranch) + sourceRemote, err := determineSourceRemote(localBranch) + if err != nil { + log.Fatal(err) + } + // Get the pushed branch name sourceBranch, _ := git.UpstreamBranch(localBranch) if sourceBranch == "" { diff --git a/cmd/util.go b/cmd/util.go index de47e6a5f..fa25b37c1 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -14,6 +14,7 @@ import ( flag "github.com/spf13/pflag" "github.com/spf13/viper" gitconfig "github.com/tcnksm/go-gitconfig" + giturls "github.com/whilp/git-urls" gitlab "github.com/xanzy/go-gitlab" "github.com/zaquestion/lab/internal/config" "github.com/zaquestion/lab/internal/git" @@ -187,7 +188,10 @@ func parseArgsRemoteAndBranch(args []string) (string, string, error) { } if remote == "" { - remote = determineSourceRemote(branch) + remote, err = determineSourceRemote(branch) + if err != nil { + return "", "", err + } } remote, err = getRemoteName(remote) if err != nil { @@ -474,24 +478,29 @@ func labURLToRepo(project *gitlab.Project) string { return urlToRepo } -func determineSourceRemote(branch string) string { +func determineSourceRemote(branch string) (string, error) { // There is a precendence of options that should be considered here: // branch..pushRemote > remote.pushDefault > branch..remote // This rule is placed in git-config(1) manpage r, err := gitconfig.Local("branch." + branch + ".pushRemote") - if err == nil { - return r - } - r, err = gitconfig.Local("remote.pushDefault") - if err == nil { - return r + if err != nil { + r, err = gitconfig.Local("remote.pushDefault") + if err != nil { + r, err = gitconfig.Local("branch." + branch + ".remote") + if err != nil { + return forkRemote, nil + } + } } - r, err = gitconfig.Local("branch." + branch + ".remote") - if err == nil { - return r + + // Parse the remote name for possible URL. + u, err := giturls.Parse(r) + if err != nil { + return "", err } - return forkRemote + path := strings.TrimPrefix(u.Path, "/") + return path, nil } // Check of a case-insensitive prefix in a string diff --git a/cmd/util_test.go b/cmd/util_test.go index 33e1d8557..130db9cba 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -345,7 +345,10 @@ func Test_determineSourceRemote(t *testing.T) { } t.Run(test.desc, func(t *testing.T) { - sourceRemote := determineSourceRemote(test.branch) + sourceRemote, err := determineSourceRemote(test.branch) + if err != nil { + t.Fatal(err) + } assert.Equal(t, test.expected, sourceRemote) }) } diff --git a/go.mod b/go.mod index 566a2398f..08f588d94 100644 --- a/go.mod +++ b/go.mod @@ -73,6 +73,7 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf // indirect github.com/subosito/gotenv v1.2.0 // indirect + github.com/whilp/git-urls v1.0.0 github.com/xanzy/ssh-agent v0.3.1 // indirect github.com/yuin/goldmark v1.4.1 // indirect github.com/yuin/goldmark-emoji v1.0.1 // indirect diff --git a/go.sum b/go.sum index c5253c930..1321c7be8 100644 --- a/go.sum +++ b/go.sum @@ -436,6 +436,8 @@ github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tcnksm/go-gitconfig v0.1.2 h1:iiDhRitByXAEyjgBqsKi9QU4o2TNtv9kPP3RgPgXBPw= github.com/tcnksm/go-gitconfig v0.1.2/go.mod h1:/8EhP4H7oJZdIPyT+/UIsG87kTzrzM4UsLGSItWYCpE= +github.com/whilp/git-urls v1.0.0 h1:95f6UMWN5FKW71ECsXRUd3FVYiXdrE7aX4NZKcPmIjU= +github.com/whilp/git-urls v1.0.0/go.mod h1:J16SAmobsqc3Qcy98brfl5f5+e0clUvg1krgwk/qCfE= github.com/xanzy/go-gitlab v0.51.0 h1:qCVJM9ijpY/ZG703p6DIK7RCUwZ4iQlBAig0TGJ/mRc= github.com/xanzy/go-gitlab v0.51.0/go.mod h1:Q+hQhV508bDPoBijv7YjK/Lvlb4PhVhJdKqXVQrUoAE= github.com/xanzy/ssh-agent v0.3.0/go.mod h1:3s9xbODqPuuhK9JV1R321M/FlMZSBvE5aY6eAcqrDh0= diff --git a/internal/git/git.go b/internal/git/git.go index 409bcc32a..e5f3fa809 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -13,6 +13,7 @@ import ( retry "github.com/avast/retry-go" "github.com/pkg/errors" gitconfig "github.com/tcnksm/go-gitconfig" + giturls "github.com/whilp/git-urls" "github.com/zaquestion/lab/internal/logger" ) @@ -181,26 +182,27 @@ func PathWithNamespace(remote string) (string, error) { if err != nil { return "", err } + if remoteURL == "" { + // Branches can track remote based on ther URL, thus we don't + // really have a remote entity in the git config, but only the + // URL of the remote. + // https://git-scm.com/docs/git-push#Documentation/git-push.txt-ltrepositorygt + remoteURL = remote + } } - parts := strings.Split(remoteURL, "//") - - if len(parts) == 1 { - // scp-like short syntax (e.g. git@gitlab.com...) - part := parts[0] - parts = strings.Split(part, ":") - } else if len(parts) == 2 { - // every other protocol syntax (e.g. ssh://, http://, git://) - part := parts[1] - parts = strings.SplitN(part, "/", 2) - } else { - return "", errors.Errorf("cannot parse remote: %s url: %s", remote, remoteURL) + u, err := giturls.Parse(remoteURL) + if err != nil { + return "", err } - if len(parts) != 2 { - return "", errors.Errorf("cannot parse remote: %s url: %s", remote, remoteURL) + // remote URLs can't refer to other files or local paths, ie., other remote + // names. + if u.Scheme == "file" { + return "", errors.Errorf("invalid remote URL format for %s", remote) } - path := parts[1] + + path := strings.TrimPrefix(u.Path, "/") path = strings.TrimSuffix(path, ".git") return path, nil } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index dead950b8..eceae38cb 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -187,10 +187,10 @@ func TestPathWithNamespace(t *testing.T) { expectedErr: "the key `remote.phoney.url` is not found", }, { - desc: "remote doesn't exist", + desc: "invalid remote URL", remote: "garbage", expected: "", - expectedErr: "cannot parse remote: garbage url: garbageurl", + expectedErr: "invalid remote URL format for garbage", }, }