chore(git-node): avoid dealing with patch files for landing

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;
  }

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);
  });
  });
 }

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();