Implement "arc:amended" and fix various problems with ArcanistMercurialAPI

Summary:
  - Implement "arc:amended", a base commit DSL rule which always selects HEAD (git) or `.` (hg) if it has "differential revision:" in the commit message. This is unambiguously correct in amend workflows, and can cover holes in other rules like "git:branch-unique(*)".
  - Fix a bunch of Mercurial stuff:
    - Our use of '.' is wrong, and based on a misunderstanding on my part of the behavior of `hg diff --rev . --rev .`, which means "ignore the second --rev flag", not ". means working directory state". As far as I know there's no explicit way to say "the working copy plus all its changes".
    - The `--prune` argument to "hg log" does not support symbolic names like ".^". Use revsets instead.
    - Reduce the number of times we need to run `hg branch`.
    - We can safely use "." to mean "the working copy revision", and do not need to do "hg --debug id" or similar.
    - Generally simplify some of the nonsense in the implementation left over from me having no idea how Mercurial works.

Test Plan:
Ran "arc which" in various scenarios in a mercurial working copy. I //think// I exercised all the changes.

Ran "arc which --base arc:amended" in hg and git working copies without "Differential Revision:" in head/. (no match) and with it (matched head/.).

Reviewers: dschleimer

Reviewed By: dschleimer

CC: Makinde, tido, phleet, aran

Maniphest Tasks: T1233

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

src/parser/ArcanistBaseCommitParser.php

@@ -166,6 +166,7 @@ final class ArcanistBaseCommitParser {
       case 'upstream':
       case 'outgoing':
       case 'bookmark':
+      case 'amended':
         return $this->api->resolveBaseCommitRule($rule, $source);
       default:
         $matches = null;

src/repository/api/ArcanistGitAPI.php

@@ -752,12 +752,12 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
            "'git push', or 'git svn dcommit', or by printing and faxing it).";
   }
 
-  public function getCommitMessageForRevision($rev) {
+  public function getCommitMessage($commit) {
     list($message) = $this->execxLocal(
-      'log -n1 %s',
+      'log -n1 --format=%C %s --',
-      $rev);
+      '%s%n%b',
-    $parser = new ArcanistDiffParser();
+      $commit);
-    return head($parser->parseDiff($message));
+    return $message;
   }
 
   public function loadWorkingCopyDifferentialRevisions(
@@ -921,6 +921,18 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
               "you specified '{$rule}' in your {$source} 'base' ".
               "configuration.");
             return self::GIT_MAGIC_ROOT_COMMIT;
+          case 'amended':
+            $text = $this->getCommitMessage('HEAD');
+            $message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
+              $text);
+            if ($message->getRevisionID()) {
+              $this->setBaseCommitExplanation(
+                "HEAD has been amended with 'Differential Revision:', ".
+                "as specified by '{$rule}' in your {$source} 'base' ".
+                "configuration.");
+              return 'HEAD^';
+            }
+            break;
           case 'upstream':
             list($err, $upstream) = $this->execManualLocal(
               "rev-parse --abbrev-ref --symbolic-full-name '@{upstream}'");

src/repository/api/ArcanistMercurialAPI.php

@@ -26,6 +26,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   private $status;
   private $base;
   private $relativeCommit;
+  private $branch;
   private $workingCopyRevision;
   private $localCommitInfo;
   private $includeDirectoryStateInDiffs;
@@ -78,9 +79,11 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function getBranchName() {
-    // TODO: I have nearly no idea how hg branches work.
+    if (!$this->branch) {
       list($stdout) = $this->execxLocal('branch');
-    return trim($stdout);
+      $this->branch = trim($stdout);
+    }
+    return $this->branch;
   }
 
   public function setRelativeCommit($commit) {
@@ -92,7 +95,8 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     }
 
     $this->relativeCommit = $commit;
-    $this->dropCaches();
+    $this->status = null;
+    $this->localCommitInfo = null;
 
     return $this;
   }
@@ -114,7 +118,8 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
       }
 
       list($err, $stdout) = $this->execManualLocal(
-        'outgoing --branch `hg branch` --style default');
+        'outgoing --branch %s --style default',
+        $this->getBranchName());
 
       if (!$err) {
         $logs = ArcanistMercurialParser::parseMercurialLog($stdout);
@@ -187,11 +192,10 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   public function getLocalCommitInformation() {
     if ($this->localCommitInfo === null) {
       list($info) = $this->execxLocal(
-        "log --template '%C' --prune %s --rev %s --branch %s --",
+        "log --template '%C' --rev %s --branch %s --",
         "{node}\1{rev}\1{author}\1{date|rfc822date}\1".
           "{branch}\1{tag}\1{parents}\1{desc}\2",
-        $this->getRelativeCommit(),
+        '(ancestors(.) - ancestors('.$this->getRelativeCommit().'))',
-        "ancestors({$this->getWorkingCopyRevision()})",
         $this->getBranchName());
       $logs = array_filter(explode("\2", $info));
 
@@ -356,26 +360,27 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   public function getRawDiffText($path) {
     $options = $this->getDiffOptions();
 
+    // NOTE: In Mercurial, "--rev x" means "diff between x and the working
+    // copy state", while "--rev x..." means "diff between x and the working
+    // copy commit" (i.e., from 'x' to '.'). The latter excludes any dirty
+    // changes in the working copy.
+
+    $range = $this->getRelativeCommit();
+    if (!$this->includeDirectoryStateInDiffs) {
+      $range .= '...';
+    }
+
     list($stdout) = $this->execxLocal(
-      'diff %C --rev %s --rev %s -- %s',
+      'diff %C --rev %s -- %s',
       $options,
-      $this->getRelativeCommit(),
+      $range,
-      $this->getDiffToRevision(),
       $path);
 
     return $stdout;
   }
 
   public function getFullMercurialDiff() {
-    $options = $this->getDiffOptions();
+    return $this->getRawDiffText('');
-
-    list($stdout) = $this->execxLocal(
-      'diff %C --rev %s --rev %s --',
-      $options,
-      $this->getRelativeCommit(),
-      $this->getDiffToRevision());
-
-    return $stdout;
   }
 
   public function getOriginalFileData($path) {
@@ -402,25 +407,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function getWorkingCopyRevision() {
-    if ($this->workingCopyRevision === null) {
+    return '.';
-      // In Mercurial, "tip" means the tip of the current branch, not what's in
-      // the working copy. The tip may be ahead of the working copy. We need to
-      // use "hg summary" to figure out what is actually in the working copy.
-      // For instance, "hg up 4 && arc diff" should not show commits 5 and
-      // above.
-
-      // Without arguments, "hg id" shows the current working directory's
-      // commit, and "--debug" expands it to a 40-character hash.
-      list($stdout) = $this->execxLocal('--debug id --id');
-
-      // Even with "--id", "hg id" will print a trailing "+" after the hash
-      // if the working copy is dirty (has uncommitted changes). We'll
-      // explicitly detect this later by calling getWorkingCopyStatus(); ignore
-      // it for now.
-      $stdout = trim($stdout);
-      $this->workingCopyRevision = rtrim($stdout, '+');
-    }
-    return $this->workingCopyRevision;
   }
 
   public function isHistoryDefaultImmutable() {
@@ -449,6 +436,13 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     }
   }
 
+  public function getCommitMessage($commit) {
+    list($message) = $this->execxLocal(
+      'log --template={desc} --rev %s',
+      $commit);
+    return $message;
+  }
+
   public function parseRelativeLocalCommit(array $argv) {
     if (count($argv) == 0) {
       return;
@@ -503,15 +497,15 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
 
   public function getCommitMessageLog() {
     list($stdout) = $this->execxLocal(
-      "log --template '{node}\\1{desc}\\0' --prune %s --rev %s --",
+      "log --template '{node}\\2{desc}\\1' --rev %s --branch %s --",
-      $this->getRelativeCommit(),
+      'ancestors(.) - ancestors('.$this->getRelativeCommit().')',
-      "ancestors({$this->getWorkingCopyRevision()})");
+      $this->getBranchName());
 
     $map = array();
 
-    $logs = explode("\0", trim($stdout));
+    $logs = explode("\1", trim($stdout));
     foreach (array_filter($logs) as $log) {
-      list($node, $desc) = explode("\1", $log);
+      list($node, $desc) = explode("\2", $log);
       $map[$node] = $desc;
     }
 
@@ -591,23 +585,6 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     return $this;
   }
 
-  private function getDiffToRevision() {
-    $this->dropCaches();
-
-    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();
-    }
-  }
-
-  private function dropCaches() {
-    $this->status = null;
-    $this->localCommitInfo = null;
-  }
-
   public function getCommitSummary($commit) {
     if ($commit == 'null') {
       return '(The Empty Void)';
@@ -670,6 +647,20 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
                 "'base' configuration.");
               return trim($outgoing_base);
             }
+          case 'amended':
+            $text = $this->getCommitMessage('.');
+            $message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
+              $text);
+            if ($message->getRevisionID()) {
+              $this->setBaseCommitExplanation(
+                "'.' has been amended with 'Differential Revision:', ".
+                "as specified by '{$rule}' in your {$source} 'base' ".
+                "configuration.");
+              // NOTE: This should be safe because Mercurial doesn't support
+              // amend until 2.2.
+              return '.^';
+            }
+            break;
           case 'bookmark':
             $revset =
               'limit('.

src/repository/api/ArcanistRepositoryAPI.php

@@ -195,7 +195,7 @@ abstract class ArcanistRepositoryAPI {
     throw new ArcanistCapabilityNotSupportedException($this);
   }
 
-  public function getCommitMessageForRevision($revision) {
+  public function getCommitMessage($commit) {
     throw new ArcanistCapabilityNotSupportedException($this);
   }
 

src/workflow/ArcanistDiffWorkflow.php

@@ -1280,10 +1280,9 @@ EOTEXT
   /**
    * @task message
    */
-  private function getCommitMessageFromCommit($rev) {
+  private function getCommitMessageFromCommit($commit) {
-    $change = $this->getRepositoryAPI()->getCommitMessageForRevision($rev);
+    $text = $this->getRepositoryAPI()->getCommitMessage($commit);
-    $message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
+    $message = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
-      $change->getMetadata('message'));
     $message->pullDataFromConduit($this->getConduit());
     $this->validateCommitMessage($message);
     return $message;