1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-29 10:12:41 +01:00

Make "arc diff" sort of detect dependent revisions

Summary:
Depends on D18645. Fixes T11343. Currently, when you "arc diff", we don't detect dependencies even if your working copy is on top of another open revision you authored.

Use the new Ref/Hardpoint infrastructure from the `experimental` branch to look up the revision associated with the base commit when you run `arc diff`. If it exists and there's exactly one open revision you authored associated with it, add "Depends on ..." to the summary. This is a bit rough but the whole `experimental` branch is a bit of a wilderness for now.

Also remove `--cache` (passthru flag for `arc lint --cache`, which I removed in D18643) and `getUnderlyingWorkingCopyRevision()` (no callsites).

(This won't yet work in Mercurial, and does not make sense in SVN.)

Test Plan: Created this revision, got an auto "depends on" up top there.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11343

Differential Revision: https://secure.phabricator.com/D18651
This commit is contained in:
epriestley 2017-09-27 09:05:09 -07:00
parent 7af60b07cb
commit 5ddc573223
5 changed files with 70 additions and 24 deletions

View file

@ -56,6 +56,10 @@ final class ArcanistRevisionRef
return idx($this->parameters, 'title'); return idx($this->parameters, 'title');
} }
public function getAuthorPHID() {
return idx($this->parameters, 'authorPHID');
}
public function addSource(ArcanistRevisionRefSource $source) { public function addSource(ArcanistRevisionRefSource $source) {
$this->sources[] = $source; $this->sources[] = $source;
return $this; return $this;

View file

@ -1053,19 +1053,22 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
return $refs; return $refs;
} }
public function getBaseCommitRef() {
$base_commit = $this->getSourceControlBaseRevision();
$base_message = $this->getCommitMessage($base_commit);
// TODO: We should also pull the tree hash.
return $this->newCommitRef()
->setCommitHash($base_commit)
->attachMessage($base_message);
}
public function getWorkingCopyRevision() { public function getWorkingCopyRevision() {
list($stdout) = $this->execxLocal('rev-parse HEAD'); list($stdout) = $this->execxLocal('rev-parse HEAD');
return rtrim($stdout, "\n"); return rtrim($stdout, "\n");
} }
public function getUnderlyingWorkingCopyRevision() {
list($err, $stdout) = $this->execManualLocal('svn find-rev HEAD');
if (!$err && $stdout) {
return rtrim($stdout, "\n");
}
return $this->getWorkingCopyRevision();
}
public function isHistoryDefaultImmutable() { public function isHistoryDefaultImmutable() {
return false; return false;
} }

View file

@ -343,11 +343,6 @@ abstract class ArcanistRepositoryAPI extends Phobject {
array $query); array $query);
abstract public function getRemoteURI(); abstract public function getRemoteURI();
public function getUnderlyingWorkingCopyRevision() {
return $this->getWorkingCopyRevision();
}
public function getChangedFiles($since_commit) { public function getChangedFiles($since_commit) {
throw new ArcanistCapabilityNotSupportedException($this); throw new ArcanistCapabilityNotSupportedException($this);
} }
@ -379,6 +374,10 @@ abstract class ArcanistRepositoryAPI extends Phobject {
throw new ArcanistCapabilityNotSupportedException($this); throw new ArcanistCapabilityNotSupportedException($this);
} }
public function getBaseCommitRef() {
throw new ArcanistCapabilityNotSupportedException($this);
}
public function hasLocalCommit($commit) { public function hasLocalCommit($commit) {
throw new ArcanistCapabilityNotSupportedException($this); throw new ArcanistCapabilityNotSupportedException($this);
} }

View file

@ -393,16 +393,6 @@ EOTEXT
), ),
'supports' => array('git', 'hg'), 'supports' => array('git', 'hg'),
), ),
'cache' => array(
'param' => 'bool',
'help' => pht(
'%d to disable lint cache, %d to enable (default).',
0,
1),
'passthru' => array(
'lint' => true,
),
),
'coverage' => array( 'coverage' => array(
'help' => pht('Always enable coverage information.'), 'help' => pht('Always enable coverage information.'),
'conflicts' => array( 'conflicts' => array(
@ -1950,6 +1940,15 @@ EOTEXT
$faux_message[] = pht('CC: %s', $this->getArgument('cc')); $faux_message[] = pht('CC: %s', $this->getArgument('cc'));
} }
// NOTE: For now, this isn't a real field, so it just ends up as the first
// part of the summary.
$depends_ref = $this->getDependsOnRevisionRef();
if ($depends_ref) {
$faux_message[] = pht(
'Depends on %s. ',
$depends_ref->getMonogram());
}
// See T12069. After T10312, the first line of a message is always parsed // See T12069. After T10312, the first line of a message is always parsed
// as a title. Add a placeholder so "Reviewers" and "CC" are never the // as a title. Add a placeholder so "Reviewers" and "CC" are never the
// first line. // first line.
@ -2904,4 +2903,45 @@ EOTEXT
} }
} }
private function getDependsOnRevisionRef() {
$api = $this->getRepositoryAPI();
$base_ref = $api->getBaseCommitRef();
$state_ref = $this->newWorkingCopyStateRef()
->setCommitRef($base_ref);
$this->newRefQuery(array($state_ref))
->needHardpoints(
array(
'revisionRefs',
))
->execute();
$revision_refs = $state_ref->getRevisionRefs();
$viewer_phid = $this->getUserPHID();
foreach ($revision_refs as $key => $revision_ref) {
// Don't automatically depend on closed revisions.
if ($revision_ref->isClosed()) {
unset($revision_refs[$key]);
continue;
}
// Don't automatically depend on revisions authored by other users.
if ($revision_ref->getAuthorPHID() != $viewer_phid) {
continue;
}
}
if (!$revision_refs) {
return null;
}
if (count($revision_refs) > 1) {
return null;
}
return head($revision_refs);
}
} }

View file

@ -201,7 +201,7 @@ EOTEXT
// This is used so that the lint engine can drop warning messages // This is used so that the lint engine can drop warning messages
// concerning lines that weren't in the change. // concerning lines that weren't in the change.
$engine->setPaths($paths); $engine->setPaths($paths);
if ($lint_all) { if (!$lint_all) {
foreach ($paths as $path) { foreach ($paths as $path) {
// Note that getChangedLines() returns null to indicate that a file // Note that getChangedLines() returns null to indicate that a file
// is binary or a directory (i.e., changed lines are not relevant). // is binary or a directory (i.e., changed lines are not relevant).