Skip to content

Commit

Permalink
remote: support remotes referred by URL only
Browse files Browse the repository at this point in the history
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 <URL>

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.<branch>.remote`, then it gets the remote URL itself; however,
in this situation, the `branch.<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 <bmeneg@redhat.com>
  • Loading branch information
bmeneg committed Feb 12, 2022
1 parent f68cb49 commit bbd51a7
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 32 deletions.
5 changes: 4 additions & 1 deletion cmd/ci_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion cmd/mr_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand Down
33 changes: 21 additions & 12 deletions cmd/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.<name>.pushRemote > remote.pushDefault > branch.<name>.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
Expand Down
5 changes: 4 additions & 1 deletion cmd/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
32 changes: 17 additions & 15 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions internal/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
}

Expand Down

0 comments on commit bbd51a7

Please sign in to comment.