From fb97b3efaabaf3a727beb6ed5f4adbf9e780f9ff Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 24 Apr 2025 18:41:14 +0200 Subject: [PATCH] Fix pr create when push.default tracking and no merge ref (#10863) * Fix pr create when push.default tracking and no merge ref * Update pkg/cmd/pr/shared/find_refs_resolution.go --------- Co-authored-by: Tyler McGoffin --- ...h-default-upstream-no-merge-ref-fork.txtar | 50 +++++++++++++++++++ ...e-push-default-upstream-no-merge-ref.txtar | 33 ++++++++++++ pkg/cmd/pr/shared/find_refs_resolution.go | 8 +-- .../pr/shared/find_refs_resolution_test.go | 8 +-- 4 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref-fork.txtar create mode 100644 acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref.txtar diff --git a/acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref-fork.txtar b/acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref-fork.txtar new file mode 100644 index 000000000..0974f9225 --- /dev/null +++ b/acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref-fork.txtar @@ -0,0 +1,50 @@ +skip 'it creates a fork owned by the user running the test' +skip 'this never worked, but could be fixed if we fixed show-refs' + +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} +env FORK=${REPO}-fork + +# Use gh as a credential helper +exec gh auth setup-git + +# Get the current username for the fork owner +exec gh api user --jq .login +stdout2env USER + +# Create a repository to act as upstream with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup of upstream +defer gh repo delete --yes ${ORG}/${REPO} + +# Create a user fork of repository. This will be owned by USER. +exec gh repo fork ${ORG}/${REPO} --fork-name ${FORK} +sleep 5 + +# Defer repo cleanup of fork +defer gh repo delete --yes ${USER}/${FORK} + +# Retrieve fork repository information +exec gh repo view ${USER}/${FORK} --json id --jq '.id' +stdout2env FORK_ID + +# Clone the repo +exec gh repo clone ${USER}/${FORK} +cd ${FORK} + +# Configure push.default so that it should use the merge ref +exec git config push.default upstream + +# But prepare a branch that doesn't have a tracking merge ref +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1 + +# Assert that the PR was created with the correct head repository and refs +exec gh pr view --json headRefName,headRepository,baseRefName,isCrossRepository +stdout {"baseRefName":"main","headRefName":"feature-branch","headRepository":{"id":"${FORK_ID}","name":"${FORK}"},"isCrossRepository":true} diff --git a/acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref.txtar b/acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref.txtar new file mode 100644 index 000000000..90c5cde50 --- /dev/null +++ b/acceptance/testdata/pr/pr-create-push-default-upstream-no-merge-ref.txtar @@ -0,0 +1,33 @@ +skip 'it creates a fork owned by the user running the test' + +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Get the current username for the fork owner +exec gh api user --jq .login +stdout2env USER + +# Create a repository to act as upstream with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup of upstream +defer gh repo delete --yes ${ORG}/${REPO} + +# Clone the repo +exec gh repo clone ${ORG}/${REPO} +cd ${REPO} + +# Configure push.default so that it should use the merge ref +exec git config push.default upstream + +# But prepare a branch that doesn't have a tracking merge ref +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1 diff --git a/pkg/cmd/pr/shared/find_refs_resolution.go b/pkg/cmd/pr/shared/find_refs_resolution.go index 833075af8..e4e51bab8 100644 --- a/pkg/cmd/pr/shared/find_refs_resolution.go +++ b/pkg/cmd/pr/shared/find_refs_resolution.go @@ -333,12 +333,12 @@ func tryDetermineDefaultPushTarget(gitClient GitConfigClient, localBranchName st } // We assume the PR's branch name is the same as whatever was provided, unless the user has specified - // push.default = upstream or tracking, then we use the branch name from the merge ref. + // push.default = upstream or tracking, then we use the branch name from the merge ref if it exists. Otherwise, we fall back to the local branch name remoteBranch := localBranchName if pushDefault == git.PushDefaultUpstream || pushDefault == git.PushDefaultTracking { - remoteBranch = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") - if remoteBranch == "" { - return defaultPushTarget{}, fmt.Errorf("could not determine remote branch name") + mergeRef := strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") + if mergeRef != "" { + remoteBranch = mergeRef } } diff --git a/pkg/cmd/pr/shared/find_refs_resolution_test.go b/pkg/cmd/pr/shared/find_refs_resolution_test.go index 8cbb62146..d2393bf10 100644 --- a/pkg/cmd/pr/shared/find_refs_resolution_test.go +++ b/pkg/cmd/pr/shared/find_refs_resolution_test.go @@ -462,7 +462,7 @@ func TestTryDetermineDefaultPRHead(t *testing.T) { }) } - t.Run("but if the merge ref is empty, error", func(t *testing.T) { + t.Run("but if the merge ref is empty, use the provided branch name", func(t *testing.T) { t.Parallel() repoResolvedFromPushRemoteClient := stubGitConfigClient{ @@ -474,12 +474,14 @@ func TestTryDetermineDefaultPRHead(t *testing.T) { pushDefaultFn: stubPushDefault(git.PushDefaultUpstream, nil), } - _, err := TryDetermineDefaultPRHead( + defaultPRHead, err := TryDetermineDefaultPRHead( repoResolvedFromPushRemoteClient, stubRemoteToRepoResolver(ghContext.Remotes{&baseRemote, &forkRemote}, nil), "feature-branch", ) - require.Error(t, err) + require.NoError(t, err) + + require.Equal(t, "feature-branch", defaultPRHead.BranchName) }) })