When a mercurial working copy has uncommitted changes, prompt to include them in "arc diff"

Summary:
See task. Allow mercurial users to diff with uncommitted changes.

  - By default, commit range is merge-base of `hg outgoing` to `.` (dirstate).
  - You can get JUST dirstate with `arc diff tip` or similar.
  - This ended up being a giant mess various other changes to deal with empty `hg outgoing` and empty dirstate.

Test Plan: Diffed with uncommitted changes, got sensible prompts and results.

Reviewers: Makinde, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T998

Differential Revision: https://secure.phabricator.com/D1954

src/__phutil_library_map__.php

@@ -54,6 +54,7 @@ phutil_register_library_map(array(
     'ArcanistLicenseLinter' => 'lint/linter/license',
     'ArcanistLintEngine' => 'lint/engine/base',
     'ArcanistLintJSONRenderer' => 'lint/renderer',
+    'ArcanistLintLikeCompilerRenderer' => 'lint/renderer',
     'ArcanistLintMessage' => 'lint/message',
     'ArcanistLintPatcher' => 'lint/patcher',
     'ArcanistLintRenderer' => 'lint/renderer',
@@ -91,6 +92,7 @@ phutil_register_library_map(array(
     'ArcanistSvnHookPreCommitWorkflow' => 'workflow/svn-hook-pre-commit',
     'ArcanistTextLinter' => 'lint/linter/text',
     'ArcanistTextLinterTestCase' => 'lint/linter/text/__tests__',
+    'ArcanistUncommittedChangesException' => 'exception/usage/uncommittedchanges',
     'ArcanistUnitTestResult' => 'unit/result',
     'ArcanistUnitWorkflow' => 'workflow/unit',
     'ArcanistUploadWorkflow' => 'workflow/upload',
@@ -168,6 +170,7 @@ phutil_register_library_map(array(
     'ArcanistSvnHookPreCommitWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistTextLinter' => 'ArcanistLinter',
     'ArcanistTextLinterTestCase' => 'ArcanistLinterTestCase',
+    'ArcanistUncommittedChangesException' => 'ArcanistUsageException',
     'ArcanistUnitWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistUploadWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistUserAbortException' => 'ArcanistUsageException',

src/exception/usage/uncommittedchanges/ArcanistUncommittedChangesException.php

@@ -0,0 +1,21 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class ArcanistUncommittedChangesException extends ArcanistUsageException {
+
+}

src/exception/usage/uncommittedchanges/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('arcanist', 'exception/usage');
+
+
+phutil_require_source('ArcanistUncommittedChangesException.php');

src/repository/api/mercurial/ArcanistMercurialAPI.php

@@ -28,6 +28,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   private $relativeCommit;
   private $workingCopyRevision;
   private $localCommitInfo;
+  private $includeDirectoryStateInDiffs;
 
   protected function buildLocalFuture(array $argv) {
 
@@ -53,15 +54,16 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function getSourceControlBaseRevision() {
-    list($stdout) = $this->execxLocal(
-      'log -l 1 --template %s -r %s',
-      '{node}\\n',
-      $this->getRelativeCommit());
-    return rtrim($stdout, "\n");
+    return $this->getCanonicalRevisionName($this->getRelativeCommit());
   }
 
   public function getCanonicalRevisionName($string) {
-    throw new ArcanistCapabilityNotSupportedException($this);
+    list($stdout) = $this->execxLocal(
+      'log -l 1 --template %s -r %s --',
+      '{node}',
+      $this->getRelativeCommit());
+
+    return $stdout;
   }
 
   public function getSourceControlPath() {
@@ -75,28 +77,40 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function setRelativeCommit($commit) {
-    if (!$this->hasLocalCommit($commit)) {
+    try {
+      $commit = $this->getCanonicalRevisionName($commit);
+    } catch (Exception $ex) {
       throw new ArcanistUsageException(
         "Commit '{$commit}' is not a valid Mercurial commit identifier.");
     }
+
     $this->relativeCommit = $commit;
     return $this;
   }
 
   public function getRelativeCommit() {
     if (empty($this->relativeCommit)) {
-      list($stdout) = $this->execxLocal(
+      list($err, $stdout) = $this->execManualLocal(
         'outgoing --branch `hg branch` --style default');
-      $logs = ArcanistMercurialParser::parseMercurialLog($stdout);
-      if (!count($logs)) {
-        throw new ArcanistUsageException("You have no outgoing changes!");
+
+      if (!$err) {
+        $logs = ArcanistMercurialParser::parseMercurialLog($stdout);
+      } else {
+        // Mercurial (in some versions?) raises an error when there's nothing
+        // outgoing.
+        $logs = array();
+      }
+
+      if (!$logs) {
+        // In Mercurial, we support operations against uncommitted changes.
+        return $this->getWorkingCopyRevision();
       }
 
       $outgoing_revs = ipull($logs, 'rev');
 
       // This is essentially an implementation of a theoretical `hg merge-base`
       // command.
-      $against = 'tip';
+      $against = $this->getWorkingCopyRevision();
       while (true) {
         // NOTE: The "^" and "~" syntaxes were only added in hg 1.9, which is
         // new as of July 2011, so do this in a compatible way. Also, "hg log"
@@ -128,7 +142,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
         }
       }
 
-      $this->relativeCommit = $against;
+      $this->setRelativeCommit($against);
     }
     return $this->relativeCommit;
   }
@@ -205,6 +219,12 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
       // just take the entire diff and parse it to figure out what's changed.
 
       $diff = $this->getFullMercurialDiff();
+
+      if (!$diff) {
+        $this->status = array();
+        return $this->status;
+      }
+
       $parser = new ArcanistDiffParser();
       $changes = $parser->parseDiff($diff);
 
@@ -273,7 +293,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
       'diff %C --rev %s --rev %s -- %s',
       $options,
       $this->getRelativeCommit(),
-      $this->getWorkingCopyRevision(),
+      $this->getDiffToRevision(),
       $path);
 
     return $stdout;
@@ -286,7 +306,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
       'diff %C --rev %s --rev %s --',
       $options,
       $this->getRelativeCommit(),
-      $this->getWorkingCopyRevision());
+      $this->getDiffToRevision());
 
     return $stdout;
   }
@@ -341,8 +361,12 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function hasLocalCommit($commit) {
-    list($err) = $this->execManualLocal('id -ir %s', $commit);
-    return !$err;
+    try {
+      $this->getCanonicalRevisionName($commit);
+      return true;
+    } catch (Exception $ex) {
+      return false;
+    }
   }
 
   public function parseRelativeLocalCommit(array $argv) {
@@ -434,4 +458,23 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     $this->execxLocal('up');
   }
 
+  public function setIncludeDirectoryStateInDiffs($include) {
+    $this->includeDirectoryStateInDiffs = $include;
+    return $this;
+  }
+
+  private function getDiffToRevision() {
+
+    // Clear status cache since it's now bogus.
+    $this->status = null;
+
+    if ($this->includeDirectoryStateInDiffs) {
+      // This is a magic Mercurial revision name which means "current
+      // directory state"; see lookup() in localrepo.py.
+      return '.';
+    } else {
+      return $this->getWorkingCopyRevision();
+    }
+  }
+
 }

src/repository/api/mercurial/__init__.php

@@ -11,7 +11,6 @@ phutil_require_module('arcanist', 'parser/diff');
 phutil_require_module('arcanist', 'parser/diff/changetype');
 phutil_require_module('arcanist', 'repository/api/base');
 phutil_require_module('arcanist', 'repository/parser/mercurial');
-phutil_require_module('arcanist', 'workflow/exception/notsupported');
 
 phutil_require_module('phutil', 'future/exec');
 phutil_require_module('phutil', 'utils');

src/workflow/base/ArcanistBaseWorkflow.php

@@ -687,9 +687,9 @@ abstract class ArcanistBaseWorkflow {
 
     $uncommitted = $api->getUncommittedChanges();
     if ($uncommitted) {
-      throw new ArcanistUsageException(
-        "You have uncommitted changes in this branch. Commit (or revert) them ".
-        "before proceeding.\n\n".
+      throw new ArcanistUncommittedChangesException(
+        "You have uncommitted changes in this working copy. Commit (or ".
+        "revert) them before proceeding.\n\n".
         $working_copy_desc.
         "  Uncommitted changes in working copy\n".
         "    ".implode("\n    ", $uncommitted)."\n");

src/workflow/base/__init__.php

@@ -7,6 +7,7 @@
 
 
 phutil_require_module('arcanist', 'exception/usage');
+phutil_require_module('arcanist', 'exception/usage/uncommittedchanges');
 phutil_require_module('arcanist', 'exception/usage/userabort');
 phutil_require_module('arcanist', 'parser/bundle');
 phutil_require_module('arcanist', 'parser/diff');

src/workflow/diff/ArcanistDiffWorkflow.php

@@ -290,6 +290,12 @@ EOTEXT
       'no-amend' => array(
         'help' => 'Never amend commits in the working copy.',
       ),
+      'uncommitted' => array(
+        'help' => 'Include uncommitted changes without prompting.',
+        'supports' => array(
+          'hg',
+        ),
+      ),
       '*' => 'paths',
     );
   }
@@ -501,7 +507,33 @@ EOTEXT
     }
 
     if ($this->requiresWorkingCopy()) {
-      $this->requireCleanWorkingCopy();
+      try {
+        $this->requireCleanWorkingCopy();
+      } catch (ArcanistUncommittedChangesException $ex) {
+        if ($repository_api instanceof ArcanistMercurialAPI) {
+
+          // Some Mercurial users prefer to use it like SVN, where they don't
+          // commit changes before sending them for review. This would be a
+          // pretty bad workflow in Git, but Mercurial users are significantly
+          // more expert at change management.
+
+          $use_dirty_changes = false;
+          if ($this->getArgument('uncommitted')) {
+            // OK.
+          } else {
+            $ok = phutil_console_confirm(
+              "You have uncommitted changes in your working copy. You can ".
+              "include them in the diff, or abort and deal with them. (Use ".
+              "'--uncommitted' to include them and skip this prompt.) ".
+              "Do you want to include uncommitted changes in the diff?");
+            if (!$ok) {
+              throw $ex;
+            }
+          }
+
+          $repository_api->setIncludeDirectoryStateInDiffs(true);
+        }
+      }
     }
   }
 
@@ -693,6 +725,10 @@ EOTEXT
       $changes = $parser->parseDiff($diff);
     } else if ($repository_api instanceof ArcanistMercurialAPI) {
       $diff = $repository_api->getFullMercurialDiff();
+      if (!strlen($diff)) {
+        throw new ArcanistUsageException(
+          "No changes found. (Did you specify the wrong commit range?)");
+      }
       $changes = $parser->parseDiff($diff);
     } else {
       throw new Exception("Repository API is not supported.");