Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15383144
D21325.id50755.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
8 KB
Referenced Files
None
Subscribers
None
D21325.id50755.diff
View Options
diff --git a/src/land/ArcanistLandCommit.php b/src/land/ArcanistLandCommit.php
--- a/src/land/ArcanistLandCommit.php
+++ b/src/land/ArcanistLandCommit.php
@@ -12,6 +12,7 @@
private $parentCommits;
private $isHeadCommit;
private $isImplicitCommit;
+ private $relatedRevisionRefs = array();
private $directSymbols = array();
private $indirectSymbols = array();
@@ -156,5 +157,14 @@
return null;
}
+ public function setRelatedRevisionRefs(array $refs) {
+ assert_instances_of($refs, 'ArcanistRevisionRef');
+ $this->relatedRevisionRefs = $refs;
+ return $this;
+ }
+
+ public function getRelatedRevisionRefs() {
+ return $this->relatedRevisionRefs;
+ }
}
diff --git a/src/land/engine/ArcanistLandEngine.php b/src/land/engine/ArcanistLandEngine.php
--- a/src/land/engine/ArcanistLandEngine.php
+++ b/src/land/engine/ArcanistLandEngine.php
@@ -835,96 +835,76 @@
foreach ($commit_map as $commit) {
$hash = $commit->getHash();
$state_ref = $state_refs[$hash];
-
$revision_refs = $state_ref->getRevisionRefs();
+ $commit->setRelatedRevisionRefs($revision_refs);
+ }
- // If we have several possible revisions but one of them matches the
- // "--revision" argument, just select it. This is relatively safe and
- // reasonable and doesn't need a warning.
+ // For commits which have exactly one related revision, select it now.
- if ($force_ref) {
- if (count($revision_refs) > 1) {
- foreach ($revision_refs as $revision_ref) {
- if ($revision_ref->getPHID() === $force_ref->getPHID()) {
- $revision_refs = array($revision_ref);
- break;
- }
- }
- }
- }
-
- if (count($revision_refs) === 1) {
- $revision_ref = head($revision_refs);
- $commit->setExplicitRevisionRef($revision_ref);
- continue;
- }
+ foreach ($commit_map as $commit) {
+ $revision_refs = $commit->getRelatedRevisionRefs();
- if (!$revision_refs) {
+ if (count($revision_refs) !== 1) {
continue;
}
- // TODO: If we have several refs but all but one are abandoned or closed
- // or authored by someone else, we could guess what you mean.
+ $revision_ref = head($revision_refs);
+ $commit->setExplicitRevisionRef($revision_ref);
+ }
- $symbols = $commit->getIndirectSymbols();
- $raw_symbols = mpull($symbols, 'getSymbol');
- $symbol_list = implode(', ', $raw_symbols);
- $display_hash = $this->getDisplayHash($hash);
+ // If we have a "--revision", select that revision for any commits with
+ // no known related revisions.
- // TODO: Include "use 'arc look --type commit abc' to figure out why"
- // once that works?
+ // Also select that revision for any commits which have several possible
+ // revisions including that revision. This is relatively safe and
+ // reasonable and doesn't require a warning.
- echo tsprintf(
- "\n%!\n%W\n\n",
- pht('AMBIGUOUS REVISION'),
- pht(
- 'The revision associated with commit "%s" (an ancestor of: %s) '.
- 'is ambiguous. These %s revision(s) are associated with the commit:',
- $display_hash,
- implode(', ', $raw_symbols),
- phutil_count($revision_refs)));
+ if ($force_ref) {
+ $force_phid = $force_ref->getPHID();
+ foreach ($commit_map as $commit) {
+ if ($commit->getExplicitRevisionRef()) {
+ continue;
+ }
- foreach ($revision_refs as $revision_ref) {
- echo tsprintf(
- '%s',
- $revision_ref->newDisplayRef());
- }
+ $revision_refs = $commit->getRelatedRevisionRefs();
- echo tsprintf("\n");
+ if ($revision_refs) {
+ $revision_refs = mpull($revision_refs, null, 'getPHID');
+ if (!isset($revision_refs[$force_phid])) {
+ continue;
+ }
+ }
- throw new PhutilArgumentUsageException(
- pht(
- 'Revision for commit "%s" is ambiguous. Use "--revision" to force '.
- 'selection of a particular revision.',
- $display_hash));
+ $commit->setExplicitRevisionRef($force_ref);
+ }
}
- if ($force_ref) {
- $phid_map = array();
- foreach ($commit_map as $commit) {
- $explicit_ref = $commit->getExplicitRevisionRef();
- if ($explicit_ref) {
- $revision_phid = $explicit_ref->getPHID();
- $phid_map[$revision_phid] = $revision_phid;
- }
- }
+ // If we have a "--revision", identify any commits which it is not yet
+ // selected for. These are commits which are not associated with the
+ // identified revision but are associated with one or more other revisions.
+ if ($force_ref) {
$force_phid = $force_ref->getPHID();
- // If none of the commits have a revision, forcing the revision is
- // reasonable and we don't need to confirm it.
+ $confirm_force = array();
+ foreach ($commit_map as $key => $commit) {
+ $revision_ref = $commit->getExplicitRevisionRef();
+
+ if (!$revision_ref) {
+ continue;
+ }
- // If some of the commits have a revision, but it's the same as the
- // revision we're forcing, forcing the revision is also reasonable.
+ if ($revision_ref->getPHID() === $force_phid) {
+ continue;
+ }
- // Discard the revision we're trying to force, then check if there's
- // anything left. If some of the commits have a different revision,
- // make sure the user is really doing what they expect.
+ $confirm_force[] = $commit;
+ }
- unset($phid_map[$force_phid]);
+ if ($confirm_force) {
- if ($phid_map) {
// TODO: Make this more clear.
+ // TODO: Show all the commits.
throw new PhutilArgumentUsageException(
pht(
@@ -933,10 +913,73 @@
'ALL these commits wiht a different unrelated revision???'));
}
- foreach ($commit_map as $commit) {
+ foreach ($confirm_force as $commit) {
$commit->setExplicitRevisionRef($force_ref);
}
}
+
+ // Finally, raise an error if we're left with ambiguous revisions. This
+ // happens when we have no "--revision" and some commits in the range
+ // that are associated with more than one revision.
+
+ $ambiguous = array();
+ foreach ($commit_map as $commit) {
+ if ($commit->getExplicitRevisionRef()) {
+ continue;
+ }
+
+ if (!$commit->getRelatedRevisionRefs()) {
+ continue;
+ }
+
+ $ambiguous[] = $commit;
+ }
+
+ if ($ambiguous) {
+ foreach ($ambiguous as $commit) {
+ $symbols = $commit->getIndirectSymbols();
+ $raw_symbols = mpull($symbols, 'getSymbol');
+ $symbol_list = implode(', ', $raw_symbols);
+ $display_hash = $this->getDisplayHash($hash);
+
+ $revision_refs = $commit->getRelatedRevisionRefs();
+
+ // TODO: Include "use 'arc look --type commit abc' to figure out why"
+ // once that works?
+
+ // TODO: We could print all the ambiguous commits.
+
+ // TODO: Suggest "--pick" as a remedy once it exists?
+
+ echo tsprintf(
+ "\n%!\n%W\n\n",
+ pht('AMBIGUOUS REVISION'),
+ pht(
+ 'The revision associated with commit "%s" (an ancestor of: %s) '.
+ 'is ambiguous. These %s revision(s) are associated with the '.
+ 'commit:',
+ $display_hash,
+ implode(', ', $raw_symbols),
+ phutil_count($revision_refs)));
+
+ foreach ($revision_refs as $revision_ref) {
+ echo tsprintf(
+ '%s',
+ $revision_ref->newDisplayRef());
+ }
+
+ echo tsprintf("\n");
+
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Revision for commit "%s" is ambiguous. Use "--revision" to force '.
+ 'selection of a particular revision.',
+ $display_hash));
+ }
+ }
+
+ // NOTE: We may exit this method with commits that are still unassociated.
+ // These will be handled later by the "implicit commits" mechanism.
}
final protected function getDisplayHash($hash) {
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Mar 15, 3:39 PM (3 d, 5 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7383450
Default Alt Text
D21325.id50755.diff (8 KB)
Attached To
Mode
D21325: Improve the logic for identifying ambiguous commits and applying "--revision" to them
Attached
Detach File
Event Timeline
Log In to Comment