diff --git a/src/land/ArcanistLandCommit.php b/src/land/ArcanistLandCommit.php index 527611ce..b00c988a 100644 --- a/src/land/ArcanistLandCommit.php +++ b/src/land/ArcanistLandCommit.php @@ -7,13 +7,15 @@ final class ArcanistLandCommit private $summary; private $displaySummary; private $parents; - private $symbols = array(); private $explicitRevisionRef; private $revisionRef = false; private $parentCommits; private $isHeadCommit; private $isImplicitCommit; + private $directSymbols = array(); + private $indirectSymbols = array(); + public function setHash($hash) { $this->hash = $hash; return $this; @@ -50,13 +52,22 @@ final class ArcanistLandCommit return $this->parents; } - public function addSymbol(ArcanistLandSymbol $symbol) { - $this->symbols[] = $symbol; + public function addDirectSymbol(ArcanistLandSymbol $symbol) { + $this->directSymbols[] = $symbol; return $this; } - public function getSymbols() { - return $this->symbols; + public function getDirectSymbols() { + return $this->directSymbols; + } + + public function addIndirectSymbol(ArcanistLandSymbol $symbol) { + $this->indirectSymbols[] = $symbol; + return $this; + } + + public function getIndirectSymbols() { + return $this->indirectSymbols; } public function setExplicitRevisionref(ArcanistRevisionRef $ref) { diff --git a/src/land/engine/ArcanistGitLandEngine.php b/src/land/engine/ArcanistGitLandEngine.php index 9e039d44..74a17ad4 100644 --- a/src/land/engine/ArcanistGitLandEngine.php +++ b/src/land/engine/ArcanistGitLandEngine.php @@ -1299,6 +1299,7 @@ final class ArcanistGitLandEngine } $commits = phutil_split_lines($commits, false); + $is_first = false; foreach ($commits as $line) { if (!strlen($line)) { continue; @@ -1331,7 +1332,12 @@ final class ArcanistGitLandEngine } $commit = $commit_map[$hash]; - $commit->addSymbol($symbol); + if ($is_first) { + $commit->addDirectSymbol($symbol); + $is_first = false; + } + + $commit->addIndirectSymbol($symbol); } } diff --git a/src/land/engine/ArcanistLandEngine.php b/src/land/engine/ArcanistLandEngine.php index 7343c660..fa613dec 100644 --- a/src/land/engine/ArcanistLandEngine.php +++ b/src/land/engine/ArcanistLandEngine.php @@ -866,7 +866,7 @@ abstract class ArcanistLandEngine extends Phobject { // 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. - $symbols = $commit->getSymbols(); + $symbols = $commit->getIndirectSymbols(); $raw_symbols = mpull($symbols, 'getSymbol'); $symbol_list = implode(', ', $raw_symbols); $display_hash = $this->getDisplayHash($hash); @@ -899,11 +899,6 @@ abstract class ArcanistLandEngine extends Phobject { $display_hash)); } - // TODO: Some of the revisions we've identified may be mapped to an - // outdated set of commits. We should look in local branches for a better - // set of commits, and try to confirm that the state we're about to land - // is the current state in Differential. - if ($force_ref) { $phid_map = array(); foreach ($commit_map as $commit) { @@ -1155,6 +1150,8 @@ abstract class ArcanistLandEngine extends Phobject { $sets[$revision_phid] = $set; } + $sets = $this->filterCommitSets($sets); + if (!$this->getShouldPreview()) { $this->confirmImplicitCommits($sets, $symbols); } @@ -1436,4 +1433,62 @@ abstract class ArcanistLandEngine extends Phobject { return $strategy; } + private function filterCommitSets(array $sets) { + assert_instances_of($sets, 'ArcanistLandCommitSet'); + $log = $this->getLogEngine(); + + // If some of the ancestor revisions are already closed, and the user did + // not specifically indicate that we should land them, and we are using + // a "squash" strategy, discard those sets. + + if ($this->isSquashStrategy()) { + $discard = array(); + foreach ($sets as $key => $set) { + $revision_ref = $set->getRevisionRef(); + + if (!$revision_ref->isClosed()) { + continue; + } + + $symbols = null; + foreach ($set->getCommits() as $commit) { + $commit_symbols = $commit->getDirectSymbols(); + if ($commit_symbols) { + $symbols = $commit_symbols; + break; + } + } + + if ($symbols) { + continue; + } + + $discard[] = $set; + unset($sets[$key]); + } + + if ($discard) { + echo tsprintf( + "\n%!\n%W\n", + pht('DISCARDING ANCESTORS'), + pht( + 'Some ancestor commits are associated with revisions that have '. + 'already been closed. These changes will be skipped:')); + + foreach ($discard as $set) { + $this->printCommitSet($set); + } + + echo tsprintf("\n"); + } + } + + // TODO: Some of the revisions we've identified may be mapped to an + // outdated set of commits. We should look in local branches for a better + // set of commits, and try to confirm that the state we're about to land + // is the current state in Differential. + + return $sets; + } + }