Merge pull request #377 from korthout/revert-376-revert-375-skipMerge…

…Commits

Re-add "Skip merge commits""

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:

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.

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,47 @@ 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.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);
+        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.length > 0 &&
+        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 +192,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,

src/git.ts

@@ -52,6 +52,27 @@ export class Git {
     }
   }
 
+  public async findMergeCommits(
+    commitShas: string[],
+    pwd: string,
+  ): Promise<string[]> {
+    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",

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<void> {
   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);
 

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"]);
+    });
+  });
+});