From b880dfd4c89a53370a8f9f93fc4aebe5cdfce407 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 2 Oct 2012 11:01:49 -0700
Subject: [PATCH] Improve handling of `--background` with `--base`

Summary:
Currently, we run `runDiffSetupBasics()` //after// splitting off background lint and unit tests. However, this means `--base` and any explicit revision name (like "HEAD^") will not be parsed, so the call to `getRelativeCommit()` in order to generate `arc lint --rev XXX` will fail or not work as expected, because it will ignore any arguments.

Instead, parse `--base`, explicit revisions, and other repository API arguments before doing lint and unit tests.

Test Plan:
  - Set global config for `base` to `arc:amended, git:branch-unique(origin/master)`.
  - Created a commit on master.
  - Ran `arc diff HEAD^`.
    - Before this change, the command fails with "Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly." when it attempts to run lint, because the `HEAD^` argument is never parsed. After
    - After this change, the command succeeds.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3574
---
 src/workflow/ArcanistDiffWorkflow.php | 50 ++++++++++++++++-----------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php
index 70496bba..1f9fabd9 100644
--- a/src/workflow/ArcanistDiffWorkflow.php
+++ b/src/workflow/ArcanistDiffWorkflow.php
@@ -368,6 +368,8 @@ EOTEXT
   public function run() {
     $this->console = PhutilConsole::getConsole();
 
+    $this->runRepositoryAPISetup();
+
     if ($this->getArgument('no-diff')) {
       $this->removeScratchFile('diff-result.json');
       $data = $this->runLintUnit();
@@ -387,6 +389,7 @@ EOTEXT
       if (!PhutilConsoleFormatter::getDisableANSI()) {
         array_unshift($argv, '--ansi');
       }
+
       $lint_unit = new ExecFuture(
         'php %s --recon diff --no-diff %Ls',
         phutil_get_library_root('arcanist').'/../scripts/arcanist.php',
@@ -541,29 +544,33 @@ EOTEXT
     return 0;
   }
 
-  private function runDiffSetupBasics() {
-    if ($this->requiresRepositoryAPI()) {
-      $repository_api = $this->getRepositoryAPI();
-      if ($this->getArgument('less-context')) {
-        $repository_api->setDiffLinesOfContext(3);
-      }
-
-      $repository_api->setBaseCommitArgumentRules(
-        $this->getArgument('base', ''));
-
-      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);
-        }
-      }
+  private function runRepositoryAPISetup() {
+    if (!$this->requiresRepositoryAPI()) {
+      return;
     }
 
+    $repository_api = $this->getRepositoryAPI();
+    if ($this->getArgument('less-context')) {
+      $repository_api->setDiffLinesOfContext(3);
+    }
+
+    $repository_api->setBaseCommitArgumentRules(
+      $this->getArgument('base', ''));
+
+    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);
+      }
+    }
+  }
+
+  private function runDiffSetupBasics() {
     $output_json = $this->getArgument('json');
     if ($output_json) {
       // TODO: We should move this to a higher-level and put an indirection
@@ -1230,6 +1237,7 @@ EOTEXT
         $argv[] = '--rev';
         $argv[] = $repository_api->getRelativeCommit();
       }
+
       $lint_workflow = $this->buildChildWorkflow('lint', $argv);
 
       if ($this->shouldAmend()) {