Page MenuHomePhabricator
Files F15436117

D9122.id21667.diff
No OneTemporary
Actions

D9122.id21667.diff
View Options

diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -730,35 +730,29 @@
sort($old_heads);
sort($new_heads);
+ if (!$old_heads && !$new_heads) {
+ // This should never be possible, as it makes no sense. Explode.
+ throw new Exception(
+ pht(
+ 'Mercurial repository has no new or old heads for branch "%s" '.
+ 'after push. This makes no sense; rejecting change.',
+ $ref));
+ }
+
if ($old_heads === $new_heads) {
// No changes to this branch, so skip it.
continue;
}
- if (!$new_heads) {
- if ($old_heads) {
- // TODO: This comment is wrong, and branches can be deleted with
- // --close-branch. Fix it soon: see T5050.
- // It looks like this push deletes a branch, but that isn't possible
- // in Mercurial, so something is going wrong here. Bail out.
- throw new Exception(
- pht(
- 'Mercurial repository has no new head for branch "%s" after '.
- 'push. This is unexpected; rejecting change.',
- $ref));
- } else {
- // Obviously, this should never be possible either, as it makes
- // no sense. Explode.
- throw new Exception(
- pht(
- 'Mercurial repository has no new or old heads for branch "%s" '.
- 'after push. This makes no sense; rejecting change.',
- $ref));
- }
- }
-
$stray_heads = array();
- if (count($old_heads) > 1) {
+
+ if ($old_heads && !$new_heads) {
+ // This is a branch deletion with "--close-branch".
+ $head_map = array();
+ foreach ($old_heads as $old_head) {
+ $head_map[$old_head] = array(self::EMPTY_HASH);
+ }
+ } else if (count($old_heads) > 1) {
// HORRIBLE: In Mercurial, branches can have multiple heads. If the
// old branch had multiple heads, we need to figure out which new
// heads descend from which old heads, so we can tell whether you're
@@ -777,13 +771,24 @@
$head_map = array();
foreach (Futures($dfutures) as $future_head => $dfuture) {
list($stdout) = $dfuture->resolvex();
- $head_map[$future_head] = array_filter(explode("\1", $stdout));
+ $descendant_heads = array_filter(explode("\1", $stdout));
+ if ($descendant_heads) {
+ // This old head has at least one descendant in the push.
+ $head_map[$future_head] = $descendant_heads;
+ } else {
+ // This old head has no descendants, so it is being deleted.
+ $head_map[$future_head] = array(self::EMPTY_HASH);
+ }
}
// Now, find all the new stray heads this push creates, if any. These
// are new heads which do not descend from the old heads.
$seen = array_fuse(array_mergev($head_map));
foreach ($new_heads as $new_head) {
+ if ($new_head === self::EMPTY_HASH) {
+ // If a branch head is being deleted, don't insert it as an add.
+ continue;
+ }
if (empty($seen[$new_head])) {
$head_map[self::EMPTY_HASH][] = $new_head;
}
@@ -808,6 +813,8 @@
$ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_APPEND;
}
+
+ $deletes_existing_head = ($new_head == self::EMPTY_HASH);
$splits_existing_head = (count($child_heads) > 1);
$creates_duplicate_head = ($old_head == self::EMPTY_HASH) &&
(count($head_map) > 1);
@@ -844,6 +851,19 @@
}
}
+ if ($deletes_existing_head) {
+ // TODO: Somewhere in here we should be setting CHANGEFLAG_REWRITE
+ // if we are also creating at least one other head to replace
+ // this one.
+
+ // NOTE: In Git, this is a dangerous change, but it is not dangerous
+ // in Mercurial. Mercurial branches are version controlled, and
+ // Mercurial does not prompt you for any special flags when pushing
+ // a `--close-branch` commit by default.
+
+ $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE;
+ }
+
$ref_update = $this->newPushLog()
->setRefType(PhabricatorRepositoryPushLog::REFTYPE_BRANCH)
->setRefName($ref)
@@ -1084,6 +1104,11 @@
$byte_limit));
}
+ if (!strlen($raw_diff)) {
+ // If the commit is actually empty, just return no changesets.
+ return array();
+ }
+
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($raw_diff);
$diff = DifferentialDiff::newFromRawChanges($changes);
diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
--- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -131,6 +131,7 @@
$this->changesets = $this->getHookEngine()->loadChangesetsForCommit(
$this->getObject()->getRefNew());
} catch (Exception $ex) {
+ phlog($ex);
$this->changesets = $ex;
}
}

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 26, 12:03 PM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7723696
Default Alt Text
D9122.id21667.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility