Page MenuHomePhabricator
Files F13961247

D9599.diff
No OneTemporary
Actions

D9599.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
@@ -127,6 +127,35 @@
$caught = null;
try {
+ if ($this->isPushForReview($all_updates)) {
+ $diff = $this->generateDiffForReview($all_updates);
+
+ // TODO: Enrich this diff on many dimensions; these are required
+ // to save the object.
+ $diff->setLintStatus(DifferentialLintStatus::LINT_NONE);
+ $diff->setUnitStatus(DifferentialUnitStatus::UNIT_NONE);
+
+ $diff->save();
+
+ $diff_uri = PhabricatorEnv::getProductionURI(
+ '/differential/diff/'.$diff->getID().'/');
+
+ $console = PhutilConsole::getConsole();
+ $console->writeErr(
+ "\n%s\n\n",
+ pht(
+ "Success! Created a new diff fom your changes:\n\n %s\n\n".
+ "Since this was a push for review, your changes will not be\n".
+ "applied to the remote. This push will now be rejected.",
+ $diff_uri));
+
+ // TODO: It would be nice to make Git think this push worked, but
+ // this is just an ease-of-use issue. For now, return an error to
+ // reject the changes.
+
+ return 77;
+ }
+
try {
$this->rejectDangerousChanges($ref_updates);
} catch (DiffusionCommitHookRejectException $ex) {
@@ -252,6 +281,36 @@
}
}
+ private function isPushForReview(array $ref_updates) {
+ assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog');
+
+ $type = $this->getRepository()->getVersionControlSystem();
+ switch ($type) {
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+ return $this->isGitPushForReview($ref_updates);
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+ return false;
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+ return false;
+ default:
+ throw new Exception(pht('Unsupported repository type "%s"!', $type));
+ }
+ }
+
+ private function generateDiffForReview(array $ref_updates) {
+ assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog');
+
+ $update = head($ref_updates);
+ $type = $this->getRepository()->getVersionControlSystem();
+ switch ($type) {
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+ return $this->generateGitDiffForReview($update);
+ default:
+ throw new Exception(pht('Unsupported repository type "%s"!', $type));
+ }
+ }
+
+
/* -( Herald )------------------------------------------------------------- */
@@ -581,6 +640,89 @@
return $content_updates;
}
+ private function isGitPushForReview(array $ref_updates) {
+ foreach ($ref_updates as $update) {
+ $name = $update->getRefName();
+ if (preg_match('(^review(?:/|\z))', $name)) {
+ if (count($ref_updates) !== 1) {
+ throw new DiffusionCommitHookRejectException(
+ pht(
+ 'This push updates several branches, including a virtual branch '.
+ 'which triggers code review (%s). When pushing changes for '.
+ 'review, you must push to ONLY the virtual review branch.',
+ $name));
+ }
+
+ $update = head($ref_updates);
+
+ $type_branch = PhabricatorRepositoryPushLog::REFTYPE_BRANCH;
+ if ($update->getRefType() != $type_branch) {
+ throw new DiffusionCommitHookRejectException(
+ pht(
+ 'This push updates a virtual ref (%s), but does not push a '.
+ 'branch to it. Virtual ref names are reserved and can only be '.
+ 'used for branches.',
+ $name));
+ }
+
+ $flags = $update->getChangeFlags();
+ $flag_add = PhabricatorRepositoryPushLog::CHANGEFLAG_ADD;
+ $flag_delete = PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE;
+ if ($flags & $flag_delete) {
+ // This is deleting a virtual branch, so let it through as a normal
+ // push. Most likely, this is cleaning up a mistake where a virtual
+ // branch was somehow made concrete.
+ return false;
+ }
+
+ if ($flags !== PhabricatorRepositoryPushLog::CHANGEFLAG_ADD) {
+ throw new DiffusionCommitHookRejectException(
+ pht(
+ 'This push updates a virtual branch (%s). Pushes to virtual '.
+ 'branches must create them, but this push does not. Usually, '.
+ 'this means someone has accidentally created a real branch '.
+ 'with this name. Verify the offending branch is safe to '.
+ 'delete, remove it, and then try pushing again.',
+ $name));
+ }
+
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ private function generateGitDiffForReview(PhabricatorRepositoryPushLog $ref) {
+ $old = $ref->getMergeBase();
+ $new = $ref->getRefNew();
+
+ // TODO: Duplication with arc.
+ $options = array(
+ // NOTE: This is not in `arc`.
+ '--binary',
+
+ '--no-ext-diff',
+ '--no-textconv',
+ '--no-color',
+ '--src-prefix=a/',
+ '--dst-prefix=b/',
+ '-U32767',
+ '-M',
+ '-C',
+ );
+
+ list($diff) = $this->getRepository()->execxLocalCommand(
+ 'diff %Ls %s --',
+ $options,
+ $old.'..'.$new);
+
+ $changes = id(new ArcanistDiffParser())->parseDiff($diff);
+
+ return DifferentialDiff::newFromRawChanges($changes);
+ }
+
+
/* -( Custom )------------------------------------------------------------- */
private function applyCustomHooks(array $updates) {

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 16, 4:00 AM (3 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6715816
Default Alt Text
D9599.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility