From 3bff9417e9a4ed8a447724e02f1d7402f5908036 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 22 Mar 2012 17:22:52 -0700 Subject: [PATCH] Set relative commit early in "arc diff" Summary: In Mercurial, we figure out if the working copy is dirty with "hg diff", and do a somewhat expensive merge-base / outgoing operation if the relative commit isn't set. We can set the relative commit earlier and avoid some extra work. We also may incorrectly cache this state (diff from merge-base of outgoing to tip) and pass the wrong rev and file dirty list to the linters. Test Plan: Made commits which changed (A, B) and then (A). Ran "arc diff tip^". Before this change, observed full outgoing + merge base resolve and both "A" and "B" passed to lint. Observed execution of fewer commands and lint executing against "A" only after this change. Reviewers: Makinde, btrahan Reviewed By: Makinde CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1998 --- .../api/mercurial/ArcanistMercurialAPI.php | 11 ++++++++--- src/workflow/diff/ArcanistDiffWorkflow.php | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/repository/api/mercurial/ArcanistMercurialAPI.php b/src/repository/api/mercurial/ArcanistMercurialAPI.php index 74a21c4e..57c256f2 100644 --- a/src/repository/api/mercurial/ArcanistMercurialAPI.php +++ b/src/repository/api/mercurial/ArcanistMercurialAPI.php @@ -84,6 +84,8 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { } $this->relativeCommit = $commit; + $this->dropCaches(); + return $this; } @@ -463,9 +465,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { } private function getDiffToRevision() { - - // Clear status cache since it's now bogus. - $this->status = null; + $this->dropCaches(); if ($this->includeDirectoryStateInDiffs) { // This is a magic Mercurial revision name which means "current @@ -476,4 +476,9 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { } } + private function dropCaches() { + $this->status = null; + $this->localCommitInfo = null; + } + } diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index 4041aef8..401f7edd 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -497,6 +497,18 @@ EOTEXT if ($this->getArgument('less-context')) { $repository_api->setDiffLinesOfContext(3); } + + if ($repository_api->supportsRelativeLocalCommits()) { + + // Parse the relative commit as soon as we can, to avoid generating + // caches we need to drop later and expensive discovery operations + // (particularly in Mercurial). + + $relative = $this->getArgument('paths'); + if ($relative) { + $repository_api->parseRelativeLocalCommit($relative); + } + } } $output_json = $this->getArgument('json'); @@ -622,8 +634,6 @@ EOTEXT } } else if ($repository_api->supportsRelativeLocalCommits()) { - $repository_api->parseRelativeLocalCommit( - $this->getArgument('paths', array())); $paths = $repository_api->getWorkingCopyStatus(); } else { throw new Exception("Unknown VCS!");