diff --git a/lib/landing_session.js b/lib/landing_session.js index 87311dbd..31be0c1c 100644 --- a/lib/landing_session.js +++ b/lib/landing_session.js @@ -30,6 +30,7 @@ class LandingSession extends Session { this.lint = lint; this.autorebase = autorebase; this.fixupAll = fixupAll; + this.expectedCommitShas = []; } get argv() { @@ -44,6 +45,8 @@ class LandingSession extends Session { async start(metadata) { const { cli } = this; this.startLanding(); + this.expectedCommitShas = + metadata.data.commits.map(({ commit }) => commit.oid); const status = metadata.status ? 'should be ready' : 'is not ready'; // NOTE(mmarchini): default answer is yes. If --yes is given, we need to be // more careful though, and we change the default to the result of our @@ -78,34 +81,46 @@ class LandingSession extends Session { } async downloadAndPatch() { - const { cli, req, repo, owner, prid } = this; + const { cli, repo, owner, prid, expectedCommitShas } = this; - // TODO(joyeecheung): restore previously downloaded patches cli.startSpinner(`Downloading patch for ${prid}`); - const patch = await req.text( - `https://github.com/${owner}/${repo}/pull/${prid}.patch`); - this.savePatch(patch); - cli.stopSpinner(`Downloaded patch to ${this.patchPath}`); + await runAsync('git', [ + '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. + 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(); - // TODO: check that patches downloaded match metadata.commits + + 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) { + cli.error(`Mismatched commits:\n${mismatchedCommits}`); + process.exit(1); + } + + const commitInfo = { base, head, shas: commitShas }; + this.saveCommitInfo(commitInfo); + try { - await forceRunAsync('git', ['am', this.patchPath], { + await forceRunAsync('git', ['cherry-pick', `${base}..${head}`], { ignoreFailure: false }); } catch (ex) { - const should3Way = await cli.prompt( - 'The normal `git am` failed. Do you want to retry with 3-way merge?'); - if (should3Way) { - await forceRunAsync('git', ['am', '--abort']); - await runAsync('git', [ - 'am', - '-3', - this.patchPath - ]); - } else { - cli.error('Failed to apply patches'); - process.exit(1); - } + await forceRunAsync('git', ['cherry-pick', '--abort']); + + cli.error('Failed to apply patches'); + process.exit(1); } // Check for and maybe assign any unmarked deprecations in the codebase. @@ -126,7 +141,7 @@ class LandingSession extends Session { } cli.ok('Patches applied'); - return patch; + return commitInfo; } getRebaseSuggestion(subjects) { @@ -173,21 +188,13 @@ class LandingSession extends Session { } } - async tryCompleteLanding(patch) { + async tryCompleteLanding(commitInfo) { const { cli } = this; + const subjects = await runAsync('git', + ['log', '--pretty=format:%s', `${commitInfo.base}..${commitInfo.head}`], + { captureStdout: 'lines' }); - const subjects = patch.match(/Subject: \[PATCH.*?\].*/g); - if (!subjects) { - cli.warn('Cannot get number of commits in the patch. ' + - 'It seems to be malformed'); - return; - } - - // XXX(joyeecheung) we cannot guarantee that no one will put a subject - // line in the commit message but that seems unlikely (some deps update - // might do that). - if (subjects.length === 1) { - // assert(subjects[0].startsWith('Subject: [PATCH]')) + if (commitInfo.shas.length === 1) { const shouldAmend = await cli.prompt( 'There is only one commit in this PR.\n' + 'do you want to amend the commit message?'); @@ -247,7 +254,7 @@ class LandingSession extends Session { } await this.tryResetBranch(); - const patch = await this.downloadAndPatch(); + const commitInfo = await this.downloadAndPatch(); const cleanLint = await this.validateLint(); if (cleanLint === LINT_RESULTS.FAILED) { @@ -280,7 +287,7 @@ class LandingSession extends Session { this.startAmending(); - await this.tryCompleteLanding(patch); + await this.tryCompleteLanding(commitInfo); } async amend() { @@ -407,13 +414,13 @@ class LandingSession extends Session { } if (this.isApplying()) { // We're still resolving conflicts. - if (this.amInProgress()) { - cli.log('Looks like you are resolving a `git am` conflict'); + if (this.cherryPickInProgress()) { + cli.log('Looks like you are resolving a `git cherry-pick` conflict'); cli.log('Please run `git status` for help'); } else { // Conflicts has been resolved - amend. this.startAmending(); - return this.tryCompleteLanding(this.patch); + return this.tryCompleteLanding(this.commitInfo); } return; } diff --git a/lib/run.js b/lib/run.js index 63ae10d2..12477907 100644 --- a/lib/run.js +++ b/lib/run.js @@ -4,15 +4,24 @@ const { spawn, spawnSync } = require('child_process'); const IGNORE = '__ignore__'; -function runAsyncBase(cmd, args, options = {}) { +function runAsyncBase(cmd, args, { + ignoreFailure = true, + spawnArgs, + captureStdout = false +} = {}) { return new Promise((resolve, reject) => { const child = spawn(cmd, args, Object.assign({ cwd: process.cwd(), - stdio: 'inherit' - }, options.spawnArgs)); + stdio: captureStdout ? ['inherit', 'pipe', 'inherit'] : 'inherit' + }, spawnArgs)); + let stdout; + if (captureStdout) { + stdout = ''; + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (chunk) => { stdout += chunk; }); + } child.on('close', (code) => { if (code !== 0) { - const { ignoreFailure = true } = options; if (ignoreFailure) { return reject(new Error(IGNORE)); } @@ -21,7 +30,11 @@ function runAsyncBase(cmd, args, options = {}) { err.messageOnly = true; return reject(err); } - return resolve(); + if (captureStdout === 'lines') { + stdout = stdout.split(/\r?\n/g); + if (stdout[stdout.length - 1] === '') stdout.pop(); + } + return resolve(stdout); }); }); } diff --git a/lib/session.js b/lib/session.js index 9ba61a5b..ab7fc020 100644 --- a/lib/session.js +++ b/lib/session.js @@ -171,12 +171,12 @@ class Session { return readFile(this.metadataPath); } - get patchPath() { - return path.join(this.pullDir, 'patch'); + get commitInfoPath() { + return path.join(this.pullDir, 'commit-info'); } - get patch() { - return readFile(this.patchPath); + get commitInfo() { + return readJson(this.commitInfoPath); } getMessagePath(rev) { @@ -196,8 +196,8 @@ class Session { writeFile(this.metadataPath, status.metadata); } - savePatch(patch) { - writeFile(this.patchPath, patch); + saveCommitInfo(commitInfo) { + writeJson(this.commitInfoPath, commitInfo); } saveMessage(rev, message) { @@ -218,20 +218,21 @@ class Session { if (this.session.state === AMENDING) { return true; } else if (this.isApplying()) { - return !this.amInProgress(); + return !this.cherryPickInProgress(); } else { return false; } } readyToFinal() { - if (this.amInProgress()) { + if (this.amInProgress() || this.cherryPickInProgress()) { return false; // git am/rebase in progress } return this.session.state === AMENDING; } // Refs: https://github.com/git/git/blob/99de064/git-rebase.sh#L208-L228 + // XXX: This may be unused at this point? amInProgress() { const amPath = path.join(this.gitDir, 'rebase-apply', 'applying'); return fs.existsSync(amPath); @@ -247,6 +248,11 @@ class Session { return fs.existsSync(normalRebasePath) || fs.existsSync(mergeRebasePath); } + cherryPickInProgress() { + const cpPath = path.join(this.gitDir, 'CHERRY_PICK_HEAD'); + return fs.existsSync(cpPath); + } + restore() { const sess = this.session; if (sess.prid) { @@ -269,6 +275,19 @@ class Session { } } + async tryAbortCherryPick() { + const { cli } = this; + if (!this.cherryPickInProgress()) { + return cli.ok('No git cherry-pick in progress'); + } + const shouldAbortCherryPick = await cli.prompt( + 'Abort previous git cherry-pick sessions?'); + if (shouldAbortCherryPick) { + await forceRunAsync('git', ['cherry-pick', '--abort']); + cli.ok('Aborted previous git cherry-pick sessions'); + } + } + async tryAbortRebase() { const { cli } = this; if (!this.rebaseInProgress()) { @@ -284,6 +303,7 @@ class Session { async tryResetBranch() { const { cli, upstream, branch } = this; + await this.tryAbortCherryPick(); await this.tryAbortAm(); await this.tryAbortRebase();