From 61c934449d17062786bfe7eaf31ae1bd05b2be7f Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 Dec 2013 14:46:46 -0800 Subject: [PATCH] Fix fatal in Git hook when a --force push completely rewrites a ref Summary: Fixes T4224. If you `git merge-base A B`, and they have //no// ancestor, the command exits with an error. Assume errors mean "no ancestry" and continue. Test Plan: Completely rewrite a repository with a `--force` push. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4224 Differential Revision: https://secure.phabricator.com/D7756 --- .../engine/DiffusionCommitHookEngine.php | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index b76c4b8549..d57f4480bb 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -256,8 +256,19 @@ final class DiffusionCommitHookEngine extends Phobject { } foreach (Futures($futures)->limit(8) as $key => $future) { - list($stdout) = $future->resolvex(); - $updates[$key]['merge-base'] = rtrim($stdout, "\n"); + + // If 'old' and 'new' have no common ancestors (for example, a force push + // which completely rewrites a ref), `git merge-base` will exit with + // an error and no output. It would be nice to find a positive test + // for this instead, but I couldn't immediately come up with one. See + // T4224. Assume this means there are no ancestors. + + list($err, $stdout) = $future->resolve(); + if ($err) { + $updates[$key]['merge-base'] = null; + } else { + $updates[$key]['merge-base'] = rtrim($stdout, "\n"); + } } return $updates;