Conflict resolve suggestion doesn't fetch commits to cherry-pick

[GuyAv46]

Describe the bug

When backporting a PR that was merged with the "squash and merge" method, and the backporting fails (due to conflicts), the suggestion message doesn't work, because it does not fetch the original PR target branch, and therefore the squashed commit sha is unknown locally.

To Reproduce

1. Add a label to backport a PR to a branch that will cause conflicts
2. Squash and merge the PR
3. Copy the suggestion to the local work env
4. See that it fails to cherry-pick

Expected behavior
If the "squash and merge" method was detected, and a failure occurred, include the original PR's base branch or the squashed commit sha at the beginning of the suggestion:

      git fetch origin ${target} ${original target branch}
      git worktree add -d .worktree/${branchname} origin/${target}
      cd .worktree/${branchname}
      git switch --create ${branchname}
      git cherry-pick -x ${commitShasToCherryPick.join(" ")}

or

      git fetch origin ${target} ${squashed commit sha}
      git worktree add -d .worktree/${branchname} origin/${target}
      cd .worktree/${branchname}
      git switch --create ${branchname}
      git cherry-pick -x ${commitShasToCherryPick.join(" ")}

[AlexVermette-Eaton]

@GuyAv46 Can you provide the version you are using for this workflow?

[GuyAv46]

I used the new v3 tag. From the run logs

Download action repository 'korthout/backport-action@v3' 
(SHA:bd410d37cdcae80be6d969823ff5a225fe5c833f)

after more thinking this problem might happen when the action detects a “rebase and merge” method. Maybe it is useful to fetch the head branch in this case? Or all the about-to-be-cherry-picked commits?

[GuyAv46]

Why not just call git fetch with no specific branch for all the cases?

[korthout]

Thanks for reporting @GuyAv46

You're right that the suggestion currently doesn't fetch all the required references. The commits to cherry-pick may not be known locally.

Reading the suggestions again, I believe this issue appears for all merge methods (including merge commit), regardless of the new experimental conflict_resolution setting.

By default:

backport-action/src/backport.ts

Lines 628 to 634 in bd410d3

	return dedent`\`\`\`bash
	git fetch origin ${target}
	git worktree add -d .worktree/${branchname} origin/${target}
	cd .worktree/${branchname}
	git switch --create ${branchname}
	git cherry-pick -x ${commitShasToCherryPick.join(" ")}
	\`\`\``;

With conflict_resolution: draft_commit_conflicts:

backport-action/src/backport.ts

Lines 616 to 623 in bd410d3

	return dedent`\`\`\`bash
	git fetch origin ${branchname}
	git worktree add --checkout .worktree/${branchname} ${branchname}
	cd .worktree/${branchname}
	git reset --hard HEAD^
	git cherry-pick -x ${commitShasToCherryPick.join(" ")}
	git push --force-with-lease
	\`\`\``;

In both cases, the commits to cherry-pick may be unreachable after the suggested git fetch.

Example:

• start on branch a
• create a new branch b
• make a change to merge back to a, open a pull request, merge it
• backport the pull request to some branch c where it conflicts for some reason
• now try the default suggestions:
  • fetch the target branch: c
  • try to cherry-pick commits from merging the pull request
    • on merge commit: the commits on b - not reachable
    • on rebase: the rebased commits on a after the merge - not reachable
    • on squash: the squashed commit on a after the merge - not reachable
• or try the other suggestion:
  • fetch the newly created backport branch with conflicts
  • try to cherry-pick commits from merging the pull request
    • on merge commit: the commits b - not reachable
    • on rebase: the rebased commits on a after the merge - not reachable
    • on squash: the squashed commit on a after the merge - not reachable

We must add an additional fetch based on the merge method to resolve this. The action already does this, so we should also suggest to do this locally:
On squash:

backport-action/src/backport.ts

Lines 147 to 155 in bd410d3

	case MergeStrategy.SQUASHED:
	// If merged via a squash merge_commit_sha represents the SHA of the squashed commit on
	// the base branch. We must fetch it and its parent in case of a shallowly cloned repo
	// To store the fetched commits indefinitely we save them to a remote ref using the sha
	await this.git.fetch(
	`+${merge_commit_sha}:refs/remotes/origin/${merge_commit_sha}`,
	this.config.pwd,
	2, // +1 in case this concerns a shallowly cloned repo
	);

On rebase:

backport-action/src/backport.ts

Lines 158 to 166 in bd410d3

	case MergeStrategy.REBASED:
	// If rebased merge_commit_sha represents the commit that the base branch was updated to
	// We must fetch it, its parents, and one extra parent in case of a shallowly cloned repo
	// To store the fetched commits indefinitely we save them to a remote ref using the sha
	await this.git.fetch(
	`+${merge_commit_sha}:refs/remotes/origin/${merge_commit_sha}`,
	this.config.pwd,
	mainpr.commits + 1, // +1 in case this concerns a shallowly cloned repo
	);

Otherwise:

backport-action/src/backport.ts

Lines 132 to 136 in bd410d3

	await this.git.fetch(
	`refs/pull/${pull_number}/head`,
	this.config.pwd,
	mainpr.commits + 1, // +1 in case this concerns a shallowly cloned repo
	);

[korthout]

As a quick fix, we can add a git fetch without any references (it performs less). We must keep the git fetch target for the default behavior because the branch of the merged pull request may have already been deleted and not passed along with a regular git fetch. My feeling is that even this solution doesn't cover all cases correctly. For example, what if this concerns a pull request from a fork, or if the remotes are named differently locally.

Eventually, I think we'll need to switch to explicit github refs and commit shas in the fetch.

Otherwise, we could simply add a note about this bug to the suggestion. I'm also open to other ideas.

@GuyAv46 @AlexVermette-Eaton, I'd welcome new contributions to resolve this 🙇. Let me know if you're interested in giving it a try. Otherwise, I'll pick this up eventually, but I don't have time at the moment. While annoying, I don't see this as a critical bug (using git fetch will in most cases resolve the problem as a workaround).