Add mainline option (#179)

README.md

@@ -106,6 +106,7 @@ The above commands will start an interactive prompt. You can use the `arrow keys
 | --github-api-base-url-v3 | Base url for Github's Rest (v3) API                    | https://api.github.com         | string  |
 | --github-api-base-url-v4 | Base url for Github's GraphQL (v4) API                 | https://api.github.com/graphql | string  |
 | --labels                 | Pull request labels                                    |                                | string  |
+| --mainline               | Parent id of merge commit                              | 1 (when no parent id given)    | number  |
 | --multiple               | Select multiple commits/branches                       | false                          | boolean |
 | --path                   | Only list commits touching files under a specific path |                                | string  |
 | --pr-description         | Pull request description suffix                        |                                | string  |

docs/configuration.md

@@ -117,6 +117,15 @@ Config:
 }
 ```
 
+#### `mainline`
+
+When backporting a merge commit the parent id must be specified. This is directly passed to `git cherry-pick` and additional details can be read on the [Git Docs](https://git-scm.com/docs/git-cherry-pick#Documentation/git-cherry-pick.txt---mainlineparent-number)
+
+**Examples:**
+
+- Defaults to 1 when no parent id is given: `backport --mainline`
+- Specifying parent id: `backport --mainline 2`
+
 #### `multipleCommits`
 
 `true`: you will be able to select multiple commits to backport. You will use `<space>` to select, and `<enter>` to confirm you selection.

package.json

@@ -14,7 +14,7 @@
     "branches",
     "branching"
   ],
-  "version": "5.1.5",
+  "version": "5.1.6-beta.1",
   "main": "./dist/index.js",
   "bin": {
     "backport": "./dist/index.js"

src/options/cliArgs.ts

@@ -75,6 +75,25 @@ export function getOptionsFromCliArgs(
       description: 'Pull request labels for the resulting backport PRs',
       type: 'array',
     })
+    .option('mainline', {
+      description:
+        'Parent id of merge commit. Defaults to 1 when supplied without arguments',
+      type: 'number',
+      coerce: (mainline) => {
+        // `--mainline` (default to 1 when no parent is given)
+        if (mainline === undefined) {
+          return 1;
+        }
+
+        // use specified mainline parent
+        if (Number.isInteger(mainline)) {
+          return mainline as number;
+        }
+
+        // Invalid value provided
+        throw new Error(`--mainline must be an integer. Received: ${mainline}`);
+      },
+    })
     .option('multiple', {
       default: configOptions.multiple,
       description: 'Select multiple branches/commits',

src/options/options.test.ts

@@ -115,6 +115,7 @@ describe('validateRequiredOptions', () => {
     fork: true,
     gitHostname: 'github.com',
     labels: [],
+    mainline: undefined,
     multiple: false,
     multipleBranches: true,
     multipleCommits: false,

src/runWithOptions.test.ts

@@ -39,6 +39,7 @@ describe('runWithOptions', () => {
       fork: true,
       gitHostname: 'github.com',
       labels: [],
+      mainline: undefined,
       multiple: false,
       multipleBranches: false,
       multipleCommits: false,

src/services/child-process-promisified.ts

@@ -3,17 +3,16 @@ import { promisify } from 'util';
 import { logger } from './logger';
 
 export async function exec(cmd: string, options: child_process.ExecOptions) {
-  logger.info(`exec: \`${cmd}\``);
   const execPromisified = promisify(child_process.exec);
   try {
     const res = await execPromisified(cmd, {
       maxBuffer: 100 * 1024 * 1024,
       ...options,
     });
-    logger.verbose(`exec response (success):`, res);
+    logger.verbose(`exec success '${cmd}':`, res);
     return res;
   } catch (e) {
-    logger.info(`exec response (error):`, e);
+    logger.info(`exec error '${cmd}':`, e);
     throw e;
   }
 }

src/services/git.test.ts

@@ -9,6 +9,11 @@ import {
   cherrypick,
   getFilesWithConflicts,
 } from '../services/git';
+import { ExecError } from '../test/ExecError';
+
+beforeEach(() => {
+  jest.clearAllMocks();
+});
 
 describe('getUnmergedFiles', () => {
   it('should split lines and remove empty', async () => {
@@ -54,7 +59,7 @@ describe('getFilesWithConflicts', () => {
         'conflicting-file.txt:1: leftover conflict marker\nconflicting-file.txt:3: leftover conflict marker\nconflicting-file.txt:5: leftover conflict marker\n',
       stderr: '',
     };
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
 
     const options = {
       repoOwner: 'elastic',
@@ -88,7 +93,7 @@ describe('createFeatureBranch', () => {
       stderr: "fatal: couldn't find remote ref 4.x\n",
     };
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     await expect(
       createFeatureBranch(options, baseBranch, featureBranch)
     ).rejects.toThrowErrorMatchingInlineSnapshot(
@@ -99,7 +104,7 @@ describe('createFeatureBranch', () => {
   it('should rethrow normal error', async () => {
     expect.assertions(1);
     const err = new Error('just a normal error');
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     expect.assertions(1);
 
     await expect(
@@ -125,13 +130,13 @@ describe('deleteRemote', () => {
       stderr: "fatal: No such remote: 'origin'\n",
     };
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     await expect(await deleteRemote(options, remoteName)).toBe(undefined);
   });
 
   it('should rethrow normal error', async () => {
     const err = new Error('just a normal error');
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     expect.assertions(1);
 
     await expect(
@@ -152,29 +157,120 @@ describe('cherrypick', () => {
     sha: 'abcd',
   };
 
-  it('should swallow cherrypick error', async () => {
+  it('should return `needsResolving: false` when no errors are encountered', async () => {
     jest
       .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
       .mockResolvedValueOnce({ stderr: '', stdout: '' });
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce({
-      killed: false,
-      code: 128,
-      signal: null,
-      cmd: 'git cherry-pick abcd',
-      stdout: '',
-      stderr: '',
+    expect(await cherrypick(options, commit)).toEqual({
+      needsResolving: false,
     });
-    await expect(await cherrypick(options, commit)).toBe(false);
   });
 
-  it('should re-throw other errors', async () => {
-    const err = new Error('non-cherrypick error');
-    jest
+  it('should use mainline option when specified', async () => {
+    const execSpy = jest
       .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
       .mockResolvedValueOnce({ stderr: '', stdout: '' });
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
+    await cherrypick({ ...options, mainline: 1 }, commit);
+
+    expect(execSpy.mock.calls[1][0]).toBe('git cherry-pick --mainline 1 abcd');
+  });
+
+  it('should return `needsResolving: true` upon cherrypick error', async () => {
+    jest
+      .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
+      .mockRejectedValueOnce(
+        new ExecError({
+          killed: false,
+          code: 128,
+          cmd: 'git cherry-pick abcd',
+          stdout: '',
+          stderr: '',
+        })
+      )
+
+      // mock getFilesWithConflicts
+      .mockRejectedValueOnce(
+        new ExecError({
+          code: 2,
+          cmd: 'git --no-pager diff --check',
+          stdout:
+            'conflicting-file.txt:1: leftover conflict marker\nconflicting-file.txt:3: leftover conflict marker\nconflicting-file.txt:5: leftover conflict marker\n',
+          stderr: '',
+        })
+      )
+
+      // mock getUnmergedFiles
+      .mockResolvedValueOnce({ stdout: '', stderr: '' });
+
+    expect(await cherrypick(options, commit)).toEqual({
+      needsResolving: true,
+    });
+  });
+
+  it('it should let the user know about the "--mainline" argument when cherry-picking a merge commit without specifying it', async () => {
+    jest
+      .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
+      .mockRejectedValueOnce(
+        new ExecError({
+          killed: false,
+          code: 128,
+          signal: null,
+          cmd: 'git cherry-pick 381c7b604110257437a289b1f1742685eb8d79c5',
+          stdout: '',
+          stderr:
+            'error: commit 381c7b604110257437a289b1f1742685eb8d79c5 is a merge but no -m option was given.\nfatal: cherry-pick failed\n',
+        })
+      );
+
+    await expect(cherrypick(options, commit)).rejects
+      .toThrowError(`Failed to cherrypick because the selected commit was a merge. Please try again by specifying the parent with the \`mainline\` argument:
+
+> backport --mainline
+
+or:
+
+> backport --mainline <parent-number>
+
+Or refer to the git documentation for more information: https://git-scm.com/docs/git-cherry-pick#Documentation/git-cherry-pick.txt---mainlineparent-number`);
+  });
+
+  it('should re-throw non-cherrypick errors', async () => {
+    jest
+      .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
+      .mockRejectedValueOnce(new Error('non-cherrypick error'))
+
+      // getFilesWithConflicts
+      .mockResolvedValueOnce({ stdout: '', stderr: '' })
+
+      // getUnmergedFiles
+      .mockResolvedValueOnce({ stdout: '', stderr: '' });
 
     await expect(
       cherrypick(options, commit)
@@ -199,13 +295,13 @@ describe('cherrypickContinue', () => {
         'error: no cherry-pick or revert in progress\nfatal: cherry-pick failed\n',
     };
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     await expect(await cherrypickContinue(options)).toBe(undefined);
   });
 
   it('should re-throw other errors', async () => {
     const err = new Error('non-cherrypick error');
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     expect.assertions(1);
 
     await expect(

src/services/git.ts

@@ -1,5 +1,6 @@
 import { resolve as pathResolve } from 'path';
 import del from 'del';
+import isEmpty from 'lodash.isempty';
 import uniq from 'lodash.uniq';
 import { BackportOptions } from '../options/options';
 import { CommitSelected } from '../types/Commit';
@@ -105,19 +106,30 @@ export async function cherrypick(
     `git fetch ${options.repoOwner} ${commit.branch}:${commit.branch} --force`,
     { cwd: getRepoPath(options) }
   );
+  const mainline =
+    options.mainline != undefined ? ` --mainline ${options.mainline}` : '';
 
-  const cmd = `git cherry-pick ${commit.sha}`;
+  const cmd = `git cherry-pick${mainline} ${commit.sha}`;
   try {
     await exec(cmd, { cwd: getRepoPath(options) });
-    return true;
+    return { needsResolving: false };
   } catch (e) {
-    // re-throw unknown errors
-    if (e.cmd !== cmd) {
-      throw e;
+    if (e.message.includes('is a merge but no -m option was given')) {
+      throw new HandledError(
+        'Failed to cherrypick because the selected commit was a merge. Please try again by specifying the parent with the `mainline` argument:\n\n> backport --mainline\n\nor:\n\n> backport --mainline <parent-number>\n\nOr refer to the git documentation for more information: https://git-scm.com/docs/git-cherry-pick#Documentation/git-cherry-pick.txt---mainlineparent-number'
+      );
     }
 
-    // handle cherrypick-related error
-    return false;
+    const isCherryPickError = e.cmd === cmd;
+    const hasConflicts = !isEmpty(await getFilesWithConflicts(options));
+    const hasUnmergedFiles = !isEmpty(await getUnmergedFiles(options));
+
+    if (isCherryPickError && (hasConflicts || hasUnmergedFiles)) {
+      return { needsResolving: true };
+    }
+
+    // re-throw error if there are no conflicts to solve
+    throw e;
   }
 }
 

src/test/ExecError.ts

@@ -3,18 +3,15 @@ export class ExecError extends Error {
   cmd: string | undefined;
   stdout: string | undefined;
   stderr: string | undefined;
-  constructor(
-    public message: string,
-    error: {
-      cmd?: string;
-      killed?: boolean;
-      code?: number;
-      signal?: NodeJS.Signals;
-      stdout?: string;
-      stderr?: string;
-    }
-  ) {
-    super(message);
+  constructor(error: {
+    cmd?: string;
+    killed?: boolean;
+    code?: number;
+    signal?: NodeJS.Signals | null;
+    stdout?: string;
+    stderr?: string;
+  }) {
+    super(error.stderr);
     this.name = 'ExecError';
     this.code = error.code;
     this.cmd = error.cmd;