feat(issue-62): make cherry-pick strategy configurable (#63)

fix #62

README.md

@@ -81,11 +81,15 @@ By default the tool will try to cherry-pick the single squashed/merged commit in
 
 Based on the original pull request, creates a new one containing the backporting to the target branch. Note that most of these information can be overridden with appropriate CLI options or GHA inputs.
 
-Right now all commits are cherry-picked using the following git-equivalent command:
+#### Default cherry-pick strategy
+
+The default cherry-pick strategy is `recursive` with `theirs` option for automatic conflicts resolution. Therefore, by default, all commits are cherry-picked using the following git-equivalent command:
 ```bash
 $ git cherry-pick -m 1 --strategy=recursive --strategy-option=theirs <sha>
 ```
 
+From version `v4.2.0` we made both `strategy` and `strategy-option` fully configurable from CLI or GitHub action, so if users need a specific strategy which differs from the default one please consider using either `--strategy` or `--strategy-option`, or their equivalent GitHub action inputs, more details in [inputs](#inputs) section.
+
 > **NOTE**: If there are any conflicts, the tool will block the process and exit signalling the failure as there are still no ways to interactively resolve them. In these cases a manual cherry-pick is needed, or alternatively users could manually resume the process in the cloned repository (here the user will have to resolve the conflicts, push the branch and create the pull request - all manually).
 
 ### Inputs
@@ -113,6 +117,8 @@ This tool comes with some inputs that allow users to override the default behavi
 | Labels       | --labels        | N            | Provide custom labels to be added to the backporting pull request                                                           | []       |
 | Inherit labels       | --inherit-labels        | N            | If enabled inherit lables from the original pull request                                                           | false       |
 | No squash       | --no-squash        | N            | If provided the backporting will try to backport all pull request commits without squashing                                                           | false       |
+| Strategy       | --strategy        | N            | Cherry pick merging strategy, see [git-merge](https://git-scm.com/docs/git-merge#_merge_strategies) doc for all possible values                                                           | "recursive"       |
+| Strategy Option       | --strategy-option        | N            | Cherry pick merging strategy option, see [git-merge](https://git-scm.com/docs/git-merge#_merge_strategies) doc for all possible values                                                           | "theirs"       |
 | Dry Run       | -d, --dry-run        | N            | If enabled the tool does not push nor create anything remotely, use this to skip PR creation                                                           | false       |
 
 > **NOTE**: `pull request` and `target branch` are *mandatory*, they must be provided as CLI options or as part of the configuration file (if used).

action.yml

@@ -59,6 +59,14 @@ inputs:
     description: "If set to true the tool will backport all commits as part of the pull request instead of the suqashed one"
     required: false
     default: "false"
+  strategy:
+    description: "Cherry-pick merge strategy"
+    required: false
+    default: "recursive"
+  strategy-option:
+    description: "Cherry-pick merge strategy option"
+    required: false
+    default: "theirs"
 
 runs:
   using: node16

dist/cli/index.js

@@ -61,6 +61,8 @@ class ArgsParser {
             labels: this.getOrDefault(args.labels, []),
             inheritLabels: this.getOrDefault(args.inheritLabels, false),
             squash: this.getOrDefault(args.squash, true),
+            strategy: this.getOrDefault(args.strategy),
+            strategyOption: this.getOrDefault(args.strategyOption),
         };
     }
 }
@@ -187,6 +189,8 @@ class CLIArgsParser extends args_parser_1.default {
             .option("--labels <labels>", "comma separated list of labels to be assigned to the backported pull request", args_utils_1.getAsCommaSeparatedList)
             .option("--inherit-labels", "if true the backported pull request will inherit labels from the original one")
             .option("--no-squash", "if provided the tool will backport all commits as part of the pull request")
+            .option("--strategy <strategy>", "cherry-pick merge strategy, default to 'recursive'", undefined)
+            .option("--strategy-option <strategy-option>", "cherry-pick merge strategy option, default to 'theirs'")
             .option("-cf, --config-file <config-file>", "configuration file containing all valid options, the json must match Args interface");
     }
     readArgs() {
@@ -217,6 +221,8 @@ class CLIArgsParser extends args_parser_1.default {
                 labels: opts.labels,
                 inheritLabels: opts.inheritLabels,
                 squash: opts.squash,
+                strategy: opts.strategy,
+                strategyOption: opts.strategyOption,
             };
         }
         return args;
@@ -295,6 +301,8 @@ class PullRequestConfigsParser extends configs_parser_1.default {
             auth: args.auth,
             folder: `${folder.startsWith("/") ? "" : process.cwd() + "/"}${args.folder ?? this.getDefaultFolder()}`,
             targetBranch: args.targetBranch,
+            mergeStrategy: args.strategy,
+            mergeStrategyOption: args.strategyOption,
             originalPullRequest: pr,
             backportPullRequest: this.getDefaultBackportPullRequest(pr, args),
             git: {
@@ -446,9 +454,19 @@ class GitCLIService {
      * @param cwd repository in which the sha should be cherry picked to
      * @param sha commit sha
      */
-    async cherryPick(cwd, sha) {
+    async cherryPick(cwd, sha, strategy = "recursive", strategyOption = "theirs") {
         this.logger.info(`Cherry picking ${sha}.`);
-        await this.git(cwd).raw(["cherry-pick", "-m", "1", "--strategy=recursive", "--strategy-option=theirs", sha]);
+        const options = ["cherry-pick", "-m", "1", `--strategy=${strategy}`, `--strategy-option=${strategyOption}`, sha];
+        try {
+            await this.git(cwd).raw(options);
+        }
+        catch (error) {
+            const diff = await this.git(cwd).diff();
+            if (diff) {
+                throw new Error(`${error}\r\nShowing git diff:\r\n` + diff);
+            }
+            throw error;
+        }
     }
     /**
      * Push a branch to a remote
@@ -1213,7 +1231,7 @@ class Runner {
         // 7. apply all changes to the new branch
         this.logger.debug("Cherry picking commits..");
         for (const sha of originalPR.commits) {
-            await git.cherryPick(configs.folder, sha);
+            await git.cherryPick(configs.folder, sha, configs.mergeStrategy, configs.mergeStrategyOption);
         }
         const backport = {
             owner: originalPR.targetRepo.owner,

dist/gha/index.js

@@ -61,6 +61,8 @@ class ArgsParser {
             labels: this.getOrDefault(args.labels, []),
             inheritLabels: this.getOrDefault(args.inheritLabels, false),
             squash: this.getOrDefault(args.squash, true),
+            strategy: this.getOrDefault(args.strategy),
+            strategyOption: this.getOrDefault(args.strategyOption),
         };
     }
 }
@@ -190,6 +192,8 @@ class GHAArgsParser extends args_parser_1.default {
                 labels: (0, args_utils_1.getAsCommaSeparatedList)((0, core_1.getInput)("labels")),
                 inheritLabels: (0, args_utils_1.getAsBooleanOrDefault)((0, core_1.getInput)("inherit-labels")),
                 squash: !(0, args_utils_1.getAsBooleanOrDefault)((0, core_1.getInput)("no-squash")),
+                strategy: (0, args_utils_1.getOrUndefined)((0, core_1.getInput)("strategy")),
+                strategyOption: (0, args_utils_1.getOrUndefined)((0, core_1.getInput)("strategy-option")),
             };
         }
         return args;
@@ -268,6 +272,8 @@ class PullRequestConfigsParser extends configs_parser_1.default {
             auth: args.auth,
             folder: `${folder.startsWith("/") ? "" : process.cwd() + "/"}${args.folder ?? this.getDefaultFolder()}`,
             targetBranch: args.targetBranch,
+            mergeStrategy: args.strategy,
+            mergeStrategyOption: args.strategyOption,
             originalPullRequest: pr,
             backportPullRequest: this.getDefaultBackportPullRequest(pr, args),
             git: {
@@ -419,9 +425,19 @@ class GitCLIService {
      * @param cwd repository in which the sha should be cherry picked to
      * @param sha commit sha
      */
-    async cherryPick(cwd, sha) {
+    async cherryPick(cwd, sha, strategy = "recursive", strategyOption = "theirs") {
         this.logger.info(`Cherry picking ${sha}.`);
-        await this.git(cwd).raw(["cherry-pick", "-m", "1", "--strategy=recursive", "--strategy-option=theirs", sha]);
+        const options = ["cherry-pick", "-m", "1", `--strategy=${strategy}`, `--strategy-option=${strategyOption}`, sha];
+        try {
+            await this.git(cwd).raw(options);
+        }
+        catch (error) {
+            const diff = await this.git(cwd).diff();
+            if (diff) {
+                throw new Error(`${error}\r\nShowing git diff:\r\n` + diff);
+            }
+            throw error;
+        }
     }
     /**
      * Push a branch to a remote
@@ -1186,7 +1202,7 @@ class Runner {
         // 7. apply all changes to the new branch
         this.logger.debug("Cherry picking commits..");
         for (const sha of originalPR.commits) {
-            await git.cherryPick(configs.folder, sha);
+            await git.cherryPick(configs.folder, sha, configs.mergeStrategy, configs.mergeStrategyOption);
         }
         const backport = {
             owner: originalPR.targetRepo.owner,

src/service/args/args-parser.ts

@@ -39,6 +39,8 @@ export default abstract class ArgsParser {
       labels: this.getOrDefault(args.labels, []),
       inheritLabels: this.getOrDefault(args.inheritLabels, false),
       squash: this.getOrDefault(args.squash, true),
+      strategy: this.getOrDefault(args.strategy),
+      strategyOption: this.getOrDefault(args.strategyOption),
     };
   }
 }

src/service/args/args.types.ts

@@ -19,4 +19,6 @@ export interface Args {
   labels?: string[], // backport pr labels
   inheritLabels?: boolean, // if true inherit labels from original pr
   squash?: boolean, // if false use squashed/merged commit otherwise backport all commits as part of the pr
+  strategy?: string, // cherry-pick merge strategy
+  strategyOption?: string, // cherry-pick merge strategy option
 }

src/service/args/cli/cli-args-parser.ts

@@ -27,6 +27,8 @@ export default class CLIArgsParser extends ArgsParser {
       .option("--labels <labels>", "comma separated list of labels to be assigned to the backported pull request", getAsCommaSeparatedList)
       .option("--inherit-labels", "if true the backported pull request will inherit labels from the original one")
       .option("--no-squash", "if provided the tool will backport all commits as part of the pull request")
+      .option("--strategy <strategy>", "cherry-pick merge strategy, default to 'recursive'", undefined)
+      .option("--strategy-option <strategy-option>", "cherry-pick merge strategy option, default to 'theirs'")
       .option("-cf, --config-file <config-file>", "configuration file containing all valid options, the json must match Args interface");
   }
 
@@ -58,6 +60,8 @@ export default class CLIArgsParser extends ArgsParser {
         labels: opts.labels,
         inheritLabels: opts.inheritLabels,
         squash: opts.squash,
+        strategy: opts.strategy,
+        strategyOption: opts.strategyOption,
       };
     }
 

src/service/args/gha/gha-args-parser.ts

@@ -30,6 +30,8 @@ export default class GHAArgsParser extends ArgsParser {
         labels: getAsCommaSeparatedList(getInput("labels")),
         inheritLabels: getAsBooleanOrDefault(getInput("inherit-labels")),
         squash: !getAsBooleanOrDefault(getInput("no-squash")),
+        strategy: getOrUndefined(getInput("strategy")),
+        strategyOption: getOrUndefined(getInput("strategy-option")),
       };
     }
 

src/service/configs/configs.types.ts

@@ -16,6 +16,8 @@ export interface Configs {
   git: LocalGit,
   folder: string,
   targetBranch: string,
+  mergeStrategy?: string, // cherry-pick merge strategy
+  mergeStrategyOption?: string, // cherry-pick merge strategy option
   originalPullRequest: GitPullRequest,
   backportPullRequest: GitPullRequest,
  }

src/service/configs/pullrequest/pr-configs-parser.ts

@@ -30,6 +30,8 @@ export default class PullRequestConfigsParser extends ConfigsParser {
       auth: args.auth,
       folder: `${folder.startsWith("/") ? "" : process.cwd() + "/"}${args.folder ?? this.getDefaultFolder()}`,
       targetBranch: args.targetBranch,
+      mergeStrategy: args.strategy,
+      mergeStrategyOption: args.strategyOption,
       originalPullRequest: pr,
       backportPullRequest: this.getDefaultBackportPullRequest(pr, args),
       git: {

src/service/git/git-cli.ts

@@ -105,9 +105,20 @@ export default class GitCLIService {
    * @param cwd repository in which the sha should be cherry picked to
    * @param sha commit sha
    */
-  async cherryPick(cwd: string, sha: string): Promise<void> {
+  async cherryPick(cwd: string, sha: string, strategy = "recursive", strategyOption = "theirs"): Promise<void> {
     this.logger.info(`Cherry picking ${sha}.`);
-    await this.git(cwd).raw(["cherry-pick", "-m", "1", "--strategy=recursive", "--strategy-option=theirs", sha]);
+
+    const options = ["cherry-pick", "-m", "1", `--strategy=${strategy}`, `--strategy-option=${strategyOption}`, sha];
+    try {
+      await this.git(cwd).raw(options);
+    } catch(error) {
+      const diff = await this.git(cwd).diff();
+      if (diff) {
+        throw new Error(`${error}\r\nShowing git diff:\r\n` + diff);
+      }
+
+      throw error;
+    }
   }
 
   /**

src/service/runner/runner.ts

@@ -99,7 +99,7 @@ export default class Runner {
     // 7. apply all changes to the new branch
     this.logger.debug("Cherry picking commits..");
     for (const sha of originalPR.commits!) {
-      await git.cherryPick(configs.folder, sha);
+      await git.cherryPick(configs.folder, sha, configs.mergeStrategy, configs.mergeStrategyOption);
     }
 
     const backport: BackportPullRequest = {

test/service/args/cli/cli-args-parser.test.ts

@@ -77,6 +77,8 @@ describe("cli args parser", () => {
     expect(args.labels).toEqual([]);
     expect(args.inheritLabels).toEqual(false);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("with config file [default, short]", () => {
@@ -103,6 +105,8 @@ describe("cli args parser", () => {
     expect(args.labels).toEqual([]);
     expect(args.inheritLabels).toEqual(false);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("valid execution [default, long]", () => {
@@ -131,6 +135,8 @@ describe("cli args parser", () => {
     expect(args.labels).toEqual([]);
     expect(args.inheritLabels).toEqual(false);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("with config file [default, long]", () => {
@@ -157,6 +163,8 @@ describe("cli args parser", () => {
     expect(args.labels).toEqual([]);
     expect(args.inheritLabels).toEqual(false);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("valid execution [override, short]", () => {
@@ -192,6 +200,8 @@ describe("cli args parser", () => {
     expect(args.labels).toEqual([]);
     expect(args.inheritLabels).toEqual(false);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("valid execution [override, long]", () => {
@@ -243,6 +253,8 @@ describe("cli args parser", () => {
     expectArrayEqual(args.labels!, ["cherry-pick :cherries:", "another spaced label"]);
     expect(args.inheritLabels).toEqual(true);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("override using config file", () => {
@@ -269,6 +281,8 @@ describe("cli args parser", () => {
     expectArrayEqual(args.labels!, ["cherry-pick :cherries:"]);
     expect(args.inheritLabels).toEqual(true);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("ignore custom option when config file is set", () => {
@@ -322,6 +336,8 @@ describe("cli args parser", () => {
     expectArrayEqual(args.labels!, ["cherry-pick :cherries:"]);
     expect(args.inheritLabels).toEqual(true);
     expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual(undefined);
+    expect(args.strategyOption).toEqual(undefined);
   });
 
   test("override squash to false", () => {
@@ -352,4 +368,38 @@ describe("cli args parser", () => {
     expect(args.inheritLabels).toEqual(false);
     expect(args.squash).toEqual(false);
   });
+
+  test("override cherry pick strategies", () => {
+    addProcessArgs([
+      "--target-branch",
+      "target",
+      "--pull-request",
+      "https://localhost/whatever/pulls/1",
+      "--strategy",
+      "ort",
+      "--strategy-option",
+      "ours",
+    ]);
+
+    const args: Args = parser.parse();
+    expect(args.dryRun).toEqual(false);
+    expect(args.auth).toEqual(undefined);
+    expect(args.gitUser).toEqual(undefined);
+    expect(args.gitEmail).toEqual(undefined);
+    expect(args.folder).toEqual(undefined);
+    expect(args.targetBranch).toEqual("target");
+    expect(args.pullRequest).toEqual("https://localhost/whatever/pulls/1");
+    expect(args.title).toEqual(undefined);
+    expect(args.body).toEqual(undefined);
+    expect(args.bodyPrefix).toEqual(undefined);
+    expect(args.bpBranchName).toEqual(undefined);
+    expect(args.reviewers).toEqual([]);
+    expect(args.assignees).toEqual([]);
+    expect(args.inheritReviewers).toEqual(true);
+    expect(args.labels).toEqual([]);
+    expect(args.inheritLabels).toEqual(false);
+    expect(args.squash).toEqual(true);
+    expect(args.strategy).toEqual("ort");
+    expect(args.strategyOption).toEqual("ours");
+  });
 });