From a2a0571d7b946620c03a517716c2f32665bae6fa Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Tue, 15 Aug 2023 13:43:44 +0200 Subject: [PATCH 1/3] Revert "Revert "Skip merge commits"" --- README.md | 10 ++++++++++ action.yml | 6 ++++++ src/backport.ts | 42 +++++++++++++++++++++++++++++++++++++----- src/git.ts | 21 +++++++++++++++++++++ src/main.ts | 16 ++++++++++++---- src/test/git.test.ts | 21 +++++++++++++++++++++ 6 files changed, 107 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 0b265a0..e32b948 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,16 @@ The action will backport the pull request to each matched target branch. Note that the pull request's headref is excluded automatically. See [How it works](#how-it-works). +### `merge_commits` + +Default: `fail` + +Specifies how the action should deal with merge commits on the merged pull request. + +- When set to `fail` the backport fails when the action detects one or more merge commits. +- When set to `skip` the action only cherry-picks non-merge commits, i.e. it ignores merge commits. + This can be useful when you [keep your pull requests in sync with the base branch using merge commits](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/keeping-your-pull-request-in-sync-with-the-base-branch). + ### `pull_description` Default: diff --git a/action.yml b/action.yml index 89a1f2f..ac73ff0 100644 --- a/action.yml +++ b/action.yml @@ -24,6 +24,12 @@ inputs: The action will backport the pull request to each matched target branch. Note that the pull request's headref is excluded automatically. default: ^backport ([^ ]+)$ + merge_commits: + description: > + Specifies how the action should deal with merge commits on the merged pull request. + When set to `fail` the backport fails when the action detects one or more merge commits. + When set to `skip` the action only cherry-picks non-merge commits, i.e. it ignores merge commits. + default: fail pull_description: description: > Template used as description (i.e. body) in the pull requests created by this action. diff --git a/src/backport.ts b/src/backport.ts index 3b5375f..8cf152c 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -11,7 +11,7 @@ type PRContent = { body: string; }; -type Config = { +export type Config = { pwd: string; labels: { pattern?: RegExp; @@ -22,6 +22,9 @@ type Config = { }; copy_labels_pattern?: RegExp; target_branches?: string; + commits: { + merge_commits: "fail" | "skip"; + }; }; enum Output { @@ -78,12 +81,41 @@ export class Backport { mainpr.commits + 1, // +1 in case this concerns a shallowly cloned repo ); + const commitShas = await this.github.getCommits(mainpr); + console.log(`Found commits: ${commitShas}`); + + console.log("Checking the merged pull request for merge commits"); + const mergeCommitShas = await this.git.findMergeCommits( + commitShas, + this.config.pwd, + ); console.log( - "Determining first and last commit shas, so we can cherry-pick the commit range", + `Encountered ${mergeCommitShas?.length ?? "no"} merge commits`, ); + if (mergeCommitShas && this.config.commits.merge_commits == "fail") { + const message = dedent`Backport failed because this pull request contains merge commits.\ + You can either backport this pull request manually, or configure the action to skip merge commits.`; + console.error(message); + this.github.createComment({ + owner, + repo, + issue_number: pull_number, + body: message, + }); + return; + } - const commitShas = await this.github.getCommits(mainpr); - console.log(`Found commits: ${commitShas}`); + let commitShasToCherryPick = commitShas; + if (mergeCommitShas && this.config.commits.merge_commits == "skip") { + console.log("Skipping merge commits: " + mergeCommitShas); + const nonMergeCommitShas = commitShas.filter( + (sha) => !mergeCommitShas.includes(sha), + ); + commitShasToCherryPick = nonMergeCommitShas; + } + console.log( + "Will cherry-pick the following commits: " + commitShasToCherryPick, + ); let labelsToCopy: string[] = []; if (typeof this.config.copy_labels_pattern !== "undefined") { @@ -154,7 +186,7 @@ export class Backport { } try { - await this.git.cherryPick(commitShas, this.config.pwd); + await this.git.cherryPick(commitShasToCherryPick, this.config.pwd); } catch (error) { const message = this.composeMessageForBackportScriptFailure( target, diff --git a/src/git.ts b/src/git.ts index e59a3f4..1464cac 100644 --- a/src/git.ts +++ b/src/git.ts @@ -52,6 +52,27 @@ export class Git { } } + public async findMergeCommits( + commitShas: string[], + pwd: string, + ): Promise { + const range = `${commitShas[0]}^..${commitShas[commitShas.length - 1]}`; + const { exitCode, stdout } = await this.git( + "rev-list", + ["--merges", range], + pwd, + ); + if (exitCode !== 0) { + throw new Error( + `'git rev-list --merges ${range}' failed with exit code ${exitCode}`, + ); + } + const mergeCommitShas = stdout + .split("\n") + .filter((sha) => sha.trim() !== ""); + return mergeCommitShas; + } + public async push(branchname: string, pwd: string) { const { exitCode } = await this.git( "push", diff --git a/src/main.ts b/src/main.ts index d69ee5e..b665fd0 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,5 +1,5 @@ import * as core from "@actions/core"; -import { Backport } from "./backport"; +import { Backport, Config } from "./backport"; import { Github } from "./github"; import { Git } from "./git"; import { execa } from "execa"; @@ -17,17 +17,25 @@ async function run(): Promise { const title = core.getInput("pull_title"); const copy_labels_pattern = core.getInput("copy_labels_pattern"); const target_branches = core.getInput("target_branches"); + const merge_commits = core.getInput("merge_commits"); + + if (merge_commits != "fail" && merge_commits != "skip") { + const message = `Expected input 'merge_commits' to be either 'fail' or 'skip', but was '${merge_commits}'`; + console.error(message); + core.setFailed(message); + return; + } const github = new Github(token); const git = new Git(execa); - const config = { + const config: Config = { pwd, labels: { pattern: pattern === "" ? undefined : new RegExp(pattern) }, pull: { description, title }, copy_labels_pattern: copy_labels_pattern === "" ? undefined : new RegExp(copy_labels_pattern), - target_branches: - target_branches === "" ? undefined : (target_branches as string), + target_branches: target_branches === "" ? undefined : target_branches, + commits: { merge_commits }, }; const backport = new Backport(github, config, git); diff --git a/src/test/git.test.ts b/src/test/git.test.ts index e3e9ff5..df206eb 100644 --- a/src/test/git.test.ts +++ b/src/test/git.test.ts @@ -44,3 +44,24 @@ describe("git.cherryPick", () => { }); }); }); + +describe("git.findMergeCommits", () => { + describe("throws Error", () => { + it("when failing with an unpexected non-zero exit code", async () => { + response.exitCode = 1; + await expect(git.findMergeCommits(["unknown"], "")).rejects.toThrowError( + `'git rev-list --merges unknown^..unknown' failed with exit code 1`, + ); + }); + }); + + describe("returns all merge commits", () => { + it("when git rev-list outputs them", async () => { + response.exitCode = 0; + response.stdout = "two\nfour"; + expect( + await git.findMergeCommits(["one", "two", "three", "four"], ""), + ).toEqual(["two", "four"]); + }); + }); +}); From 483eb25776f965436d59fd42a1590a9e087021ab Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Tue, 15 Aug 2023 13:45:27 +0200 Subject: [PATCH 2/3] fix: remove typo from error message --- src/backport.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backport.ts b/src/backport.ts index 8cf152c..881132f 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -93,7 +93,7 @@ export class Backport { `Encountered ${mergeCommitShas?.length ?? "no"} merge commits`, ); if (mergeCommitShas && this.config.commits.merge_commits == "fail") { - const message = dedent`Backport failed because this pull request contains merge commits.\ + const message = dedent`Backport failed because this pull request contains merge commits. \ You can either backport this pull request manually, or configure the action to skip merge commits.`; console.error(message); this.github.createComment({ From 8ffa5c7fbc8b0627b7111d683dd798af73696afe Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Tue, 15 Aug 2023 14:02:47 +0200 Subject: [PATCH 3/3] fix: check length of merge commit array I thought that an empty array would be falsy, but that's not the case. So I have to check the length of the array instead. --- src/backport.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/backport.ts b/src/backport.ts index 881132f..013baa6 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -90,9 +90,12 @@ export class Backport { this.config.pwd, ); console.log( - `Encountered ${mergeCommitShas?.length ?? "no"} merge commits`, + `Encountered ${mergeCommitShas.length ?? "no"} merge commits`, ); - if (mergeCommitShas && this.config.commits.merge_commits == "fail") { + if ( + mergeCommitShas.length > 0 && + this.config.commits.merge_commits == "fail" + ) { const message = dedent`Backport failed because this pull request contains merge commits. \ You can either backport this pull request manually, or configure the action to skip merge commits.`; console.error(message); @@ -106,7 +109,10 @@ export class Backport { } let commitShasToCherryPick = commitShas; - if (mergeCommitShas && this.config.commits.merge_commits == "skip") { + if ( + mergeCommitShas.length > 0 && + this.config.commits.merge_commits == "skip" + ) { console.log("Skipping merge commits: " + mergeCommitShas); const nonMergeCommitShas = commitShas.filter( (sha) => !mergeCommitShas.includes(sha),