From bf666814a6a0fcf878dc56c0b88dcb6b1f10827e Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Tue, 7 Nov 2023 21:07:40 +0100 Subject: [PATCH 1/4] refactor: remove unused error reasons This compose for backport script failure method was a remnant from when this action used a shell script to perform the git commands. Most reasons were dead code. --- src/backport.ts | 72 ++++++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/src/backport.ts b/src/backport.ts index ec395cc..a7b9915 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -176,9 +176,8 @@ export class Backport { this.config.pwd, ); } catch (error) { - const message = this.composeMessageForBackportScriptFailure( + const message = this.composeMessageForCheckoutFailure( target, - 3, baseref, headref, branchname, @@ -197,9 +196,8 @@ export class Backport { try { await this.git.cherryPick(commitShasToCherryPick, this.config.pwd); } catch (error) { - const message = this.composeMessageForBackportScriptFailure( + const message = this.composeMessageForCherryPickFailure( target, - 4, baseref, headref, branchname, @@ -380,42 +378,60 @@ export class Backport { Please ensure that this Github repo has a branch named \`${target}\`.`; } - private composeMessageForBackportScriptFailure( + private composeMessageForCheckoutFailure( target: string, - exitcode: number, baseref: string, headref: string, branchname: string, ): string { - const reasons: { [key: number]: string } = { - 1: "due to an unknown script error", - 2: "because it was unable to create/access the git worktree directory", - 3: "because it was unable to create a new branch", - 4: "because it was unable to cherry-pick the commit(s)", - 5: "because 1 or more of the commits are not available", - 6: "because 1 or more of the commits are not available", - }; - const reason = reasons[exitcode] ?? "due to an unknown script error"; - - const suggestion = - exitcode <= 4 - ? dedent`\`\`\`bash - git fetch origin ${target} - git worktree add -d .worktree/${branchname} origin/${target} - cd .worktree/${branchname} - git checkout -b ${branchname} - ancref=$(git merge-base ${baseref} ${headref}) - git cherry-pick -x $ancref..${headref} - \`\`\`` - : dedent`Note that rebase and squash merges are not supported at this time. - For more information see https://github.com/korthout/backport-action/issues/46.`; + const reason = "because it was unable to create a new branch"; + const suggestion = this.composeSuggestion( + target, + branchname, + baseref, + headref, + ); + return dedent`Backport failed for \`${target}\`, ${reason}. + Please cherry-pick the changes locally. + ${suggestion}`; + } + + private composeMessageForCherryPickFailure( + target: string, + baseref: string, + headref: string, + branchname: string, + ): string { + const reason = "because it was unable to cherry-pick the commit(s)"; + const suggestion = this.composeSuggestion( + target, + branchname, + baseref, + headref, + ); return dedent`Backport failed for \`${target}\`, ${reason}. Please cherry-pick the changes locally. ${suggestion}`; } + private composeSuggestion( + target: string, + branchname: string, + baseref: string, + headref: string, + ) { + return dedent`\`\`\`bash + git fetch origin ${target} + git worktree add -d .worktree/${branchname} origin/${target} + cd .worktree/${branchname} + git checkout -b ${branchname} + ancref=$(git merge-base ${baseref} ${headref}) + git cherry-pick -x $ancref..${headref} + \`\`\``; + } + private composeMessageForGitPushFailure( target: string, exitcode: number, From 3e20570b16fb7d75e1a3f4c8fd3d12180ffb3542 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Tue, 7 Nov 2023 21:17:19 +0100 Subject: [PATCH 2/4] fix: suggest to cherry-pick specific commits The manual instructions still suggested to determine the commits to cherry-pick by determining the common ancestor (merge-base) of the head and base refs, first. However, this way has been abandoned for quite some time now. This especially became a problem now that merge commits can be skipped, meaning that the suggestion no longer produced a reliable result. Instead, we can simply suggest the same as the action does. It knows exactly which commits to cherry-pick. --- src/backport.ts | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/backport.ts b/src/backport.ts index a7b9915..5b4b8b3 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -53,8 +53,6 @@ export class Backport { const repo = payload.repository?.name ?? this.github.getRepo().repo; const pull_number = this.github.getPullNumber(); const mainpr = await this.github.getPullRequest(pull_number); - const headref = mainpr.head.sha; - const baseref = mainpr.base.sha; if (!(await this.github.isMerged(mainpr))) { const message = "Only merged pull requests can be backported."; @@ -178,9 +176,8 @@ export class Backport { } catch (error) { const message = this.composeMessageForCheckoutFailure( target, - baseref, - headref, branchname, + commitShasToCherryPick, ); console.error(message); successByTarget.set(target, false); @@ -198,9 +195,8 @@ export class Backport { } catch (error) { const message = this.composeMessageForCherryPickFailure( target, - baseref, - headref, branchname, + commitShasToCherryPick, ); console.error(message); successByTarget.set(target, false); @@ -380,16 +376,14 @@ export class Backport { private composeMessageForCheckoutFailure( target: string, - baseref: string, - headref: string, branchname: string, + commitShasToCherryPick: string[], ): string { const reason = "because it was unable to create a new branch"; const suggestion = this.composeSuggestion( target, branchname, - baseref, - headref, + commitShasToCherryPick, ); return dedent`Backport failed for \`${target}\`, ${reason}. @@ -399,16 +393,14 @@ export class Backport { private composeMessageForCherryPickFailure( target: string, - baseref: string, - headref: string, branchname: string, + commitShasToCherryPick: string[], ): string { const reason = "because it was unable to cherry-pick the commit(s)"; const suggestion = this.composeSuggestion( target, branchname, - baseref, - headref, + commitShasToCherryPick, ); return dedent`Backport failed for \`${target}\`, ${reason}. @@ -419,16 +411,14 @@ export class Backport { private composeSuggestion( target: string, branchname: string, - baseref: string, - headref: string, + commitShasToCherryPick: string[], ) { return dedent`\`\`\`bash git fetch origin ${target} git worktree add -d .worktree/${branchname} origin/${target} cd .worktree/${branchname} git checkout -b ${branchname} - ancref=$(git merge-base ${baseref} ${headref}) - git cherry-pick -x $ancref..${headref} + git cherry-pick -x ${commitShasToCherryPick.join(" ")} \`\`\``; } From 4fed99722ae85c403587c07d09e715f239cb8c55 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Tue, 7 Nov 2023 21:23:58 +0100 Subject: [PATCH 3/4] fix: instruct to resolve conflicts When the cherr-picking failed, it's very likely that there are conflicts. We should instruct the user to cherry-pick locally and resolve any conflicts. --- src/backport.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backport.ts b/src/backport.ts index 5b4b8b3..79dd41d 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -404,7 +404,7 @@ export class Backport { ); return dedent`Backport failed for \`${target}\`, ${reason}. - Please cherry-pick the changes locally. + Please cherry-pick the changes locally and resolve any conflicts. ${suggestion}`; } From 19a4042a81561b087a003ecd52439e6477cf2c7d Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Tue, 7 Nov 2023 21:32:20 +0100 Subject: [PATCH 4/4] fix: suggest git switch over checkout The switch command has been around for 4 years now, and while still marked as experimental, I think it's fine to "switch" to it in the suggestion for local cherry-picking. It's unlikely that users are using such old git versions that it doesn't support git switch --create. --- src/backport.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backport.ts b/src/backport.ts index 79dd41d..50a4c1a 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -417,7 +417,7 @@ export class Backport { git fetch origin ${target} git worktree add -d .worktree/${branchname} origin/${target} cd .worktree/${branchname} - git checkout -b ${branchname} + git switch --create ${branchname} git cherry-pick -x ${commitShasToCherryPick.join(" ")} \`\`\``; }