Skip to content

[gh pr checkout] Add --no-tags option to git fetch commands in checko… #10479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
williammartin merged 2 commits into from
Feb 21, 2025
Merged

[gh pr checkout] Add --no-tags option to git fetch commands in checko… #10479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e331daa
[gh pr checkout] Add --no-tags option to git fetch commands in checko…
latzskim Feb 20, 2025
437c52f
Test pr checkout force on different branch
williammartin Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions pkg/cmd/pr/checkout/checkout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts
refSpec += fmt.Sprintf(":refs/remotes/%s", remoteBranch)
}

cmds = append(cmds, []string{"fetch", remote.Name, refSpec})
cmds = append(cmds, []string{"fetch", remote.Name, refSpec, "--no-tags"})

localBranch := pr.HeadRefName
if opts.BranchName != "" {
Expand Down Expand Up @@ -202,7 +202,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
ref := fmt.Sprintf("refs/pull/%d/head", pr.Number)

if opts.Detach {
cmds = append(cmds, []string{"fetch", baseURLOrName, ref})
cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"})
cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"})
return cmds
}
Expand All @@ -218,21 +218,20 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
currentBranch, _ := opts.Branch()
if localBranch == currentBranch {
// PR head matches currently checked out branch
cmds = append(cmds, []string{"fetch", baseURLOrName, ref})
cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"})
if opts.Force {
cmds = append(cmds, []string{"reset", "--hard", "FETCH_HEAD"})
} else {
// TODO: check if non-fast-forward and suggest to use `--force`
cmds = append(cmds, []string{"merge", "--ff-only", "FETCH_HEAD"})
}
} else {
// TODO: check if non-fast-forward and suggest to use `--force`
fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"}
if opts.Force {
cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"})
} else {
// TODO: check if non-fast-forward and suggest to use `--force`
cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)})
fetchCmd = append(fetchCmd, "--force")
}

cmds = append(cmds, fetchCmd)
cmds = append(cmds, []string{"checkout", localBranch})
}

Expand Down
55 changes: 39 additions & 16 deletions pkg/cmd/pr/checkout/checkout_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
},
},
Expand Down Expand Up @@ -140,7 +140,7 @@ func Test_checkoutRun(t *testing.T) {
"origin": "OWNER/REPO",
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
Expand Down Expand Up @@ -174,7 +174,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/foobar`, 1, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git checkout -b foobar --track origin/feature`, 0, "")
},
},
Expand Down Expand Up @@ -205,7 +205,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git config branch\.foobar\.merge`, 1, "")
cs.Register(`git fetch origin refs/pull/123/head:foobar`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:foobar --no-tags`, 0, "")
cs.Register(`git checkout foobar`, 0, "")
cs.Register(`git config branch\.foobar\.remote https://github.com/hubot/REPO.git`, 0, "")
cs.Register(`git config branch\.foobar\.pushRemote https://github.com/hubot/REPO.git`, 0, "")
Expand Down Expand Up @@ -260,7 +260,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
},
remotes: map[string]string{
Expand Down Expand Up @@ -417,7 +417,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)

cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")

Expand All @@ -436,7 +436,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "")
Expand Down Expand Up @@ -468,7 +468,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "")
cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --track robot-fork/feature`, 0, "")

Expand All @@ -488,7 +488,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
Expand All @@ -501,6 +501,29 @@ func TestPRCheckout_differentRepo(t *testing.T) {
assert.Equal(t, "", output.Stderr())
}

func TestPRCheckout_differentRepoForce(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)

baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
finder := shared.RunCommandFinder("123", pr, baseRepo)
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags --force`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
cs.Register(`git config branch\.feature\.pushRemote origin`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")

output, err := runCommand(http, nil, "master", `123 --force`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
}

func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
Expand All @@ -510,7 +533,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git checkout feature`, 0, "")

Expand All @@ -529,7 +552,7 @@ func TestPRCheckout_detachedHead(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git checkout feature`, 0, "")

Expand All @@ -548,7 +571,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git merge --ff-only FETCH_HEAD`, 0, "")

Expand Down Expand Up @@ -585,7 +608,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote https://github\.com/hubot/REPO\.git`, 0, "")
Expand All @@ -606,7 +629,7 @@ func TestPRCheckout_recurseSubmodules(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "")
Expand All @@ -627,7 +650,7 @@ func TestPRCheckout_force(t *testing.T) {

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git reset --hard refs/remotes/origin/feature`, 0, "")
Expand All @@ -649,7 +672,7 @@ func TestPRCheckout_detach(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git checkout --detach FETCH_HEAD`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "")

output, err := runCommand(http, nil, "", `123 --detach`, baseRepo)
assert.NoError(t, err)
Expand Down