Page MenuHomePhabricator
Files F15461241

D14968.id36162.diff
No OneTemporary
Actions

D14968.id36162.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2143,6 +2143,7 @@
'PhabricatorDiffPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php',
'PhabricatorDifferenceEngine' => 'infrastructure/diff/PhabricatorDifferenceEngine.php',
'PhabricatorDifferentialApplication' => 'applications/differential/application/PhabricatorDifferentialApplication.php',
+ 'PhabricatorDifferentialAttachCommitWorkflow' => 'applications/differential/management/PhabricatorDifferentialAttachCommitWorkflow.php',
'PhabricatorDifferentialConfigOptions' => 'applications/differential/config/PhabricatorDifferentialConfigOptions.php',
'PhabricatorDifferentialExtractWorkflow' => 'applications/differential/management/PhabricatorDifferentialExtractWorkflow.php',
'PhabricatorDifferentialManagementWorkflow' => 'applications/differential/management/PhabricatorDifferentialManagementWorkflow.php',
@@ -6379,6 +6380,7 @@
'PhabricatorDiffPreferencesSettingsPanel' => 'PhabricatorSettingsPanel',
'PhabricatorDifferenceEngine' => 'Phobject',
'PhabricatorDifferentialApplication' => 'PhabricatorApplication',
+ 'PhabricatorDifferentialAttachCommitWorkflow' => 'PhabricatorDifferentialManagementWorkflow',
'PhabricatorDifferentialConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorDifferentialExtractWorkflow' => 'PhabricatorDifferentialManagementWorkflow',
'PhabricatorDifferentialManagementWorkflow' => 'PhabricatorManagementWorkflow',
diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
--- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
+++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
@@ -87,4 +87,174 @@
return $diff->save();
}
+ public function isDiffChangedBeforeCommit(
+ PhabricatorRepositoryCommit $commit,
+ DifferentialDiff $old,
+ DifferentialDiff $new) {
+
+ $viewer = $this->getViewer();
+ $repository = $commit->getRepository();
+ $identifier = $commit->getCommitIdentifier();
+
+ $vs_changesets = array();
+ foreach ($old->getChangesets() as $changeset) {
+ $path = $changeset->getAbsoluteRepositoryPath($repository, $old);
+ $path = ltrim($path, '/');
+ $vs_changesets[$path] = $changeset;
+ }
+
+ $changesets = array();
+ foreach ($new->getChangesets() as $changeset) {
+ $path = $changeset->getAbsoluteRepositoryPath($repository, $new);
+ $path = ltrim($path, '/');
+ $changesets[$path] = $changeset;
+ }
+
+ if (array_fill_keys(array_keys($changesets), true) !=
+ array_fill_keys(array_keys($vs_changesets), true)) {
+ return true;
+ }
+
+ $file_phids = array();
+ foreach ($vs_changesets as $changeset) {
+ $metadata = $changeset->getMetadata();
+ $file_phid = idx($metadata, 'new:binary-phid');
+ if ($file_phid) {
+ $file_phids[$file_phid] = $file_phid;
+ }
+ }
+
+ $files = array();
+ if ($file_phids) {
+ $files = id(new PhabricatorFileQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withPHIDs($file_phids)
+ ->execute();
+ $files = mpull($files, null, 'getPHID');
+ }
+
+ foreach ($changesets as $path => $changeset) {
+ $vs_changeset = $vs_changesets[$path];
+
+ $file_phid = idx($vs_changeset->getMetadata(), 'new:binary-phid');
+ if ($file_phid) {
+ if (!isset($files[$file_phid])) {
+ return true;
+ }
+
+ $drequest = DiffusionRequest::newFromDictionary(array(
+ 'user' => $viewer,
+ 'repository' => $repository,
+ 'commit' => $identifier,
+ 'path' => $path,
+ ));
+
+ $corpus = DiffusionFileContentQuery::newFromDiffusionRequest($drequest)
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->loadFileContent()
+ ->getCorpus();
+
+ if ($files[$file_phid]->loadFileData() != $corpus) {
+ return true;
+ }
+ } else {
+ $context = implode("\n", $changeset->makeChangesWithContext());
+ $vs_context = implode("\n", $vs_changeset->makeChangesWithContext());
+
+ // We couldn't just compare $context and $vs_context because following
+ // diffs will be considered different:
+ //
+ // -(empty line)
+ // -echo 'test';
+ // (empty line)
+ //
+ // (empty line)
+ // -echo "test";
+ // -(empty line)
+
+ $hunk = id(new DifferentialModernHunk())->setChanges($context);
+ $vs_hunk = id(new DifferentialModernHunk())->setChanges($vs_context);
+ if ($hunk->makeOldFile() != $vs_hunk->makeOldFile() ||
+ $hunk->makeNewFile() != $vs_hunk->makeNewFile()) {
+ return true;
+ }
+ }
+ }
+
+ return false;
+ }
+
+ public function updateRevisionWithCommit(
+ DifferentialRevision $revision,
+ PhabricatorRepositoryCommit $commit,
+ array $more_xactions,
+ PhabricatorContentSource $content_source) {
+
+ $viewer = $this->getViewer();
+ $result_data = array();
+
+ $new_diff = $this->newDiffFromCommit($commit);
+
+ $old_diff = $revision->getActiveDiff();
+ $changed_uri = null;
+ if ($old_diff) {
+ $old_diff = id(new DifferentialDiffQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($old_diff->getID()))
+ ->needChangesets(true)
+ ->executeOne();
+ if ($old_diff) {
+ $has_changed = $this->isDiffChangedBeforeCommit(
+ $commit,
+ $old_diff,
+ $new_diff);
+ if ($has_changed) {
+ $result_data['vsDiff'] = $old_diff->getID();
+
+ $revision_monogram = $revision->getMonogram();
+ $old_id = $old_diff->getID();
+ $new_id = $new_diff->getID();
+
+ $changed_uri = "/{$revision_monogram}?vs={$old_id}&id={$new_id}#toc";
+ $changed_uri = PhabricatorEnv::getProductionURI($changed_uri);
+ }
+ }
+ }
+
+ $xactions = array();
+
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+ ->setIgnoreOnNoEffect(true)
+ ->setNewValue($new_diff->getPHID())
+ ->setMetadataValue('isCommitUpdate', true);
+
+ foreach ($more_xactions as $more_xaction) {
+ $xactions[] = $more_xaction;
+ }
+
+ $editor = id(new DifferentialTransactionEditor())
+ ->setActor($viewer)
+ ->setContinueOnMissingFields(true)
+ ->setContentSource($content_source)
+ ->setChangedPriorToCommitURI($changed_uri)
+ ->setIsCloseByCommit(true);
+
+ $author_phid = $this->getAuthorPHID();
+ if ($author_phid !== null) {
+ $editor->setActingAsPHID($author_phid);
+ }
+
+ try {
+ $editor->applyTransactions($revision, $xactions);
+ } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
+ // NOTE: We've marked transactions other than the CLOSE transaction
+ // as ignored when they don't have an effect, so this means that we
+ // lost a race to close the revision. That's perfectly fine, we can
+ // just continue normally.
+ }
+
+ return $result_data;
+ }
+
}
diff --git a/src/applications/differential/management/PhabricatorDifferentialAttachCommitWorkflow.php b/src/applications/differential/management/PhabricatorDifferentialAttachCommitWorkflow.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/management/PhabricatorDifferentialAttachCommitWorkflow.php
@@ -0,0 +1,90 @@
+<?php
+
+final class PhabricatorDifferentialAttachCommitWorkflow
+ extends PhabricatorDifferentialManagementWorkflow {
+
+ protected function didConstruct() {
+ $this
+ ->setName('attach-commit')
+ ->setExamples('**attach-commit** __commit__ __revision__')
+ ->setSynopsis(pht('Forcefully attach a commit to a revision.'))
+ ->setArguments(
+ array(
+ array(
+ 'name' => 'argv',
+ 'wildcard' => true,
+ 'help' => pht('Commit, and a revision to attach it to.'),
+ ),
+ ));
+ }
+
+ public function execute(PhutilArgumentParser $args) {
+ $viewer = $this->getViewer();
+
+ $argv = $args->getArg('argv');
+ if (count($argv) !== 2) {
+ throw new PhutilArgumentUsageException(
+ pht('Specify a commit and a revision to attach it to.'));
+ }
+
+ $commit_name = head($argv);
+ $revision_name = last($argv);
+
+ $commit = id(new DiffusionCommitQuery())
+ ->setViewer($viewer)
+ ->withIdentifiers(array($commit_name))
+ ->executeOne();
+ if (!$commit) {
+ throw new PhutilArgumentUsageException(
+ pht('Commit "%s" does not exist.', $commit_name));
+ }
+
+ $revision = id(new PhabricatorObjectQuery())
+ ->setViewer($viewer)
+ ->withNames(array($revision_name))
+ ->executeOne();
+
+ if (!$revision) {
+ throw new PhutilArgumentUsageException(
+ pht('Revision "%s" does not exist.', $revision_name));
+ }
+
+ if (!($revision instanceof DifferentialRevision)) {
+ throw new PhutilArgumentUsageException(
+ pht('Object "%s" must be a Differential revision.', $revision_name));
+ }
+
+ // Reload the revision to get the active diff.
+ $revision = id(new DifferentialRevisionQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($revision->getID()))
+ ->needActiveDiffs(true)
+ ->executeOne();
+
+ $differential_phid = id(new PhabricatorDifferentialApplication())
+ ->getPHID();
+
+ $extraction_engine = id(new DifferentialDiffExtractionEngine())
+ ->setViewer($viewer)
+ ->setAuthorPHID($differential_phid);
+
+ $content_source = PhabricatorContentSource::newForSource(
+ PhabricatorContentSource::SOURCE_CONSOLE,
+ array());
+
+ $extraction_engine->updateRevisionWithCommit(
+ $revision,
+ $commit,
+ array(),
+ $content_source);
+
+ echo tsprintf(
+ "%s\n",
+ pht(
+ 'Attached "%s" to "%s".',
+ $commit->getMonogram(),
+ $revision->getMonogram()));
+ }
+
+
+}
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -216,50 +216,24 @@
$low_level_query->getRevisionMatchData());
}
- $diff = id(new DifferentialDiffExtractionEngine())
+ $extraction_engine = id(new DifferentialDiffExtractionEngine())
->setViewer($actor)
- ->setAuthorPHID($acting_as_phid)
- ->newDiffFromCommit($commit);
-
- $vs_diff = $this->loadChangedByCommit($revision, $diff);
- $changed_uri = null;
- if ($vs_diff) {
- $data->setCommitDetail('vsDiff', $vs_diff->getID());
-
- $changed_uri = PhabricatorEnv::getProductionURI(
- '/D'.$revision->getID().
- '?vs='.$vs_diff->getID().
- '&id='.$diff->getID().
- '#toc');
- }
-
- $xactions = array();
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
- ->setIgnoreOnNoEffect(true)
- ->setNewValue($diff->getPHID())
- ->setMetadataValue('isCommitUpdate', true);
- $xactions[] = $commit_close_xaction;
+ ->setAuthorPHID($acting_as_phid);
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_DAEMON,
array());
- $editor = id(new DifferentialTransactionEditor())
- ->setActor($actor)
- ->setActingAsPHID($acting_as_phid)
- ->setContinueOnMissingFields(true)
- ->setContentSource($content_source)
- ->setChangedPriorToCommitURI($changed_uri)
- ->setIsCloseByCommit(true);
-
- try {
- $editor->applyTransactions($revision, $xactions);
- } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
- // NOTE: We've marked transactions other than the CLOSE transaction
- // as ignored when they don't have an effect, so this means that we
- // lost a race to close the revision. That's perfectly fine, we can
- // just continue normally.
+ $update_data = $extraction_engine->updateRevisionWithCommit(
+ $revision,
+ $commit,
+ array(
+ $commit_close_xaction,
+ ),
+ $content_source);
+
+ foreach ($update_data as $key => $value) {
+ $data->setCommitDetail($key, $value);
}
}
}
@@ -280,112 +254,6 @@
PhabricatorRepositoryCommit::IMPORTED_MESSAGE);
}
-
- private function loadChangedByCommit(
- DifferentialRevision $revision,
- DifferentialDiff $diff) {
-
- $repository = $this->repository;
-
- $vs_diff = id(new DifferentialDiffQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withRevisionIDs(array($revision->getID()))
- ->needChangesets(true)
- ->setLimit(1)
- ->executeOne();
- if (!$vs_diff) {
- return null;
- }
-
- if ($vs_diff->getCreationMethod() == 'commit') {
- return null;
- }
-
- $vs_changesets = array();
- foreach ($vs_diff->getChangesets() as $changeset) {
- $path = $changeset->getAbsoluteRepositoryPath($repository, $vs_diff);
- $path = ltrim($path, '/');
- $vs_changesets[$path] = $changeset;
- }
-
- $changesets = array();
- foreach ($diff->getChangesets() as $changeset) {
- $path = $changeset->getAbsoluteRepositoryPath($repository, $diff);
- $path = ltrim($path, '/');
- $changesets[$path] = $changeset;
- }
-
- if (array_fill_keys(array_keys($changesets), true) !=
- array_fill_keys(array_keys($vs_changesets), true)) {
- return $vs_diff;
- }
-
- $file_phids = array();
- foreach ($vs_changesets as $changeset) {
- $metadata = $changeset->getMetadata();
- $file_phid = idx($metadata, 'new:binary-phid');
- if ($file_phid) {
- $file_phids[$file_phid] = $file_phid;
- }
- }
-
- $files = array();
- if ($file_phids) {
- $files = id(new PhabricatorFileQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withPHIDs($file_phids)
- ->execute();
- $files = mpull($files, null, 'getPHID');
- }
-
- foreach ($changesets as $path => $changeset) {
- $vs_changeset = $vs_changesets[$path];
-
- $file_phid = idx($vs_changeset->getMetadata(), 'new:binary-phid');
- if ($file_phid) {
- if (!isset($files[$file_phid])) {
- return $vs_diff;
- }
- $drequest = DiffusionRequest::newFromDictionary(array(
- 'user' => PhabricatorUser::getOmnipotentUser(),
- 'repository' => $this->repository,
- 'commit' => $this->commit->getCommitIdentifier(),
- 'path' => $path,
- ));
- $corpus = DiffusionFileContentQuery::newFromDiffusionRequest($drequest)
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->loadFileContent()
- ->getCorpus();
- if ($files[$file_phid]->loadFileData() != $corpus) {
- return $vs_diff;
- }
- } else {
- $context = implode("\n", $changeset->makeChangesWithContext());
- $vs_context = implode("\n", $vs_changeset->makeChangesWithContext());
-
- // We couldn't just compare $context and $vs_context because following
- // diffs will be considered different:
- //
- // -(empty line)
- // -echo 'test';
- // (empty line)
- //
- // (empty line)
- // -echo "test";
- // -(empty line)
-
- $hunk = id(new DifferentialModernHunk())->setChanges($context);
- $vs_hunk = id(new DifferentialModernHunk())->setChanges($vs_context);
- if ($hunk->makeOldFile() != $vs_hunk->makeOldFile() ||
- $hunk->makeNewFile() != $vs_hunk->makeNewFile()) {
- return $vs_diff;
- }
- }
- }
-
- return null;
- }
-
private function resolveUserPHID(
PhabricatorRepositoryCommit $commit,
$user_name) {

File Metadata

Mime Type
text/plain
Expires
Wed, Apr 2, 6:34 AM (3 d, 5 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7693115
Default Alt Text
D14968.id36162.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility