Page MenuHomePhabricator
Files F14053120

D7761.id17596.diff
No OneTemporary
Actions

D7761.id17596.diff
View Options

Index: src/applications/diffusion/controller/DiffusionPushLogListController.php
===================================================================
--- src/applications/diffusion/controller/DiffusionPushLogListController.php
+++ src/applications/diffusion/controller/DiffusionPushLogListController.php
@@ -83,6 +83,10 @@
'href' => '/r'.$callsign.$log->getRefNew(),
),
$log->getRefNewShort()),
+
+ // TODO: Make these human-readable.
+ $log->getChangeFlags(),
+ $log->getRejectCode(),
phabricator_datetime($log->getEpoch(), $viewer),
);
}
@@ -98,6 +102,8 @@
pht('Name'),
pht('Old'),
pht('New'),
+ pht('Flags'),
+ pht('Code'),
pht('Date'),
))
->setColumnClasses(
Index: src/applications/diffusion/engine/DiffusionCommitHookEngine.php
===================================================================
--- src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -1,9 +1,12 @@
<?php
/**
- * @task git Git Hooks
- * @task hg Mercurial Hooks
- * @task svn Subversion Hooks
+ * @task config Configuring the Hook Engine
+ * @task hook Hook Execution
+ * @task git Git Hooks
+ * @task hg Mercurial Hooks
+ * @task svn Subversion Hooks
+ * @task internal Internals
*/
final class DiffusionCommitHookEngine extends Phobject {
@@ -11,6 +14,8 @@
const ENV_REMOTE_ADDRESS = 'PHABRICATOR_REMOTE_ADDRESS';
const ENV_REMOTE_PROTOCOL = 'PHABRICATOR_REMOTE_PROTOCOL';
+ const EMPTY_HASH = '0000000000000000000000000000000000000000';
+
private $viewer;
private $repository;
private $stdin;
@@ -20,6 +25,10 @@
private $remoteProtocol;
private $transactionKey;
+
+/* -( Config )------------------------------------------------------------- */
+
+
public function setRemoteProtocol($remote_protocol) {
$this->remoteProtocol = $remote_protocol;
return $this;
@@ -88,171 +97,190 @@
return $this->viewer;
}
+
+/* -( Hook Execution )----------------------------------------------------- */
+
+
public function execute() {
+ $ref_updates = $this->findRefUpdates();
+ $all_updates = $ref_updates;
+
+ $caught = null;
+ try {
+
+ try {
+ $this->rejectDangerousChanges($ref_updates);
+ } catch (DiffusionCommitHookRejectException $ex) {
+ // If we're rejecting dangerous changes, flag everything that we've
+ // seen as rejected so it's clear that none of it was accepted.
+ foreach ($all_updates as $update) {
+ $update->setRejectCode(
+ PhabricatorRepositoryPushLog::REJECT_DANGEROUS);
+ }
+ throw $ex;
+ }
+
+ // TODO: Fire ref herald rules.
+
+ $content_updates = $this->findContentUpdates($ref_updates);
+ $all_updates = array_merge($all_updates, $content_updates);
+
+ // TODO: Fire content Herald rules.
+ // TODO: Fire external hooks.
+
+ // If we make it this far, we're accepting these changes. Mark all the
+ // logs as accepted.
+ foreach ($all_updates as $update) {
+ $update->setRejectCode(PhabricatorRepositoryPushLog::REJECT_ACCEPT);
+ }
+ } catch (Exception $ex) {
+ // We'll throw this again in a minute, but we want to save all the logs
+ // first.
+ $caught = $ex;
+ }
+
+ // Save all the logs no matter what the outcome was.
+ foreach ($all_updates as $update) {
+ $update->save();
+ }
+
+ if ($caught) {
+ throw $caught;
+ }
+
+ return 0;
+ }
+
+ private function findRefUpdates() {
$type = $this->getRepository()->getVersionControlSystem();
switch ($type) {
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
- $err = $this->executeGitHook();
- break;
- case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
- $err = $this->executeSubversionHook();
- break;
+ return $this->findGitRefUpdates();
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
- $err = $this->executeMercurialHook();
- break;
+ return $this->findMercurialRefUpdates();
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+ return $this->findSubversionRefUpdates();
default:
throw new Exception(pht('Unsupported repository type "%s"!', $type));
}
-
- return $err;
}
- private function newPushLog() {
- return PhabricatorRepositoryPushLog::initializeNewLog($this->getViewer())
- ->setRepositoryPHID($this->getRepository()->getPHID())
- ->setEpoch(time())
- ->setRemoteAddress($this->getRemoteAddressForLog())
- ->setRemoteProtocol($this->getRemoteProtocol())
- ->setTransactionKey($this->getTransactionKey())
- ->setRejectCode(PhabricatorRepositoryPushLog::REJECT_ACCEPT)
- ->setRejectDetails(null);
- }
-
-
- /**
- * @task git
- */
- private function executeGitHook() {
- $updates = $this->parseGitUpdates($this->getStdin());
+ private function rejectDangerousChanges(array $ref_updates) {
+ assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog');
- $this->rejectGitDangerousChanges($updates);
+ $repository = $this->getRepository();
+ if ($repository->shouldAllowDangerousChanges()) {
+ return;
+ }
- // TODO: Do cheap checks: non-ff commits, mutating refs without access,
- // creating or deleting things you can't touch. We can do all non-content
- // checks here.
+ $flag_dangerous = PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS;
- $updates = $this->findGitNewCommits($updates);
+ foreach ($ref_updates as $ref_update) {
+ if (!$ref_update->hasChangeFlags($flag_dangerous)) {
+ // This is not a dangerous change.
+ continue;
+ }
- // TODO: Now, do content checks.
+ // We either have a branch deletion or a non fast-forward branch update.
+ // Format a message and reject the push.
- // TODO: Generalize this; just getting some data in the database for now.
+ $message = pht(
+ "DANGEROUS CHANGE: %s\n".
+ "Dangerous change protection is enabled for this repository.\n".
+ "Edit the repository configuration before making dangerous changes.",
+ $ref_update->getDangerousChangeDescription());
- $logs = array();
- foreach ($updates as $update) {
- $log = $this->newPushLog()
- ->setRefType($update['type'])
- ->setRefNameHash(PhabricatorHash::digestForIndex($update['ref']))
- ->setRefNameRaw($update['ref'])
- ->setRefNameEncoding(phutil_is_utf8($update['ref']) ? 'utf8' : null)
- ->setRefOld($update['old'])
- ->setRefNew($update['new'])
- ->setMergeBase(idx($update, 'merge-base'));
+ throw new DiffusionCommitHookRejectException($message);
+ }
+ }
- $flags = 0;
- if ($update['operation'] == 'create') {
- $flags = $flags | PhabricatorRepositoryPushLog::CHANGEFLAG_ADD;
- } else if ($update['operation'] == 'delete') {
- $flags = $flags | PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE;
- } else {
- // TODO: This isn't correct; these might be APPEND or REWRITE, and
- // if they're REWRITE they might be DANGEROUS. Fix this when this
- // gets generalized.
- $flags = $flags | PhabricatorRepositoryPushLog::CHANGEFLAG_APPEND;
- }
+ private function findContentUpdates(array $ref_updates) {
+ assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog');
- $log->setChangeFlags($flags);
- $logs[] = $log;
+ $type = $this->getRepository()->getVersionControlSystem();
+ switch ($type) {
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+ return $this->findGitContentUpdates($ref_updates);
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+ return $this->findMercurialContentUpdates($ref_updates);
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+ return $this->findSubversionContentUpdates($ref_updates);
+ default:
+ throw new Exception(pht('Unsupported repository type "%s"!', $type));
}
+ }
- // Now, build logs for all the commits.
- // TODO: Generalize this, too.
- $commits = array_mergev(ipull($updates, 'commits'));
- $commits = array_unique($commits);
- foreach ($commits as $commit) {
- $log = $this->newPushLog()
- ->setRefType(PhabricatorRepositoryPushLog::REFTYPE_COMMIT)
- ->setRefNew($commit)
- ->setChangeFlags(PhabricatorRepositoryPushLog::CHANGEFLAG_ADD);
- $logs[] = $log;
- }
- foreach ($logs as $log) {
- $log->save();
- }
+/* -( Git )---------------------------------------------------------------- */
- return 0;
- }
+ private function findGitRefUpdates() {
+ $ref_updates = array();
- /**
- * @task git
- */
- private function parseGitUpdates($stdin) {
- $updates = array();
+ // First, parse stdin, which lists all the ref changes. The input looks
+ // like this:
+ //
+ // <old hash> <new hash> <ref>
+ $stdin = $this->getStdin();
$lines = phutil_split_lines($stdin, $retain_endings = false);
foreach ($lines as $line) {
$parts = explode(' ', $line, 3);
if (count($parts) != 3) {
throw new Exception(pht('Expected "old new ref", got "%s".', $line));
}
- $update = array(
- 'old' => $parts[0],
- 'old.short' => substr($parts[0], 0, 8),
- 'new' => $parts[1],
- 'new.short' => substr($parts[1], 0, 8),
- 'ref' => $parts[2],
- );
-
- if (preg_match('(^refs/heads/)', $update['ref'])) {
- $update['type'] = 'branch';
- $update['ref.short'] = substr($update['ref'], strlen('refs/heads/'));
- } else if (preg_match('(^refs/tags/)', $update['ref'])) {
- $update['type'] = 'tag';
- $update['ref.short'] = substr($update['ref'], strlen('refs/tags/'));
+
+ $ref_old = $parts[0];
+ $ref_new = $parts[1];
+ $ref_raw = $parts[2];
+
+ if (preg_match('(^refs/heads/)', $ref_raw)) {
+ $ref_type = PhabricatorRepositoryPushLog::REFTYPE_BRANCH;
+ } else if (preg_match('(^refs/tags/)', $ref_raw)) {
+ $ref_type = PhabricatorRepositoryPushLog::REFTYPE_TAG;
} else {
- $update['type'] = 'unknown';
+ $ref_type = PhabricatorRepositoryPushLog::REFTYPE_UNKNOWN;
}
- $updates[] = $update;
+ $ref_update = $this->newPushLog()
+ ->setRefType($ref_type)
+ ->setRefName($ref_raw)
+ ->setRefOld($ref_old)
+ ->setRefNew($ref_new);
+
+ $ref_updates[] = $ref_update;
}
- $updates = $this->findGitMergeBases($updates);
+ $this->findGitMergeBases($ref_updates);
+ $this->findGitChangeFlags($ref_updates);
- return $updates;
+ return $ref_updates;
}
- /**
- * @task git
- */
- private function findGitMergeBases(array $updates) {
- $empty = str_repeat('0', 40);
+ private function findGitMergeBases(array $ref_updates) {
+ assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog');
$futures = array();
- foreach ($updates as $key => $update) {
- // Updates are in the form:
- //
- // <old hash> <new hash> <ref>
- //
+ foreach ($ref_updates as $key => $ref_update) {
// If the old hash is "00000...", the ref is being created (either a new
// branch, or a new tag). If the new hash is "00000...", the ref is being
// deleted. If both are nonempty, the ref is being updated. For updates,
// we'll figure out the `merge-base` of the old and new objects here. This
// lets us reject non-FF changes cheaply; later, we'll figure out exactly
// which commits are new.
+ $ref_old = $ref_update->getRefOld();
+ $ref_new = $ref_update->getRefNew();
- if ($update['old'] == $empty) {
- $updates[$key]['operation'] = 'create';
- } else if ($update['new'] == $empty) {
- $updates[$key]['operation'] = 'delete';
- } else {
- $updates[$key]['operation'] = 'update';
- $futures[$key] = $this->getRepository()->getLocalCommandFuture(
- 'merge-base %s %s',
- $update['old'],
- $update['new']);
+ if (($ref_old === self::EMPTY_HASH) ||
+ ($ref_new === self::EMPTY_HASH)) {
+ continue;
}
+
+ $futures[$key] = $this->getRepository()->getLocalCommandFuture(
+ 'merge-base %s %s',
+ $ref_old,
+ $ref_new);
}
foreach (Futures($futures)->limit(8) as $key => $future) {
@@ -264,20 +292,86 @@
// T4224. Assume this means there are no ancestors.
list($err, $stdout) = $future->resolve();
+
if ($err) {
- $updates[$key]['merge-base'] = null;
+ $merge_base = null;
} else {
- $updates[$key]['merge-base'] = rtrim($stdout, "\n");
+ $merge_base = rtrim($stdout, "\n");
}
+
+ $ref_update->setMergeBase($merge_base);
}
- return $updates;
+ return $ref_updates;
}
- private function findGitNewCommits(array $updates) {
+
+ private function findGitChangeFlags(array $ref_updates) {
+ assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog');
+
+ foreach ($ref_updates as $key => $ref_update) {
+ $ref_old = $ref_update->getRefOld();
+ $ref_new = $ref_update->getRefNew();
+ $ref_type = $ref_update->getRefType();
+
+ $ref_flags = 0;
+ $dangerous = null;
+
+ if ($ref_old === self::EMPTY_HASH) {
+ $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_ADD;
+ } else if ($ref_new === self::EMPTY_HASH) {
+ $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE;
+ if ($ref_type == PhabricatorRepositoryPushLog::REFTYPE_BRANCH) {
+ $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS;
+ $dangerous = pht(
+ "The change you're attempting to push deletes the branch '%s'.",
+ $ref_update->getRefName());
+ }
+ } else {
+ $merge_base = $ref_update->getMergeBase();
+ if ($merge_base == $ref_old) {
+ // This is a fast-forward update to an existing branch.
+ // These are safe.
+ $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_APPEND;
+ } else {
+ $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_REWRITE;
+
+ // For now, we don't consider deleting or moving tags to be a
+ // "dangerous" update. It's way harder to get wrong and should be easy
+ // to recover from once we have better logging. Only add the dangerous
+ // flag if this ref is a branch.
+
+ if ($ref_type == PhabricatorRepositoryPushLog::REFTYPE_BRANCH) {
+ $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS;
+
+ $dangerous = pht(
+ "DANGEROUS CHANGE: The change you're attempting to push updates ".
+ "the branch '%s' from '%s' to '%s', but this is not a ".
+ "fast-forward. Pushes which rewrite published branch history ".
+ "are dangerous.",
+ $ref_update->getRefName(),
+ $ref_update->getRefOldShort(),
+ $ref_update->getRefNewShort());
+ }
+ }
+ }
+
+ $ref_update->setChangeFlags($ref_flags);
+ if ($dangerous !== null) {
+ $ref_update->attachDangerousChangeDescription($dangerous);
+ }
+ }
+
+ return $ref_updates;
+ }
+
+
+ private function findGitContentUpdates(array $ref_updates) {
+ $flag_delete = PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE;
+
$futures = array();
- foreach ($updates as $key => $update) {
- if ($update['operation'] == 'delete') {
+ foreach ($ref_updates as $key => $ref_update) {
+ if ($ref_update->hasChangeFlags($flag_delete)) {
// Deleting a branch or tag can never create any new commits.
continue;
}
@@ -289,84 +383,78 @@
$futures[$key] = $this->getRepository()->getLocalCommandFuture(
'log --format=%s %s --not --all',
'%H',
- $update['new']);
+ $ref_update->getRefNew());
}
+ $content_updates = array();
foreach (Futures($futures)->limit(8) as $key => $future) {
list($stdout) = $future->resolvex();
+
+ if (!strlen(trim($stdout))) {
+ // This change doesn't have any new commits. One common case of this
+ // is creating a new tag which points at an existing commit.
+ continue;
+ }
+
$commits = phutil_split_lines($stdout, $retain_newlines = false);
- $updates[$key]['commits'] = $commits;
+
+ foreach ($commits as $commit) {
+ $content_updates[$commit] = $this->newPushLog()
+ ->setRefType(PhabricatorRepositoryPushLog::REFTYPE_COMMIT)
+ ->setRefNew($commit)
+ ->setChangeFlags(PhabricatorRepositoryPushLog::CHANGEFLAG_ADD);
+ }
}
- return $updates;
+ return $content_updates;
}
- private function rejectGitDangerousChanges(array $updates) {
- $repository = $this->getRepository();
- if ($repository->shouldAllowDangerousChanges()) {
- return;
- }
- foreach ($updates as $update) {
- if ($update['type'] != 'branch') {
- // For now, we don't consider deleting or moving tags to be a
- // "dangerous" update. It's way harder to get wrong and should be easy
- // to recover from once we have better logging.
- continue;
- }
+/* -( Mercurial )---------------------------------------------------------- */
- if ($update['operation'] == 'create') {
- // Creating a branch is never dangerous.
- continue;
- }
- if ($update['operation'] == 'update') {
- if ($update['old'] == $update['merge-base']) {
- // This is a fast-forward update to an existing branch.
- // These are safe.
- continue;
- }
- }
+ private function findMercurialRefUpdates() {
+ // TODO: Implement.
+ return array();
+ }
- // We either have a branch deletion or a non fast-forward branch update.
- // Format a message and reject the push.
+ private function findMercurialContentUpdates(array $ref_updates) {
+ // TODO: Implement.
+ return array();
+ }
- if ($update['operation'] == 'delete') {
- $message = pht(
- "DANGEROUS CHANGE: The change you're attempting to push deletes ".
- "the branch '%s'.",
- $update['ref.short']);
- } else {
- $message = pht(
- "DANGEROUS CHANGE: The change you're attempting to push updates ".
- "the branch '%s' from '%s' to '%s', but this is not a fast-forward. ".
- "Pushes which rewrite published branch history are dangerous.",
- $update['ref.short'],
- $update['old.short'],
- $update['new.short']);
- }
- $boilerplate = pht(
- "Dangerous change protection is enabled for this repository.\n".
- "Edit the repository configuration before making dangerous changes.");
+/* -( Subversion )--------------------------------------------------------- */
- $message = $message."\n".$boilerplate;
- throw new DiffusionCommitHookRejectException($message);
- }
+ private function findSubversionRefUpdates() {
+ // TODO: Implement.
+ return array();
}
- private function executeSubversionHook() {
+ private function findSubversionContentUpdates(array $ref_updates) {
+ // TODO: Implement.
+ return array();
+ }
- // TODO: Do useful things here, too.
- return 0;
- }
+/* -( Internals )---------------------------------------------------------- */
- private function executeMercurialHook() {
- // TODO: Here, too, useful things should be done.
+ private function newPushLog() {
+ // NOTE: By default, we create these with REJECT_BROKEN as the reject
+ // code. This indicates a broken hook, and covers the case where we
+ // encounter some unexpected exception and consequently reject the changes.
- return 0;
+ return PhabricatorRepositoryPushLog::initializeNewLog($this->getViewer())
+ ->attachRepository($this->getRepository())
+ ->setRepositoryPHID($this->getRepository()->getPHID())
+ ->setEpoch(time())
+ ->setRemoteAddress($this->getRemoteAddressForLog())
+ ->setRemoteProtocol($this->getRemoteProtocol())
+ ->setTransactionKey($this->getTransactionKey())
+ ->setRejectCode(PhabricatorRepositoryPushLog::REJECT_BROKEN)
+ ->setRejectDetails(null);
}
+
}
Index: src/applications/repository/storage/PhabricatorRepositoryPushLog.php
===================================================================
--- src/applications/repository/storage/PhabricatorRepositoryPushLog.php
+++ src/applications/repository/storage/PhabricatorRepositoryPushLog.php
@@ -18,6 +18,7 @@
const REFTYPE_BOOKMARK = 'bookmark';
const REFTYPE_SVN = 'svn';
const REFTYPE_COMMIT = 'commit';
+ const REFTYPE_UNKNOWN = 'unknown';
const CHANGEFLAG_ADD = 1;
const CHANGEFLAG_DELETE = 2;
@@ -29,6 +30,7 @@
const REJECT_DANGEROUS = 1;
const REJECT_HERALD = 2;
const REJECT_EXTERNAL = 3;
+ const REJECT_BROKEN = 4;
protected $repositoryPHID;
protected $epoch;
@@ -47,6 +49,7 @@
protected $rejectCode;
protected $rejectDetails;
+ private $dangerousChangeDescription = self::ATTACHABLE;
private $repository = self::ATTACHABLE;
public static function initializeNewLog(PhabricatorUser $viewer) {
@@ -76,6 +79,16 @@
return phutil_utf8ize($this->getRefNameRaw());
}
+ public function setRefName($ref_raw) {
+ $encoding = phutil_is_utf8($ref_raw) ? 'utf8' : null;
+
+ $this->setRefNameRaw($ref_raw);
+ $this->setRefNameHash(PhabricatorHash::digestForIndex($ref_raw));
+ $this->setRefNameEncoding($encoding);
+
+ return $this;
+ }
+
public function getRefOldShort() {
if ($this->getRepository()->isSVN()) {
return $this->getRefOld();
@@ -90,6 +103,19 @@
return substr($this->getRefNew(), 0, 12);
}
+ public function hasChangeFlags($mask) {
+ return ($this->changeFlags & $mask);
+ }
+
+ public function attachDangerousChangeDescription($description) {
+ $this->dangerousChangeDescription = $description;
+ return $this;
+ }
+
+ public function getDangerousChangeDescription() {
+ return $this->assertAttached($this->dangerousChangeDescription);
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 16, 12:51 PM (3 d, 20 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6752605
Default Alt Text
D7761.id17596.diff (22 KB)

Event Timeline

Phabricator · Built by Phacility