Revert "chore: remove duplicated code (#917)"

This reverts commit edf5ab6.

Author: RafaelGSS

lib/cherry_pick.js

@@ -1,13 +1,23 @@
+import os from 'node:os';
 import path from 'node:path';
 import { getMetadata } from '../components/metadata.js';
 
 import {
-  runAsync, runSync
+  runAsync, runSync, forceRunAsync
 } from './run.js';
+import { writeFile } from './file.js';
+import {
+  shortSha, getEditor
+} from './utils.js';
 import { getNcuDir } from './config.js';
-import LandingSession, { LINT_RESULTS } from './landing_session.js';
 
-export default class CherryPick {
+const LINT_RESULTS = {
+  SKIPPED: 'skipped',
+  FAILED: 'failed',
+  SUCCESS: 'success'
+};
+
+export default class CheckPick {
   constructor(prid, dir, cli, {
     owner,
     repo,
@@ -36,6 +46,11 @@ export default class CherryPick {
     return this.options.lint;
   }
 
+  getUpstreamHead() {
+    const { upstream, branch } = this;
+    return runSync('git', ['rev-parse', `${upstream}/${branch}`]).trim();
+  }
+
   getCurrentRev() {
     return runSync('git', ['rev-parse', 'HEAD']).trim();
   }
@@ -58,6 +73,16 @@ export default class CherryPick {
     return path.resolve(this.ncuDir, `${this.prid}`);
   }
 
+  getMessagePath(rev) {
+    return path.resolve(this.pullDir, `${shortSha(rev)}.COMMIT_EDITMSG`);
+  }
+
+  saveMessage(rev, message) {
+    const file = this.getMessagePath(rev);
+    writeFile(file, message);
+    return file;
+  }
+
   async start() {
     const { cli } = this;
 
@@ -66,7 +91,7 @@ export default class CherryPick {
       owner: this.owner,
       repo: this.repo
     }, false, cli);
-    this.expectedCommitShas =
+    const expectedCommitShas =
       metadata.data.commits.map(({ commit }) => commit.oid);
 
     const amend = await cli.prompt(
@@ -79,7 +104,7 @@ export default class CherryPick {
     }
 
     try {
-      const commitInfo = await this.downloadAndPatch();
+      const commitInfo = await this.downloadAndPatch(expectedCommitShas);
       const cleanLint = await this.validateLint();
       if (cleanLint === LINT_RESULTS.FAILED) {
         cli.error('Patch still contains lint errors. ' +
@@ -95,6 +120,68 @@ export default class CherryPick {
     }
   }
 
+  async downloadAndPatch(expectedCommitShas) {
+    const { cli, repo, owner, prid } = this;
+
+    cli.startSpinner(`Downloading patch for ${prid}`);
+    // fetch via ssh to handle private repo
+    await runAsync('git', [
+      'fetch', `git@github.com:${owner}/${repo}.git`,
+      `refs/pull/${prid}/merge`]);
+    // We fetched the commit that would result if we used `git merge`.
+    // ^1 and ^2 refer to the PR base and the PR head, respectively.
+    const [base, head] = await runAsync('git',
+      ['rev-parse', 'FETCH_HEAD^1', 'FETCH_HEAD^2'],
+      { captureStdout: 'lines' });
+    const commitShas = await runAsync('git',
+      ['rev-list', `${base}..${head}`],
+      { captureStdout: 'lines' });
+    cli.stopSpinner(`Fetched commits as ${shortSha(base)}..${shortSha(head)}`);
+    cli.separator();
+
+    const mismatchedCommits = [
+      ...commitShas.filter((sha) => !expectedCommitShas.includes(sha))
+        .map((sha) => `Unexpected commit ${sha}`),
+      ...expectedCommitShas.filter((sha) => !commitShas.includes(sha))
+        .map((sha) => `Missing commit ${sha}`)
+    ].join('\n');
+    if (mismatchedCommits.length > 0) {
+      throw new Error(`Mismatched commits:\n${mismatchedCommits}`);
+    }
+
+    const commitInfo = { base, head, shas: commitShas };
+
+    try {
+      await forceRunAsync('git', ['cherry-pick', `${base}..${head}`], {
+        ignoreFailure: false
+      });
+    } catch (ex) {
+      await forceRunAsync('git', ['cherry-pick', '--abort']);
+      throw new Error('Failed to apply patches');
+    }
+
+    cli.ok('Patches applied');
+    return commitInfo;
+  }
+
+  async validateLint() {
+    // The linter is currently only run on non-Windows platforms.
+    if (os.platform() === 'win32') {
+      return LINT_RESULTS.SKIPPED;
+    }
+
+    if (!this.lint) {
+      return LINT_RESULTS.SKIPPED;
+    }
+
+    try {
+      await runAsync('make', ['lint']);
+      return LINT_RESULTS.SUCCESS;
+    } catch {
+      return LINT_RESULTS.FAILED;
+    }
+  }
+
   async amend(metadata, commitInfo) {
     const { cli } = this;
     const subjects = await runAsync('git',
@@ -116,23 +203,102 @@ export default class CherryPick {
       await runAsync('git', ['commit', '--amend', '--no-edit']);
     }
 
-    return LandingSession.prototype.amend.call(this, metadata);
+    return this._amend(metadata);
   }
 
-  readyToAmend() {
-    return true;
-  }
+  async _amend(metadataStr) {
+    const { cli } = this;
 
-  startAmending() {
-    // No-op
-  }
+    const rev = this.getCurrentRev();
+    const original = runSync('git', [
+      'show', 'HEAD', '-s', '--format=%B'
+    ]).trim();
+    // git has very specific rules about what is a trailer and what is not.
+    // Instead of trying to implement those ourselves, let git parse the
+    // original commit message and see if it outputs any trailers.
+    const originalHasTrailers = runSync('git', [
+      'interpret-trailers', '--parse', '--no-divider'
+    ], {
+      input: `${original}\n`
+    }).trim().length !== 0;
+    const metadata = metadataStr.trim().split('\n');
+    const amended = original.split('\n');
+
+    // If the original commit message already contains trailers (such as
+    // "Co-authored-by"), we simply add our own metadata after those. Otherwise,
+    // we have to add an empty line so that git recognizes our own metadata as
+    // trailers in the amended commit message.
+    if (!originalHasTrailers) {
+      amended.push('');
+    }
+
+    const BACKPORT_RE = /BACKPORT-PR-URL\s*:\s*(\S+)/i;
+    const PR_RE = /PR-URL\s*:\s*(\S+)/i;
+    const REVIEW_RE = /Reviewed-By\s*:\s*(\S+)/i;
+    const CVE_RE = /CVE-ID\s*:\s*(\S+)/i;
 
-  saveCommitInfo() {
-    // No-op
+    let containCVETrailer = false;
+    for (const line of metadata) {
+      if (line.length !== 0 && original.includes(line)) {
+        if (line.match(CVE_RE)) {
+          containCVETrailer = true;
+        }
+        if (originalHasTrailers) {
+          cli.warn(`Found ${line}, skipping..`);
+        } else {
+          throw new Error(
+            'Git found no trailers in the original commit message, ' +
+            `but '${line}' is present and should be a trailer.`);
+        }
+      } else {
+        if (line.match(BACKPORT_RE)) {
+          let prIndex = amended.findIndex(datum => datum.match(PR_RE));
+          if (prIndex === -1) {
+            prIndex = amended.findIndex(datum => datum.match(REVIEW_RE)) - 1;
+          }
+          amended.splice(prIndex + 1, 0, line);
+        } else {
+          amended.push(line);
+        }
+      }
+    }
+
+    if (!containCVETrailer && this.includeCVE) {
+      const cveID = await cli.prompt(
+        'Git found no CVE-ID trailer in the original commit message. ' +
+        'Please, provide the CVE-ID',
+        { questionType: 'input', defaultAnswer: 'CVE-2023-XXXXX' }
+      );
+      amended.push('CVE-ID: ' + cveID);
+    }
+
+    const message = amended.join('\n');
+    const messageFile = this.saveMessage(rev, message);
+    cli.separator('New Message');
+    cli.log(message.trim());
+    const takeMessage = await cli.prompt('Use this message?');
+    if (takeMessage) {
+      await runAsync('git', ['commit', '--amend', '-F', messageFile]);
+      return true;
+    }
+
+    const editor = await getEditor({ git: true });
+    if (editor) {
+      try {
+        await forceRunAsync(
+          editor,
+          [`"${messageFile}"`],
+          { ignoreFailure: false, spawnArgs: { shell: true } }
+        );
+        await runAsync('git', ['commit', '--amend', '-F', messageFile]);
+        return true;
+      } catch {
+        cli.warn(`Please manually edit ${messageFile}, then run\n` +
+          `\`git commit --amend -F ${messageFile}\` ` +
+          'to finish amending the message');
+        throw new Error(
+          'Failed to edit the message using the configured editor');
+      }
+    }
   }
 }
-
-CherryPick.prototype.downloadAndPatch = LandingSession.prototype.downloadAndPatch;
-CherryPick.prototype.validateLint = LandingSession.prototype.validateLint;
-CherryPick.prototype.getMessagePath = LandingSession.prototype.getMessagePath;
-CherryPick.prototype.saveMessage = LandingSession.prototype.saveMessage;

lib/landing_session.js

@@ -10,7 +10,7 @@ import {
 
 const isWindows = process.platform === 'win32';
 
-export const LINT_RESULTS = {
+const LINT_RESULTS = {
   SKIPPED: 'skipped',
   FAILED: 'failed',
   SUCCESS: 'success'
@@ -84,11 +84,11 @@ export default class LandingSession extends Session {
   }
 
   async downloadAndPatch() {
-    const { cli, upstream, prid, expectedCommitShas } = this;
+    const { cli, repo, owner, prid, expectedCommitShas } = this;
 
     cli.startSpinner(`Downloading patch for ${prid}`);
     await runAsync('git', [
-      'fetch', upstream,
+      'fetch', `https://github.com/${owner}/${repo}.git`,
       `refs/pull/${prid}/merge`]);
     // We fetched the commit that would result if we used `git merge`.
     // ^1 and ^2 refer to the PR base and the PR head, respectively.
@@ -315,14 +315,9 @@ export default class LandingSession extends Session {
     const BACKPORT_RE = /BACKPORT-PR-URL\s*:\s*(\S+)/i;
     const PR_RE = /PR-URL\s*:\s*(\S+)/i;
     const REVIEW_RE = /Reviewed-By\s*:\s*(\S+)/i;
-    const CVE_RE = /CVE-ID\s*:\s*(\S+)/i;
 
-    let containCVETrailer = false;
     for (const line of metadata) {
       if (line.length !== 0 && original.includes(line)) {
-        if (line.match(CVE_RE)) {
-          containCVETrailer = true;
-        }
         if (originalHasTrailers) {
           cli.warn(`Found ${line}, skipping..`);
         } else {
@@ -343,15 +338,6 @@ export default class LandingSession extends Session {
       }
     }
 
-    if (!containCVETrailer && this.includeCVE) {
-      const cveID = await cli.prompt(
-        'Git found no CVE-ID trailer in the original commit message. ' +
-        'Please, provide the CVE-ID',
-        { questionType: 'input', defaultAnswer: 'CVE-2023-XXXXX' }
-      );
-      amended.push('CVE-ID: ' + cveID);
-    }
-
     const message = amended.join('\n');
     const messageFile = this.saveMessage(rev, message);
     cli.separator('New Message');