diff --git a/commands/mr/checkout/mr_checkout.go b/commands/mr/checkout/mr_checkout.go index 346167fabefa6ca3ab912deb4531a79f31513225..d5fa8f6522deb86deb2d67a6e955b22312bdb704 100644 --- a/commands/mr/checkout/mr_checkout.go +++ b/commands/mr/checkout/mr_checkout.go @@ -37,23 +37,9 @@ func NewCmdCheckout(f *cmdutils.Factory) *cobra.Command { `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - var err error - var upstream string - - if mrCheckoutCfg.upstream != "" { - upstream = mrCheckoutCfg.upstream - - if val := strings.Split(mrCheckoutCfg.upstream, "/"); len(val) > 1 { - // Verify that we have the remote set - repo, err := f.Remotes() - if err != nil { - return err - } - _, err = repo.FindByName(val[0]) - if err != nil { - return err - } - } + // Validate upstream if provided + if err := validateUpstream(f, mrCheckoutCfg.upstream); err != nil { + return err } apiClient, err := f.HttpClient() @@ -70,58 +56,32 @@ func NewCmdCheckout(f *cmdutils.Factory) *cobra.Command { mrCheckoutCfg.branch = mr.SourceBranch } - var mrRef string - var mrProject *gitlab.Project - - mrProject, err = api.GetProject(apiClient, mr.SourceProjectID) + mrRef, remoteURL, err := getMRRef(apiClient, mr) if err != nil { - // If we don't have access to the source project, let's try the target project - mrProject, err = api.GetProject(apiClient, mr.TargetProjectID) - if err != nil { - return err - } else { - // We found the target project, let's find the ref another way - mrRef = fmt.Sprintf("refs/merge-requests/%d/head", mr.IID) - } - - } else { - mrRef = fmt.Sprintf("refs/heads/%s", mr.SourceBranch) - } - - fetchRefSpec := fmt.Sprintf("%s:%s", mrRef, mrCheckoutCfg.branch) - if err := git.RunCmd([]string{"fetch", mrProject.SSHURLToRepo, fetchRefSpec}); err != nil { - // the remote may have diverged from local after git operations - // try fetching without updating the branch ref before giving up - if err := git.RunCmd([]string{"fetch", mrProject.SSHURLToRepo, mrRef}); err != nil { - return err - } + return err } - // .remote is needed for `git pull` to work - // .pushRemote is needed for `git push` to work, if user has set `remote.pushDefault`. - // see https://git-scm.com/docs/git-config#Documentation/git-config.txt-branchltnamegtremote - if err := git.RunCmd([]string{"config", fmt.Sprintf("branch.%s.remote", mrCheckoutCfg.branch), mrProject.SSHURLToRepo}); err != nil { + // Handle branch comparison and update (returns current branch info) + currentBranch, err := handleBranchUpdate(cmd, mrRef, remoteURL, args) + if err != nil { return err } - if mr.AllowCollaboration { - if err := git.RunCmd([]string{"config", fmt.Sprintf("branch.%s.pushRemote", mrCheckoutCfg.branch), mrProject.SSHURLToRepo}); err != nil { - return err - } - } - if err := git.RunCmd([]string{"config", fmt.Sprintf("branch.%s.merge", mrCheckoutCfg.branch), mrRef}); err != nil { + + // Setup MR branch configuration + if err := setupMRBranchConfig(mrCheckoutCfg.branch, remoteURL, mrRef, mr.AllowCollaboration); err != nil { return err } - // Check out branch var gr git.StandardGitCommand - - if err := git.CheckoutBranch(mrCheckoutCfg.branch, gr); err != nil { - return err + if currentBranch != mrCheckoutCfg.branch { + err := git.CheckoutBranch(mrCheckoutCfg.branch, gr) + if err != nil { + return err + } } - // Check out the branch - if upstream != "" { - if err := git.RunCmd([]string{"branch", "--set-upstream-to", upstream}); err != nil { + if mrCheckoutCfg.upstream != "" { + if err := git.RunCmd([]string{"branch", "--set-upstream-to", mrCheckoutCfg.upstream}); err != nil { return err } } @@ -134,3 +94,183 @@ func NewCmdCheckout(f *cmdutils.Factory) *cobra.Command { mrCheckoutCmd.Flags().StringVarP(&mrCheckoutCfg.upstream, "set-upstream-to", "u", "", "Set tracking of checked-out branch to [REMOTE/]BRANCH.") return mrCheckoutCmd } + +// validateUpstream validates the upstream configuration +func validateUpstream(f *cmdutils.Factory, upstream string) error { + if upstream == "" { + return nil + } + + if val := strings.Split(upstream, "/"); len(val) > 1 { + // Verify that we have the remote set + repo, err := f.Remotes() + if err != nil { + return err + } + _, err = repo.FindByName(val[0]) + if err != nil { + return err + } + } + return nil +} + +// getMRRef gets ref and remoteURL for the merge request +func getMRRef(apiClient *gitlab.Client, mr *gitlab.MergeRequest) (string, string, error) { + var mrRef string + var mrProject *gitlab.Project + + mrProject, err := api.GetProject(apiClient, mr.SourceProjectID) + if err != nil { + // If we don't have access to the source project, try the target project + mrProject, err = api.GetProject(apiClient, mr.TargetProjectID) + if err != nil { + return "", "", err + } + // Use merge request ref for target project + mrRef = fmt.Sprintf("refs/merge-requests/%d/head", mr.IID) + } else { + mrRef = fmt.Sprintf("refs/heads/%s", mr.SourceBranch) + } + + // Get the preferred remote URL format based on current branch config + remoteURL, err := getPreferredRemoteURL(mrProject) + if err != nil { + return "", "", fmt.Errorf("failed to determine remote URL: %w", err) + } + + return mrRef, remoteURL, nil +} + +func getPreferredRemoteURL(mrProject *gitlab.Project) (string, error) { + currentBranch, err := git.CurrentBranch() + if err != nil { + // Can't determine current branch, default to SSH + return mrProject.SSHURLToRepo, nil + } + + branchConfig := git.ReadBranchConfig(currentBranch) + + // Check if branch has direct remote URL configured (rare case) + if branchConfig.RemoteURL != nil { + return matchingProtocolURL(branchConfig.RemoteURL.String(), mrProject), nil + } + + // Check if branch uses named remote (common case) + if branchConfig.RemoteName != "" { + remoteURL, err := git.GetRemoteURL(branchConfig.RemoteName) + if err == nil { + return matchingProtocolURL(remoteURL, mrProject), nil + } + } + + // No branch config found, default to SSH + return mrProject.SSHURLToRepo, nil +} + +// matchingProtocolURL returns the GitLab project URL that matches the existing URL's protocol +func matchingProtocolURL(existingURL string, mrProject *gitlab.Project) string { + if strings.HasPrefix(existingURL, "git@") || strings.HasPrefix(existingURL, "ssh://") { + return mrProject.SSHURLToRepo + } + return mrProject.HTTPURLToRepo +} + +func handleBranchUpdate(cmd *cobra.Command, mrRef, remoteURL string, args []string) (string, error) { + branchExists := git.HasLocalBranch(mrCheckoutCfg.branch) + currentBranch, _ := git.CurrentBranch() + isOnTargetBranch := currentBranch == mrCheckoutCfg.branch + + if branchExists { + tempRef := fmt.Sprintf("refs/remotes/mr-checkout-temp/%s", mrCheckoutCfg.branch) + + if err := git.FetchRefToTempRef(remoteURL, mrRef, tempRef); err != nil { + return currentBranch, fmt.Errorf("failed to fetch merge request updates: %w", err) + } + + comparison, err := git.CompareBranches(mrCheckoutCfg.branch, tempRef) + + defer func() { + _ = git.DeleteRef(tempRef) // Ignore cleanup errors + }() + + if comparison.IsUpToDate { + if isOnTargetBranch { + fmt.Fprintf(cmd.OutOrStdout(), "✓ Already on branch '%s' which is up to date with the merge request.\n", mrCheckoutCfg.branch) + return currentBranch, nil + } + fmt.Fprintf(cmd.OutOrStdout(), "✓ Branch '%s' is up to date with the merge request.\n", mrCheckoutCfg.branch) + } else if comparison.HasDiverged { + return currentBranch, handleDivergedBranch(cmd, comparison, mrCheckoutCfg.branch, args) + } else if comparison.IsAhead { + return currentBranch, handleAheadBranch(cmd, comparison, mrCheckoutCfg.branch) + } else if comparison.IsBehind { + if err := handleBehindBranch(cmd, mrCheckoutCfg.branch, comparison.BehindCount); err != nil { + return currentBranch, err + } + } + + err = git.UpdateBranchToRef(mrCheckoutCfg.branch, remoteURL, mrRef, currentBranch, branchExists) + return currentBranch, err + } + + err := git.UpdateBranchToRef(mrCheckoutCfg.branch, remoteURL, mrRef, currentBranch, branchExists) + return currentBranch, err +} + +// setupMRBranchConfig sets up git config for a merge request branch +func setupMRBranchConfig(branchName, remoteURL, mergeRef string, allowCollaboration bool) error { + // Set remote for git pull + if err := git.RunCmd([]string{"config", fmt.Sprintf("branch.%s.remote", branchName), remoteURL}); err != nil { + return fmt.Errorf("failed to set branch remote: %w", err) + } + + // Set push remote if collaboration is allowed + if allowCollaboration { + if err := git.RunCmd([]string{"config", fmt.Sprintf("branch.%s.pushRemote", branchName), remoteURL}); err != nil { + return fmt.Errorf("failed to set branch push remote: %w", err) + } + } + + // Set merge ref + if err := git.RunCmd([]string{"config", fmt.Sprintf("branch.%s.merge", branchName), mergeRef}); err != nil { + return fmt.Errorf("failed to set branch merge ref: %w", err) + } + + return nil +} + +// handleDivergedBranch handles when a branch has diverged from the MR +func handleDivergedBranch(cmd *cobra.Command, comparison *git.BranchComparison, branchName string, args []string) error { + fmt.Fprintf(cmd.OutOrStdout(), "\n⚠️ Your local branch '%s' has diverged from the merge request:\n", branchName) + fmt.Fprintf(cmd.OutOrStdout(), " - %s commit(s) ahead (local commits not in MR)\n", comparison.AheadCount) + fmt.Fprintf(cmd.OutOrStdout(), " - %s commit(s) behind (MR commits not in local)\n", comparison.BehindCount) + fmt.Fprintf(cmd.OutOrStdout(), "\n❌ Cannot update branch: You have local commits that would be lost.\n") + return fmt.Errorf("branch has diverged from merge request") +} + +// handleAheadBranch handles when a local branch is ahead of the MR +func handleAheadBranch(cmd *cobra.Command, comparison *git.BranchComparison, branchName string) error { + fmt.Fprintf(cmd.OutOrStdout(), "\n⚠️ Your local branch '%s' is %s commit(s) ahead of the merge request.\n", branchName, comparison.AheadCount) + fmt.Fprintf(cmd.OutOrStdout(), "\n❌ Cannot update branch: You have local commits that would be lost.\n") + return fmt.Errorf("local commits would be lost") +} + +// handleBehindBranch handles when a local branch is behind the MR +func handleBehindBranch(cmd *cobra.Command, branchName, behindCount string) error { + fmt.Fprintf(cmd.OutOrStdout(), "\n⚠️ Your local branch '%s' is %s commit(s) behind the merge request.\n", branchName, behindCount) + + // Check for uncommitted changes + hasUncommitted, changeCount, err := git.HasUncommittedChanges() + if err != nil { + return fmt.Errorf("failed to check for uncommitted changes: %w", err) + } + + if hasUncommitted { + fmt.Fprintf(cmd.OutOrStdout(), "\n❌ Cannot update branch: You have %d uncommitted change(s) that would be lost.\n", changeCount) + return fmt.Errorf("uncommitted changes would be lost") + } + + fmt.Fprintf(cmd.OutOrStdout(), "\nUpdating branch to match the merge request...\n") + return nil +} diff --git a/commands/mr/checkout/mr_checkout_test.go b/commands/mr/checkout/mr_checkout_test.go index 56c2cefd556b91f9b09bbfe41e64384093c18fe8..0bdea4c0c51f156245cdca54bba164a91d81a95c 100644 --- a/commands/mr/checkout/mr_checkout_test.go +++ b/commands/mr/checkout/mr_checkout_test.go @@ -1,24 +1,26 @@ package checkout import ( + "bytes" "net/http" "net/url" "strings" "testing" - "gitlab.com/gitlab-org/cli/commands/cmdtest" - - "github.com/MakeNowJust/heredoc/v2" - "gitlab.com/gitlab-org/cli/pkg/git" - "github.com/stretchr/testify/assert" + gitlab "gitlab.com/gitlab-org/api/client-go" + "gitlab.com/gitlab-org/cli/commands/cmdtest" "gitlab.com/gitlab-org/cli/internal/glrepo" + "gitlab.com/gitlab-org/cli/pkg/git" "gitlab.com/gitlab-org/cli/pkg/httpmock" "gitlab.com/gitlab-org/cli/test" ) func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) { - ios, _, stdout, stderr := cmdtest.InitIOStreams(isTTY, "") + outBuf := &bytes.Buffer{} + errBuf := &bytes.Buffer{} + ios, _, _, _ := cmdtest.InitIOStreams(isTTY, "") + pu, _ := url.Parse("https://gitlab.com/OWNER/REPO.git") factory := cmdtest.InitFactory(ios, rt) @@ -52,7 +54,21 @@ func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*t cmd := NewCmdCheckout(factory) - return cmdtest.ExecuteCommand(cmd, cli, stdout, stderr) + // Set on root cmd, thus we also need to set it here. + cmd.SilenceUsage = true + + cmd.SetOut(outBuf) + cmd.SetErr(errBuf) + + args := strings.Fields(cli) + cmd.SetArgs(args) + + err := cmd.Execute() + + return &test.CmdOut{ + OutBuf: outBuf, + ErrBuf: errBuf, + }, err } func TestMrCheckout(t *testing.T) { @@ -64,14 +80,14 @@ func TestMrCheckout(t *testing.T) { } tests := []struct { - name string - commandArgs string - branch string - httpMocks []httpMock - - shelloutStubs []string - + name string + commandArgs string + branch string + httpMocks []httpMock + shelloutStubs []string expectedShellouts []string + expectedError bool + expectedOutput string }{ { name: "when a valid MR is checked out using MR id", @@ -79,48 +95,127 @@ func TestMrCheckout(t *testing.T) { branch: "main", httpMocks: []httpMock{ { - http.MethodGet, - "/api/v4/projects/OWNER/REPO/merge_requests/123", - http.StatusOK, - `{ - "id": 123, - "iid": 123, - "project_id": 3, - "source_project_id": 3, - "title": "test mr title", - "description": "test mr description", - "allow_collaboration": false, - "state": "opened", - "source_branch":"feat-new-mr" - }`, + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, }, { - http.MethodGet, - "/api/v4/projects/3", - http.StatusOK, - `{ - "id": 3, - "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git" - }`, + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, }, }, shelloutStubs: []string{ - "HEAD branch: master\n", + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "git@gitlab.com:OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "oldcommit\n", + "oldcommit\n", + "\n", + "\n", + "\n", "\n", "\n", - heredoc.Doc(` - deadbeef HEAD - deadb00f refs/remotes/upstream/feat-new-mr - deadbeef refs/remotes/origin/feat-new-mr - `), }, - expectedShellouts: []string{ - "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git fetch git@gitlab.com:OWNER/REPO.git +refs/heads/feat-new-mr:feat-new-mr", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", "git config branch.feat-new-mr.remote git@gitlab.com:OWNER/REPO.git", "git config branch.feat-new-mr.merge refs/heads/feat-new-mr", "git checkout feat-new-mr", }, + expectedOutput: "✓ Branch 'feat-new-mr' is up to date with the merge request.\n", + }, + { + name: "when a valid MR is checked out with HTTPS remote", + commandArgs: "123", + branch: "main", + httpMocks: []httpMock{ + { + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "https://gitlab.com/OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "oldcommit\n", + "oldcommit\n", + "\n", + "\n", + "\n", + "\n", + "\n", + }, + expectedShellouts: []string{ + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch https://gitlab.com/OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git fetch https://gitlab.com/OWNER/REPO.git +refs/heads/feat-new-mr:feat-new-mr", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", + "git config branch.feat-new-mr.remote https://gitlab.com/OWNER/REPO.git", + "git config branch.feat-new-mr.merge refs/heads/feat-new-mr", + "git checkout feat-new-mr", + }, + expectedOutput: "✓ Branch 'feat-new-mr' is up to date with the merge request.\n", }, { name: "when a valid MR comes from a forked private project", @@ -128,57 +223,72 @@ func TestMrCheckout(t *testing.T) { branch: "main", httpMocks: []httpMock{ { - http.MethodGet, - "/api/v4/projects/OWNER/REPO/merge_requests/123", - http.StatusOK, - `{ - "id": 123, - "iid": 123, - "project_id": 3, - "source_project_id": 3, - "target_project_id": 4, - "title": "test mr title", - "description": "test mr description", - "allow_collaboration": false, - "state": "opened", - "source_branch":"feat-new-mr" - }`, + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "target_project_id": 4, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, }, { - http.MethodGet, - "/api/v4/projects/4", - http.StatusOK, - `{ - "id": 4, - "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git" - }`, + method: http.MethodGet, + path: "/api/v4/projects/4", + status: http.StatusOK, + body: `{ + "id": 4, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, }, { - http.MethodGet, - "/api/v4/projects/3", - http.StatusNotFound, - `{ + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusNotFound, + body: `{ "message":"404 Project Not Found" }`, }, }, shelloutStubs: []string{ - "HEAD branch: master\n", + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "git@gitlab.com:OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "oldcommit\n", + "oldcommit\n", + "\n", + "\n", + "\n", "\n", "\n", - heredoc.Doc(` - deadbeef HEAD - deadb00f refs/remotes/upstream/feat-new-mr - deadbeef refs/remotes/origin/feat-new-mr - `), }, - expectedShellouts: []string{ - "git fetch git@gitlab.com:OWNER/REPO.git refs/merge-requests/123/head:feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch git@gitlab.com:OWNER/REPO.git refs/merge-requests/123/head:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git fetch git@gitlab.com:OWNER/REPO.git +refs/merge-requests/123/head:feat-new-mr", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", "git config branch.feat-new-mr.remote git@gitlab.com:OWNER/REPO.git", "git config branch.feat-new-mr.merge refs/merge-requests/123/head", "git checkout feat-new-mr", }, + expectedOutput: "✓ Branch 'feat-new-mr' is up to date with the merge request.\n", }, { name: "when a valid MR is checked out using MR id and specifying branch", @@ -186,50 +296,65 @@ func TestMrCheckout(t *testing.T) { branch: "main", httpMocks: []httpMock{ { - http.MethodGet, - "/api/v4/projects/OWNER/REPO/merge_requests/123", - http.StatusOK, - `{ - "id": 123, - "iid": 123, - "project_id": 3, - "source_project_id": 4, - "title": "test mr title", - "description": "test mr description", - "allow_collaboration": true, - "state": "opened", - "source_branch":"feat-new-mr" - }`, + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 4, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": true, + "state": "opened", + "source_branch":"feat-new-mr" + }`, }, { - http.MethodGet, - "/api/v4/projects/4", - http.StatusOK, - `{ - "id": 3, - "ssh_url_to_repo": "git@gitlab.com:FORK_OWNER/REPO.git" - }`, + method: http.MethodGet, + path: "/api/v4/projects/4", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:FORK_OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/FORK_OWNER/REPO.git" + }`, }, }, shelloutStubs: []string{ - "HEAD branch: master\n", + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "https://gitlab.com/OWNER/REPO.git\n", + "refs/heads/foo\n", + "main\n", + "\n", + "oldcommit\n", + "oldcommit\n", + "\n", + "\n", + "\n", "\n", "\n", "\n", - heredoc.Doc(` - deadbeef HEAD - deadb00f refs/remotes/upstream/feat-new-mr - deadbeef refs/remotes/origin/feat-new-mr - `), }, - expectedShellouts: []string{ - "git fetch git@gitlab.com:FORK_OWNER/REPO.git refs/heads/feat-new-mr:foo", - "git config branch.foo.remote git@gitlab.com:FORK_OWNER/REPO.git", - "git config branch.foo.pushRemote git@gitlab.com:FORK_OWNER/REPO.git", + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/foo", + "git symbolic-ref --quiet --short HEAD", + "git fetch https://gitlab.com/FORK_OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/foo", + "git rev-parse foo", + "git rev-parse refs/remotes/mr-checkout-temp/foo", + "git fetch https://gitlab.com/FORK_OWNER/REPO.git +refs/heads/feat-new-mr:foo", + "git update-ref -d refs/remotes/mr-checkout-temp/foo", + "git config branch.foo.remote https://gitlab.com/FORK_OWNER/REPO.git", + "git config branch.foo.pushRemote https://gitlab.com/FORK_OWNER/REPO.git", "git config branch.foo.merge refs/heads/feat-new-mr", "git checkout foo", }, + expectedOutput: "✓ Branch 'foo' is up to date with the merge request.\n", }, } @@ -251,14 +376,503 @@ func TestMrCheckout(t *testing.T) { output, err := runCommand(fakeHTTP, tc.branch, false, tc.commandArgs) if assert.NoErrorf(t, err, "error running command `mr checkout %s`: %v", tc.commandArgs, err) { - assert.Empty(t, output.String()) + if tc.expectedOutput != "" { + assert.Equal(t, tc.expectedOutput, output.String()) + } else { + assert.Empty(t, output.String()) + } assert.Empty(t, output.Stderr()) } assert.Equal(t, len(tc.expectedShellouts), cs.Count) for idx, expectedShellout := range tc.expectedShellouts { - assert.Equal(t, expectedShellout, strings.Join(cs.Calls[idx].Args, " ")) + if idx < len(cs.Calls) { + assert.Equal(t, expectedShellout, strings.Join(cs.Calls[idx].Args, " ")) + } + } + }) + } +} + +func TestMrCheckoutWithBranchComparison(t *testing.T) { + type httpMock struct { + method string + path string + status int + body string + } + + tests := []struct { + name string + commandArgs string + branch string + httpMocks []httpMock + shelloutStubs []string + expectedShellouts []string + expectedError bool + expectedOutput string + }{ + { + name: "when branch exists and is behind", + commandArgs: "123", + branch: "main", + httpMocks: []httpMock{ + { + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "git@gitlab.com:OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "deadbeef\n", + "newbeef\n", + "2\n", + "0\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + }, + expectedShellouts: []string{ + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count feat-new-mr..refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count refs/remotes/mr-checkout-temp/feat-new-mr..feat-new-mr", + "git status --porcelain", + "git fetch git@gitlab.com:OWNER/REPO.git +refs/heads/feat-new-mr:feat-new-mr", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", + "git config branch.feat-new-mr.remote git@gitlab.com:OWNER/REPO.git", + "git config branch.feat-new-mr.merge refs/heads/feat-new-mr", + "git checkout feat-new-mr", + }, + expectedOutput: "\n⚠️ Your local branch 'feat-new-mr' is 2 commit(s) behind the merge request.\n\nUpdating branch to match the merge request...\n", + }, + { + name: "when branch has local commits", + commandArgs: "123", + branch: "main", + httpMocks: []httpMock{ + { + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "git@gitlab.com:OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "deadbeef\n", + "newbeef\n", + "0\n", + "2\n", + "\n", + }, + expectedShellouts: []string{ + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count feat-new-mr..refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count refs/remotes/mr-checkout-temp/feat-new-mr..feat-new-mr", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", + }, + expectedError: true, + expectedOutput: "\n⚠️ Your local branch 'feat-new-mr' is 2 commit(s) ahead of the merge request.\n\n❌ Cannot update branch: You have local commits that would be lost.\n", + }, + { + name: "when branch has diverged", + commandArgs: "123", + branch: "main", + httpMocks: []httpMock{ + { + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "git@gitlab.com:OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "deadbeef\n", + "newbeef\n", + "3\n", + "2\n", + "\n", + }, + expectedShellouts: []string{ + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count feat-new-mr..refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count refs/remotes/mr-checkout-temp/feat-new-mr..feat-new-mr", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", + }, + expectedError: true, + expectedOutput: "\n⚠️ Your local branch 'feat-new-mr' has diverged from the merge request:\n - 2 commit(s) ahead (local commits not in MR)\n - 3 commit(s) behind (MR commits not in local)\n\n❌ Cannot update branch: You have local commits that would be lost.\n", + }, + { + name: "when branch has uncommitted changes", + commandArgs: "123", + branch: "main", + httpMocks: []httpMock{ + { + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "git@gitlab.com:OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "deadbeef\n", + "newbeef\n", + "2\n", + "0\n", + "M file.txt\n?? new-file.txt\n", + "\n", + }, + expectedShellouts: []string{ + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count feat-new-mr..refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-list --count refs/remotes/mr-checkout-temp/feat-new-mr..feat-new-mr", + "git status --porcelain", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", + }, + expectedError: true, + expectedOutput: "\n⚠️ Your local branch 'feat-new-mr' is 2 commit(s) behind the merge request.\n\n❌ Cannot update branch: You have 2 uncommitted change(s) that would be lost.\n", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + for _, mock := range tc.httpMocks { + fakeHTTP.RegisterResponder(mock.method, mock.path, httpmock.NewStringResponse(mock.status, mock.body)) + } + + cs, csTeardown := test.InitCmdStubber() + defer csTeardown() + for _, stub := range tc.shelloutStubs { + cs.Stub(stub) + } + + output, err := runCommand(fakeHTTP, tc.branch, false, tc.commandArgs) + + if tc.expectedError { + assert.Error(t, err, "expected error for test case: %s", tc.name) + } else { + assert.NoErrorf(t, err, "error running command `mr checkout %s`: %v", tc.commandArgs, err) + assert.Empty(t, output.Stderr()) + } + + if tc.expectedOutput != "" { + assert.Equal(t, tc.expectedOutput, output.String()) + } + + assert.Equal(t, len(tc.expectedShellouts), cs.Count, "number of shell commands for test case: %s", tc.name) + for idx, expectedShellout := range tc.expectedShellouts { + if idx < len(cs.Calls) { + assert.Equal(t, expectedShellout, strings.Join(cs.Calls[idx].Args, " ")) + } } }) } } + +func TestMatchingProtocolURL(t *testing.T) { + project := &gitlab.Project{ + SSHURLToRepo: "git@gitlab.com:owner/repo.git", + HTTPURLToRepo: "https://gitlab.com/owner/repo.git", + } + + tests := []struct { + name string + existingURL string + expected string + }{ + { + name: "SSH URL returns SSH", + existingURL: "git@gitlab.com:other/repo.git", + expected: "git@gitlab.com:owner/repo.git", + }, + { + name: "SSH protocol URL returns SSH", + existingURL: "ssh://git@gitlab.com/other/repo.git", + expected: "git@gitlab.com:owner/repo.git", + }, + { + name: "HTTPS URL returns HTTPS", + existingURL: "https://gitlab.com/other/repo.git", + expected: "https://gitlab.com/owner/repo.git", + }, + { + name: "HTTP URL returns HTTPS", + existingURL: "http://gitlab.com/other/repo.git", + expected: "https://gitlab.com/owner/repo.git", + }, + { + name: "Unknown protocol returns HTTPS", + existingURL: "ftp://gitlab.com/other/repo.git", + expected: "https://gitlab.com/owner/repo.git", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := matchingProtocolURL(tc.existingURL, project) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestMrCheckoutWithSetUpstream(t *testing.T) { + tests := []struct { + name string + commandArgs string + branch string + httpMocks []httpMock + shelloutStubs []string + expectedShellouts []string + expectedError bool + expectedOutput string + }{ + { + name: "when --set-upstream-to is used with valid remote", + commandArgs: "123 --set-upstream-to upstream/main", + branch: "main", + httpMocks: []httpMock{ + { + method: http.MethodGet, + path: "/api/v4/projects/OWNER/REPO/merge_requests/123", + status: http.StatusOK, + body: `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + method: http.MethodGet, + path: "/api/v4/projects/3", + status: http.StatusOK, + body: `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git", + "http_url_to_repo": "https://gitlab.com/OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "main\n", + "branch.main.remote origin\nbranch.main.merge refs/heads/main\n", + "git@gitlab.com:OWNER/REPO.git\n", + "refs/heads/feat-new-mr\n", + "main\n", + "\n", + "oldcommit\n", + "oldcommit\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + }, + expectedShellouts: []string{ + "git symbolic-ref --quiet --short HEAD", + "git config --get-regexp ^branch\\.main\\.(remote|merge)$", + "git config remote.origin.url", + "git rev-parse --verify refs/heads/feat-new-mr", + "git symbolic-ref --quiet --short HEAD", + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:refs/remotes/mr-checkout-temp/feat-new-mr", + "git rev-parse feat-new-mr", + "git rev-parse refs/remotes/mr-checkout-temp/feat-new-mr", + "git fetch git@gitlab.com:OWNER/REPO.git +refs/heads/feat-new-mr:feat-new-mr", + "git update-ref -d refs/remotes/mr-checkout-temp/feat-new-mr", + "git config branch.feat-new-mr.remote git@gitlab.com:OWNER/REPO.git", + "git config branch.feat-new-mr.merge refs/heads/feat-new-mr", + "git checkout feat-new-mr", + "git branch --set-upstream-to upstream/main", + }, + expectedOutput: "✓ Branch 'feat-new-mr' is up to date with the merge request.\n", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + for _, mock := range tc.httpMocks { + fakeHTTP.RegisterResponder(mock.method, mock.path, httpmock.NewStringResponse(mock.status, mock.body)) + } + + cs, csTeardown := test.InitCmdStubber() + defer csTeardown() + for _, stub := range tc.shelloutStubs { + cs.Stub(stub) + } + + output, err := runCommand(fakeHTTP, tc.branch, false, tc.commandArgs) + + if tc.expectedError { + assert.Error(t, err, "expected error for test case: %s", tc.name) + } else { + assert.NoErrorf(t, err, "error running command `mr checkout %s`: %v", tc.commandArgs, err) + assert.Empty(t, output.Stderr()) + } + + if tc.expectedOutput != "" { + assert.Equal(t, tc.expectedOutput, output.String()) + } + + assert.Equal(t, len(tc.expectedShellouts), cs.Count, "number of shell commands for test case: %s", tc.name) + for idx, expectedShellout := range tc.expectedShellouts { + if idx < len(cs.Calls) { + assert.Equal(t, expectedShellout, strings.Join(cs.Calls[idx].Args, " ")) + } + } + }) + } +} + +type httpMock struct { + method string + path string + status int + body string +} diff --git a/pkg/git/git.go b/pkg/git/git.go index a56e258cff37c1b485660dc347ec9d55c6ed890a..22cb47fa64dd267c58676be48379edc41aca0869 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -604,3 +604,128 @@ func ListTags() ([]string, error) { return strings.Fields(tagsStr), nil } + +// BranchComparison represents the relationship between two git refs +type BranchComparison struct { + LocalSHA string + RemoteSHA string + AheadCount string + BehindCount string + IsUpToDate bool + HasDiverged bool + IsAhead bool + IsBehind bool +} + +// CompareBranches compares a local branch with a remote ref and returns detailed comparison info +func CompareBranches(localBranch, remoteRef string) (*BranchComparison, error) { + // Get local SHA + localCmd := GitCommand("rev-parse", localBranch) + localOutput, err := run.PrepareCmd(localCmd).Output() + if err != nil { + return nil, fmt.Errorf("failed to get local branch SHA: %w", err) + } + localSHA := strings.TrimSpace(string(localOutput)) + + // Get remote SHA + remoteCmd := GitCommand("rev-parse", remoteRef) + remoteOutput, err := run.PrepareCmd(remoteCmd).Output() + if err != nil { + return nil, fmt.Errorf("failed to get remote ref SHA: %w", err) + } + remoteSHA := strings.TrimSpace(string(remoteOutput)) + + comparison := &BranchComparison{ + LocalSHA: localSHA, + RemoteSHA: remoteSHA, + } + + // If SHAs are the same, branches are up to date + if localSHA == remoteSHA { + comparison.IsUpToDate = true + return comparison, nil + } + + behindCmd := GitCommand("rev-list", "--count", fmt.Sprintf("%s..%s", localBranch, remoteRef)) + behindOutput, _ := run.PrepareCmd(behindCmd).Output() + comparison.BehindCount = strings.TrimSpace(string(behindOutput)) + + aheadCmd := GitCommand("rev-list", "--count", fmt.Sprintf("%s..%s", remoteRef, localBranch)) + aheadOutput, _ := run.PrepareCmd(aheadCmd).Output() + comparison.AheadCount = strings.TrimSpace(string(aheadOutput)) + + comparison.IsAhead = comparison.AheadCount != "0" + comparison.IsBehind = comparison.BehindCount != "0" + comparison.HasDiverged = comparison.IsAhead && comparison.IsBehind + + return comparison, nil +} + +// FetchRefToTempRef fetches a remote ref to a temporary local ref for comparison +func FetchRefToTempRef(remoteURL, sourceRef, tempRef string) error { + tempFetchSpec := fmt.Sprintf("%s:%s", sourceRef, tempRef) + if err := RunCmd([]string{"fetch", remoteURL, tempFetchSpec}); err != nil { + return fmt.Errorf("failed to fetch to temp ref: %w", err) + } + return nil +} + +func DeleteRef(ref string) error { + return RunCmd([]string{"update-ref", "-d", ref}) +} + +// HasUncommittedChanges checks if there are any uncommitted changes that would prevent a branch update +func HasUncommittedChanges() (bool, int, error) { + count, err := UncommittedChangeCount() + if err != nil { + return false, 0, err + } + return count > 0, count, nil +} + +// UpdateBranchToRef safely updates a local branch to match a remote ref +func UpdateBranchToRef(branchName, remoteURL, remoteRef string, currentBranch string, branchExists bool) error { + isOnTargetBranch := currentBranch == branchName + + if isOnTargetBranch && branchExists { + // We're on the branch we want to update - use fetch + reset approach + if err := RunCmd([]string{"fetch", remoteURL, remoteRef}); err != nil { + return fmt.Errorf("failed to fetch: %w", err) + } + + // Check for uncommitted changes + hasChanges, changeCount, err := HasUncommittedChanges() + if err != nil { + return fmt.Errorf("failed to check for uncommitted changes: %w", err) + } + if hasChanges { + return fmt.Errorf("branch has %d uncommitted changes that would be lost", changeCount) + } + + // Check if local branch has commits that would be lost + comparison, err := CompareBranches(branchName, "FETCH_HEAD") + if err != nil { + return fmt.Errorf("failed to compare branches: %w", err) + } + if comparison.IsAhead { + return fmt.Errorf("branch has %s local commits that would be lost", comparison.AheadCount) + } + + // Reset the current branch to match the fetched ref + if err := RunCmd([]string{"reset", "--hard", "FETCH_HEAD"}); err != nil { + return fmt.Errorf("failed to reset branch: %w", err) + } + } else { + // Not on the target branch, can use normal fetch + fetchRefSpec := fmt.Sprintf("%s:%s", remoteRef, branchName) + if branchExists { + fetchRefSpec = fmt.Sprintf("+%s:%s", remoteRef, branchName) + } + + if err := RunCmd([]string{"fetch", remoteURL, fetchRefSpec}); err != nil { + return fmt.Errorf("failed to fetch to branch: %w", err) + } + } + + return nil +}