Skip to content

Commit edf5ab6

Browse files
aduh95aduh95
authored
chore: remove duplicated code (#917)
1 parent 7bf2350 commit edf5ab6

File tree

2 files changed

+36
-188
lines changed

2 files changed

+36
-188
lines changed

‎lib/cherry_pick.js‎

Lines changed: 19 additions & 185 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,13 @@
1-
import os from 'node:os';
21
import path from 'node:path';
32
import { getMetadata } from '../components/metadata.js';
43

54
import {
6-
runAsync, runSync, forceRunAsync
5+
runAsync, runSync
76
} from './run.js';
8-
import { writeFile } from './file.js';
9-
import {
10-
shortSha, getEditor
11-
} from './utils.js';
127
import { getNcuDir } from './config.js';
8+
import LandingSession, { LINT_RESULTS } from './landing_session.js';
139

14-
const LINT_RESULTS = {
15-
SKIPPED: 'skipped',
16-
FAILED: 'failed',
17-
SUCCESS: 'success'
18-
};
19-
20-
export default class CheckPick {
10+
export default class CherryPick {
2111
constructor(prid, dir, cli, {
2212
owner,
2313
repo,
@@ -46,11 +36,6 @@ export default class CheckPick {
4636
return this.options.lint;
4737
}
4838

49-
getUpstreamHead() {
50-
const { upstream, branch } = this;
51-
return runSync('git', ['rev-parse', `${upstream}/${branch}`]).trim();
52-
}
53-
5439
getCurrentRev() {
5540
return runSync('git', ['rev-parse', 'HEAD']).trim();
5641
}
@@ -73,16 +58,6 @@ export default class CheckPick {
7358
return path.resolve(this.ncuDir, `${this.prid}`);
7459
}
7560

76-
getMessagePath(rev) {
77-
return path.resolve(this.pullDir, `${shortSha(rev)}.COMMIT_EDITMSG`);
78-
}
79-
80-
saveMessage(rev, message) {
81-
const file = this.getMessagePath(rev);
82-
writeFile(file, message);
83-
return file;
84-
}
85-
8661
async start() {
8762
const { cli } = this;
8863

@@ -91,7 +66,7 @@ export default class CheckPick {
9166
owner: this.owner,
9267
repo: this.repo
9368
}, false, cli);
94-
const expectedCommitShas =
69+
this.expectedCommitShas =
9570
metadata.data.commits.map(({ commit }) => commit.oid);
9671

9772
const amend = await cli.prompt(
@@ -104,7 +79,7 @@ export default class CheckPick {
10479
}
10580

10681
try {
107-
const commitInfo = await this.downloadAndPatch(expectedCommitShas);
82+
const commitInfo = await this.downloadAndPatch();
10883
const cleanLint = await this.validateLint();
10984
if (cleanLint === LINT_RESULTS.FAILED) {
11085
cli.error('Patch still contains lint errors. ' +
@@ -120,68 +95,6 @@ export default class CheckPick {
12095
}
12196
}
12297

123-
async downloadAndPatch(expectedCommitShas) {
124-
const { cli, repo, owner, prid } = this;
125-
126-
cli.startSpinner(`Downloading patch for ${prid}`);
127-
// fetch via ssh to handle private repo
128-
await runAsync('git', [
129-
'fetch', `git@github.com:${owner}/${repo}.git`,
130-
`refs/pull/${prid}/merge`]);
131-
// We fetched the commit that would result if we used `git merge`.
132-
// ^1 and ^2 refer to the PR base and the PR head, respectively.
133-
const [base, head] = await runAsync('git',
134-
['rev-parse', 'FETCH_HEAD^1', 'FETCH_HEAD^2'],
135-
{ captureStdout: 'lines' });
136-
const commitShas = await runAsync('git',
137-
['rev-list', `${base}..${head}`],
138-
{ captureStdout: 'lines' });
139-
cli.stopSpinner(`Fetched commits as ${shortSha(base)}..${shortSha(head)}`);
140-
cli.separator();
141-
142-
const mismatchedCommits = [
143-
...commitShas.filter((sha) => !expectedCommitShas.includes(sha))
144-
.map((sha) => `Unexpected commit ${sha}`),
145-
...expectedCommitShas.filter((sha) => !commitShas.includes(sha))
146-
.map((sha) => `Missing commit ${sha}`)
147-
].join('\n');
148-
if (mismatchedCommits.length > 0) {
149-
throw new Error(`Mismatched commits:\n${mismatchedCommits}`);
150-
}
151-
152-
const commitInfo = { base, head, shas: commitShas };
153-
154-
try {
155-
await forceRunAsync('git', ['cherry-pick', `${base}..${head}`], {
156-
ignoreFailure: false
157-
});
158-
} catch (ex) {
159-
await forceRunAsync('git', ['cherry-pick', '--abort']);
160-
throw new Error('Failed to apply patches');
161-
}
162-
163-
cli.ok('Patches applied');
164-
return commitInfo;
165-
}
166-
167-
async validateLint() {
168-
// The linter is currently only run on non-Windows platforms.
169-
if (os.platform() === 'win32') {
170-
return LINT_RESULTS.SKIPPED;
171-
}
172-
173-
if (!this.lint) {
174-
return LINT_RESULTS.SKIPPED;
175-
}
176-
177-
try {
178-
await runAsync('make', ['lint']);
179-
return LINT_RESULTS.SUCCESS;
180-
} catch {
181-
return LINT_RESULTS.FAILED;
182-
}
183-
}
184-
18598
async amend(metadata, commitInfo) {
18699
const { cli } = this;
187100
const subjects = await runAsync('git',
@@ -203,102 +116,23 @@ export default class CheckPick {
203116
await runAsync('git', ['commit', '--amend', '--no-edit']);
204117
}
205118

206-
return this._amend(metadata);
119+
return LandingSession.prototype.amend.call(this, metadata);
207120
}
208121

209-
async _amend(metadataStr) {
210-
const { cli } = this;
211-
212-
const rev = this.getCurrentRev();
213-
const original = runSync('git', [
214-
'show', 'HEAD', '-s', '--format=%B'
215-
]).trim();
216-
// git has very specific rules about what is a trailer and what is not.
217-
// Instead of trying to implement those ourselves, let git parse the
218-
// original commit message and see if it outputs any trailers.
219-
const originalHasTrailers = runSync('git', [
220-
'interpret-trailers', '--parse', '--no-divider'
221-
], {
222-
input: `${original}\n`
223-
}).trim().length !== 0;
224-
const metadata = metadataStr.trim().split('\n');
225-
const amended = original.split('\n');
226-
227-
// If the original commit message already contains trailers (such as
228-
// "Co-authored-by"), we simply add our own metadata after those. Otherwise,
229-
// we have to add an empty line so that git recognizes our own metadata as
230-
// trailers in the amended commit message.
231-
if (!originalHasTrailers) {
232-
amended.push('');
233-
}
234-
235-
const BACKPORT_RE = /BACKPORT-PR-URL\s*:\s*(\S+)/i;
236-
const PR_RE = /PR-URL\s*:\s*(\S+)/i;
237-
const REVIEW_RE = /Reviewed-By\s*:\s*(\S+)/i;
238-
const CVE_RE = /CVE-ID\s*:\s*(\S+)/i;
239-
240-
let containCVETrailer = false;
241-
for (const line of metadata) {
242-
if (line.length !== 0 && original.includes(line)) {
243-
if (line.match(CVE_RE)) {
244-
containCVETrailer = true;
245-
}
246-
if (originalHasTrailers) {
247-
cli.warn(`Found ${line}, skipping..`);
248-
} else {
249-
throw new Error(
250-
'Git found no trailers in the original commit message, ' +
251-
`but '${line}' is present and should be a trailer.`);
252-
}
253-
} else {
254-
if (line.match(BACKPORT_RE)) {
255-
let prIndex = amended.findIndex(datum => datum.match(PR_RE));
256-
if (prIndex === -1) {
257-
prIndex = amended.findIndex(datum => datum.match(REVIEW_RE)) - 1;
258-
}
259-
amended.splice(prIndex + 1, 0, line);
260-
} else {
261-
amended.push(line);
262-
}
263-
}
264-
}
265-
266-
if (!containCVETrailer && this.includeCVE) {
267-
const cveID = await cli.prompt(
268-
'Git found no CVE-ID trailer in the original commit message. ' +
269-
'Please, provide the CVE-ID',
270-
{ questionType: 'input', defaultAnswer: 'CVE-2023-XXXXX' }
271-
);
272-
amended.push('CVE-ID: ' + cveID);
273-
}
122+
readyToAmend() {
123+
return true;
124+
}
274125

275-
const message = amended.join('\n');
276-
const messageFile = this.saveMessage(rev, message);
277-
cli.separator('New Message');
278-
cli.log(message.trim());
279-
const takeMessage = await cli.prompt('Use this message?');
280-
if (takeMessage) {
281-
await runAsync('git', ['commit', '--amend', '-F', messageFile]);
282-
return true;
283-
}
126+
startAmending() {
127+
// No-op
128+
}
284129

285-
const editor = await getEditor({ git: true });
286-
if (editor) {
287-
try {
288-
await forceRunAsync(
289-
editor,
290-
[`"${messageFile}"`],
291-
{ ignoreFailure: false, spawnArgs: { shell: true } }
292-
);
293-
await runAsync('git', ['commit', '--amend', '-F', messageFile]);
294-
return true;
295-
} catch {
296-
cli.warn(`Please manually edit ${messageFile}, then run\n` +
297-
`\`git commit --amend -F ${messageFile}\` ` +
298-
'to finish amending the message');
299-
throw new Error(
300-
'Failed to edit the message using the configured editor');
301-
}
302-
}
130+
saveCommitInfo() {
131+
// No-op
303132
}
304133
}
134+
135+
CherryPick.prototype.downloadAndPatch = LandingSession.prototype.downloadAndPatch;
136+
CherryPick.prototype.validateLint = LandingSession.prototype.validateLint;
137+
CherryPick.prototype.getMessagePath = LandingSession.prototype.getMessagePath;
138+
CherryPick.prototype.saveMessage = LandingSession.prototype.saveMessage;

‎lib/landing_session.js‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010

1111
const isWindows = process.platform === 'win32';
1212

13-
const LINT_RESULTS = {
13+
export const LINT_RESULTS = {
1414
SKIPPED: 'skipped',
1515
FAILED: 'failed',
1616
SUCCESS: 'success'
@@ -84,11 +84,11 @@ export default class LandingSession extends Session {
8484
}
8585

8686
async downloadAndPatch() {
87-
const { cli, repo, owner, prid, expectedCommitShas } = this;
87+
const { cli, upstream, prid, expectedCommitShas } = this;
8888

8989
cli.startSpinner(`Downloading patch for ${prid}`);
9090
await runAsync('git', [
91-
'fetch', `https://github.com/${owner}/${repo}.git`,
91+
'fetch', upstream,
9292
`refs/pull/${prid}/merge`]);
9393
// We fetched the commit that would result if we used `git merge`.
9494
// ^1 and ^2 refer to the PR base and the PR head, respectively.
@@ -315,9 +315,14 @@ export default class LandingSession extends Session {
315315
const BACKPORT_RE = /BACKPORT-PR-URL\s*:\s*(\S+)/i;
316316
const PR_RE = /PR-URL\s*:\s*(\S+)/i;
317317
const REVIEW_RE = /Reviewed-By\s*:\s*(\S+)/i;
318+
const CVE_RE = /CVE-ID\s*:\s*(\S+)/i;
318319

320+
let containCVETrailer = false;
319321
for (const line of metadata) {
320322
if (line.length !== 0 && original.includes(line)) {
323+
if (line.match(CVE_RE)) {
324+
containCVETrailer = true;
325+
}
321326
if (originalHasTrailers) {
322327
cli.warn(`Found ${line}, skipping..`);
323328
} else {
@@ -338,6 +343,15 @@ export default class LandingSession extends Session {
338343
}
339344
}
340345

346+
if (!containCVETrailer && this.includeCVE) {
347+
const cveID = await cli.prompt(
348+
'Git found no CVE-ID trailer in the original commit message. ' +
349+
'Please, provide the CVE-ID',
350+
{ questionType: 'input', defaultAnswer: 'CVE-2023-XXXXX' }
351+
);
352+
amended.push('CVE-ID: ' + cveID);
353+
}
354+
341355
const message = amended.join('\n');
342356
const messageFile = this.saveMessage(rev, message);
343357
cli.separator('New Message');

0 commit comments

Comments
 (0)