Sustainable Kibana Architecture: Relocate script v5 (#204461)

## Summary

* Automatically cherry-pick manual commits from the PR branch.
* Remove duplicate cleanup logic when `--pr <number>` is provided.
* Add a pre-relocate hook to allow for custom initial commits.
* Perform PR checkup before cleaning the local repo.
* Use `cli-table3` library to print nice tables with the summary of
modules being moved.

packages/kbn-relocate/relocate.ts

@@ -15,7 +15,7 @@ import { orderBy } from 'lodash';
 import type { ToolingLog } from '@kbn/tooling-log';
 import { getPackages } from '@kbn/repo-packages';
 import { REPO_ROOT } from '@kbn/repo-info';
-import type { Package } from './types';
+import type { Package, PullRequest } from './types';
 import { DESCRIPTION, EXCLUDED_MODULES, KIBANA_FOLDER, NEW_BRANCH } from './constants';
 import {
   belongsTo,
@@ -26,7 +26,15 @@ import {
 } from './utils/relocate';
 import { safeExec } from './utils/exec';
 import { relocatePlan, relocateSummary } from './utils/logging';
-import { checkoutBranch, checkoutResetPr, findGithubLogin, findRemoteName } from './utils/git';
+import {
+  checkoutBranch,
+  checkoutResetPr,
+  cherryPickManualCommits,
+  findGithubLogin,
+  findPr,
+  findRemoteName,
+  getManualCommits,
+} from './utils/git';
 
 const moveModule = async (module: Package, log: ToolingLog) => {
   const destination = calculateModuleTargetFolder(module);
@@ -128,6 +136,9 @@ export const findAndMoveModule = async (moduleId: string, log: ToolingLog) => {
 };
 
 export const findAndRelocateModules = async (params: RelocateModulesParams, log: ToolingLog) => {
+  const { prNumber, baseBranch, ...findParams } = params;
+  let pr: PullRequest | undefined;
+
   const upstream = await findRemoteName('elastic/kibana');
   if (!upstream) {
     log.error(
@@ -142,8 +153,6 @@ export const findAndRelocateModules = async (params: RelocateModulesParams, log:
     return;
   }
 
-  const { prNumber, baseBranch, ...findParams } = params;
-
   const toMove = findModules(findParams, log);
   if (!toMove.length) {
     log.info(
@@ -153,40 +162,60 @@ export const findAndRelocateModules = async (params: RelocateModulesParams, log:
   }
 
   relocatePlan(toMove, log);
-  const res1 = await inquirer.prompt({
+
+  const resConfirmPlan = await inquirer.prompt({
     type: 'confirm',
     name: 'confirmPlan',
     message: `The script will RESET CHANGES in this repository, relocate the modules above and update references. Proceed?`,
   });
 
-  if (!res1.confirmPlan) {
+  if (!resConfirmPlan.confirmPlan) {
     log.info('Aborting');
     return;
   }
 
+  if (prNumber) {
+    pr = await findPr(prNumber);
+
+    if (getManualCommits(pr.commits).length > 0) {
+      const resOverride = await inquirer.prompt({
+        type: 'confirm',
+        name: 'overrideManualCommits',
+        message: 'Detected manual commits in the PR, do you want to override them?',
+      });
+      if (!resOverride.overrideManualCommits) {
+        return;
+      }
+    }
+  }
+
   // start with a clean repo
   await safeExec(`git restore --staged .`);
   await safeExec(`git restore .`);
   await safeExec(`git clean -f -d`);
   await safeExec(`git checkout ${baseBranch} && git pull ${upstream} ${baseBranch}`);
 
-  if (prNumber) {
+  if (pr) {
     // checkout existing PR, reset all commits, rebase from baseBranch
     try {
-      if (!(await checkoutResetPr(baseBranch, prNumber))) {
+      await checkoutResetPr(pr, baseBranch);
-        log.info('Aborting');
-        return;
-      }
     } catch (error) {
       log.error(`Error checking out / resetting PR #${prNumber}:`);
       log.error(error);
       return;
     }
   } else {
-    // checkout [new] branch
+    // checkout new branch
     await checkoutBranch(NEW_BRANCH);
   }
 
+  // push changes in the branch
+  await inquirer.prompt({
+    type: 'confirm',
+    name: 'readyRelocate',
+    message: `Ready to relocate! You can commit changes previous to the relocation at this point. Confirm to proceed with the relocation`,
+  });
+
   // relocate modules
   await safeExec(`yarn kbn bootstrap`);
   const movedCount = await relocateModules(toMove, log);
@@ -197,10 +226,15 @@ export const findAndRelocateModules = async (params: RelocateModulesParams, log:
     );
     return;
   }
+
   relocateSummary(log);
 
+  if (pr) {
+    await cherryPickManualCommits(pr, log);
+  }
+
   // push changes in the branch
-  const res2 = await inquirer.prompt({
+  const resPushBranch = await inquirer.prompt({
     type: 'confirm',
     name: 'pushBranch',
     message: `Relocation finished! You can commit extra changes at this point. Confirm to proceed pushing the current branch`,
@@ -210,7 +244,7 @@ export const findAndRelocateModules = async (params: RelocateModulesParams, log:
     ? `git push --force-with-lease`
     : `git push --set-upstream ${origin} ${NEW_BRANCH}`;
 
-  if (!res2.pushBranch) {
+  if (!resPushBranch.pushBranch) {
     log.info(`Remember to push changes with "${pushCmd}"`);
     return;
   }

packages/kbn-relocate/types.ts

@@ -14,6 +14,7 @@ export interface CommitAuthor {
 }
 
 export interface Commit {
+  oid: string;
   messageHeadline: string;
   authors: CommitAuthor[];
 }

packages/kbn-relocate/utils/git.ts

@@ -8,17 +8,24 @@
  */
 
 import inquirer from 'inquirer';
+import type { ToolingLog } from '@kbn/tooling-log';
 import type { Commit, PullRequest } from '../types';
 import { safeExec } from './exec';
 
 export const findRemoteName = async (repo: string) => {
-  const res = await safeExec('git remote -v');
+  const res = await safeExec('git remote -v', true, false);
-  const remotes = res.stdout.split('\n').map((line) => line.split(/\t| /).filter(Boolean));
+  const remotes = res.stdout
-  return remotes.find(([_, url]) => url.includes(`github.com/${repo}`))?.[0];
+    .trim()
+    .split('\n')
+    .map((line) => line.split(/\t| /).filter(Boolean))
+    .filter((chunks) => chunks.length >= 2);
+  return remotes.find(
+    ([, url]) => url.includes(`github.com/${repo}`) || url.includes(`github.com:${repo}`)
+  )?.[0];
 };
 
 export const findGithubLogin = async () => {
-  const res = await safeExec('gh auth status');
+  const res = await safeExec('gh auth status', true, false);
   // e.g. ✓ Logged in to github.com account gsoldevila (/Users/gsoldevila/.config/gh/hosts.yml)
   const loginLine = res.stdout
     .split('\n')
@@ -34,17 +41,16 @@ export const findPr = async (number: string): Promise<PullRequest> => {
   return { ...JSON.parse(res.stdout), number };
 };
 
-export function hasManualCommits(commits: Commit[]) {
+export const isManualCommit = (commit: Commit) =>
-  const manualCommits = commits.filter(
+  !commit.messageHeadline.startsWith('Relocating module ') &&
-    (commit) =>
+  !commit.messageHeadline.startsWith('Moving modules owned by ') &&
-      !commit.messageHeadline.startsWith('Relocating module ') &&
+  !commit.messageHeadline.startsWith('Merge branch ') &&
-      !commit.messageHeadline.startsWith('Moving modules owned by ') &&
+  commit.authors.some(
-      commit.authors.some(
+    (author) => author.login !== 'kibanamachine' && author.login !== 'elasticmachine'
-        (author) => author.login !== 'kibanamachine' && author.login !== 'elasticmachine'
-      )
   );
 
-  return manualCommits.length > 0;
+export function getManualCommits(commits: Commit[]) {
+  return commits.filter(isManualCommit);
 }
 
 export async function getLastCommitMessage() {
@@ -87,33 +93,14 @@ async function deleteBranches(...branchNames: string[]) {
   );
 }
 
-export const checkoutResetPr = async (baseBranch: string, prNumber: string): Promise<boolean> => {
+export const checkoutResetPr = async (pr: PullRequest, baseBranch: string) => {
-  const pr = await findPr(prNumber);
-
-  if (hasManualCommits(pr.commits)) {
-    const res = await inquirer.prompt({
-      type: 'confirm',
-      name: 'overrideManualCommits',
-      message: 'Detected manual commits in the PR, do you want to override them?',
-    });
-    if (!res.overrideManualCommits) {
-      return false;
-    }
-  }
-
-  // previous cleanup on current branch
-  await safeExec(`git restore --staged .`);
-  await safeExec(`git restore .`);
-  await safeExec(`git clean -f -d`);
-
   // delete existing branch
   await deleteBranches(pr.headRefName);
 
   // checkout the PR branch
-  await safeExec(`gh pr checkout ${prNumber}`);
+  await safeExec(`gh pr checkout ${pr.number}`);
   await resetAllCommits(pr.commits.length);
   await safeExec(`git rebase ${baseBranch}`);
-  return true;
 };
 
 export const checkoutBranch = async (branch: string) => {
@@ -124,3 +111,71 @@ export const checkoutBranch = async (branch: string) => {
     await safeExec(`git checkout -b ${branch}`);
   }
 };
+
+export const cherryPickManualCommits = async (pr: PullRequest, log: ToolingLog) => {
+  const manualCommits = getManualCommits(pr.commits);
+  if (manualCommits.length) {
+    log.info(`Found manual commits on https://github.com/elastic/kibana/pull/${pr.number}/commits`);
+
+    for (let i = 0; i < manualCommits.length; ++i) {
+      const { oid, messageHeadline, authors } = manualCommits[i];
+      const url = `https://github.com/elastic/kibana/pull/${pr.number}/commits/${oid}`;
+
+      const res = await inquirer.prompt({
+        type: 'list',
+        choices: [
+          { name: 'Yes, attempt to cherry-pick', value: 'yes' },
+          { name: 'No, I will add it manually (press when finished)', value: 'no' },
+        ],
+        name: 'cherryPick',
+        message: `Do you want to cherry pick '${messageHeadline}' (${authors[0].login})?`,
+      });
+
+      if (res.cherryPick === 'yes') {
+        try {
+          await safeExec(`git cherry-pick ${oid}`);
+          log.info(`Commit '${messageHeadline}' (${authors[0].login}) cherry-picked successfully!`);
+        } catch (error) {
+          log.info(`Error trying to cherry-pick: ${url}`);
+          log.error(error.message);
+          const res2 = await inquirer.prompt({
+            type: 'list',
+            choices: [
+              { name: 'Abort this cherry-pick', value: 'abort' },
+              { name: 'Conflicts solved (git cherry-pick --continue)', value: 'continue' },
+              { name: 'I solved the conflicts and commited', value: 'done' },
+            ],
+            name: 'cherryPickFailed',
+            message: `Automatic cherry-pick failed, manual intervention required`,
+          });
+
+          if (res2.cherryPickFailed === 'abort') {
+            try {
+              await safeExec(`git cherry-pick --abort`);
+              log.warning(
+                'Cherry-pick aborted, please review changes in that commit and apply them manually if needed!'
+              );
+            } catch (error2) {
+              log.error(
+                'Cherry-pick --abort failed, please cleanup your working tree before continuing!'
+              );
+            }
+          } else if (res2.cherryPickFailed === 'continue') {
+            try {
+              await safeExec(`git cherry-pick --continue`);
+              log.info(
+                `Commit '${messageHeadline}' (${authors[0].login}) cherry-picked successfully!`
+              );
+            } catch (error2) {
+              await inquirer.prompt({
+                type: 'confirm',
+                name: 'cherryPickContinueFailed',
+                message: `Cherry pick --continue failed, please address conflicts AND COMMIT manually. Hit confirm when ready`,
+              });
+            }
+          }
+        }
+      }
+    }
+  }
+};

packages/kbn-relocate/utils/logging.ts

@@ -10,6 +10,7 @@
 import type { ToolingLog } from '@kbn/tooling-log';
 import { appendFileSync, writeFileSync } from 'fs';
 import dedent from 'dedent';
+import Table from 'cli-table3';
 import type { Package } from '../types';
 import { calculateModuleTargetFolder } from './relocate';
 import {
@@ -21,6 +22,20 @@ import {
   UPDATED_RELATIVE_PATHS,
 } from '../constants';
 
+export const createModuleTable = (entries: string[][]) => {
+  const table = new Table({
+    head: ['Id', 'Target folder'],
+    colAligns: ['left', 'left'],
+    style: {
+      'padding-left': 2,
+      'padding-right': 2,
+    },
+  });
+
+  table.push(...entries);
+  return table;
+};
+
 export const relocatePlan = (modules: Package[], log: ToolingLog) => {
   const plugins = modules.filter((module) => module.manifest.type === 'plugin');
   const packages = modules.filter((module) => module.manifest.type !== 'plugin');
@@ -37,11 +52,8 @@ export const relocatePlan = (modules: Package[], log: ToolingLog) => {
     \n\n`;
 
     appendFileSync(DESCRIPTION, pluginList);
-    log.info(
+    const plgTable = createModuleTable(plugins.map((plg) => [plg.id, target(plg)]));
-      `${plugins.length} plugin(s) are going to be relocated:\n${plugins
+    log.info(`${plugins.length} plugin(s) are going to be relocated:\n${plgTable.toString()}`);
-        .map((plg) => `${plg.id} => ${target(plg)}`)
-        .join('\n')}`
-    );
   }
 
   if (packages.length) {
@@ -53,11 +65,8 @@ export const relocatePlan = (modules: Package[], log: ToolingLog) => {
     \n\n`;
 
     appendFileSync(DESCRIPTION, packageList);
-    log.info(
+    const pkgTable = createModuleTable(packages.map((pkg) => [pkg.id, target(pkg)]));
-      `${packages.length} packages(s) are going to be relocated:\n${packages
+    log.info(`${packages.length} packages(s) are going to be relocated:\n${pkgTable.toString()}`);
-        .map((plg) => `${plg.id} => ${target(plg)}`)
-        .join('\n')}`
-    );
   }
 };