Skip to content

Commit

Permalink
cmd/mr_checkout: fix fetch reference existence check
Browse files Browse the repository at this point in the history
When the tracking mode was enabled (`-t`) the reference being passed to
`git show-ref` was being wrongly composed, since the code was always
expecting a simple branch name instead of a full remote ref. This patch
fixes it by separating the check in two steps: 1) check if the local
branch to which the MR will be checked-out exists and 2) if tracking is
enabled, check if the remote ref already exists. Both are influenced by
`--force`.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
  • Loading branch information
bmeneg committed Feb 7, 2022
1 parent caa16da commit 1ce393c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 14 deletions.
40 changes: 27 additions & 13 deletions cmd/mr_checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cmd

import (
"fmt"
"os"

"github.com/MakeNowJust/heredoc/v2"
"github.com/rsteube/carapace"
Expand Down Expand Up @@ -63,6 +62,18 @@ var checkoutCmd = &cobra.Command{
if mrCheckoutCfg.branch == "" {
mrCheckoutCfg.branch = mr.SourceBranch
}

err = git.New("show-ref", "--verify", "--quiet", "refs/heads/"+mrCheckoutCfg.branch).Run()
if err == nil {
if mrCheckoutCfg.force {
if err := git.New("branch", "-D", mrCheckoutCfg.branch).Run(); err != nil {
log.Fatal(err)
}
} else {
log.Fatalf("branch %s already exists", mrCheckoutCfg.branch)
}
}

// By default, fetch to configured branch
fetchToRef := mrCheckoutCfg.branch

Expand Down Expand Up @@ -101,26 +112,29 @@ var checkoutCmd = &cobra.Command{
}
}

fetchToRef = fmt.Sprintf("refs/remotes/%s/%s", remoteName, mr.SourceBranch)
}

if err := git.New("show-ref", "--verify", "--quiet", "refs/heads/"+fetchToRef).Run(); err == nil {
if mrCheckoutCfg.force {
if err := git.New("branch", "-D", mrCheckoutCfg.branch).Run(); err != nil {
log.Fatal(err)
trackRef := fmt.Sprintf("refs/remotes/%s/%s", remoteName, mr.SourceBranch)
err = git.New("show-ref", "--verify", "--quiet", trackRef).Run()
if err == nil {
if mrCheckoutCfg.force {
err = git.New("update-ref", "-d", trackRef).Run()
if err != nil {
log.Fatal(err)
}
} else {
log.Fatalf("remote reference %s already exists", trackRef)
}
} else {
fmt.Println("ERROR: mr", mrID, "branch", fetchToRef, "already exists.")
os.Exit(1)
}

fetchToRef = trackRef
}

// https://docs.gitlab.com/ce/user/project/merge_requests/#checkout-merge-requests-locally
// https://docs.gitlab.com/ee/user/project/merge_requests/reviews/#checkout-merge-requests-locally-through-the-head-ref
mrRef := fmt.Sprintf("refs/merge-requests/%d/head", mrID)
fetchRefSpec := fmt.Sprintf("%s:%s", mrRef, fetchToRef)
if err := git.New("fetch", targetRemote, fetchRefSpec).Run(); err != nil {
log.Fatal(err)
}

if mrCheckoutCfg.track {
// Create configured branch with tracking from fetchToRef
// git branch --flags <branchname> [<start-point>]
Expand All @@ -141,7 +155,7 @@ func init() {
checkoutCmd.Flags().BoolVarP(&mrCheckoutCfg.track, "track", "t", false, "set branch to track remote branch, adds remote if needed")
// useHTTP is defined in "project_create.go"
checkoutCmd.Flags().BoolVar(&useHTTP, "http", false, "checkout using HTTP protocol instead of SSH")
checkoutCmd.Flags().BoolVarP(&mrCheckoutCfg.force, "force", "f", false, "force branch checkout and override existing branch")
checkoutCmd.Flags().BoolVarP(&mrCheckoutCfg.force, "force", "f", false, "force branch and remote reference override")
mrCmd.AddCommand(checkoutCmd)
carapace.Gen(checkoutCmd).PositionalCompletion(
carapace.ActionCallback(func(c carapace.Context) carapace.Action {
Expand Down
2 changes: 1 addition & 1 deletion cmd/mr_checkout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func Test_mrDoubleCheckoutFailCmdRun(t *testing.T) {
t.Log(eLog)
t.Fatal(err)
}
require.Contains(t, eLog, "ERROR: mr 1 branch mrtest already exists")
require.Contains(t, eLog, "branch mrtest already exists")
}

func Test_mrDoubleCheckoutForceRun(t *testing.T) {
Expand Down

0 comments on commit 1ce393c

Please sign in to comment.