Create fix and test for hg merge.

Summary:
Running Mercurial's own integration tests revealed that we had a bug here:
https://www.mercurial-scm.org/repo/hg/file/tip/tests/test-histedit-arguments.t

Somewhat unsurprisingly, it was time to finally address a longstanding `TODO`
in `Dirstate.cpp`. The issue was that, after running `hg merge --tool :local`,
`hg status` was not including a merged file in the list of modified files. Because
the information from `hg status` is used to create a commit context, that meant
that when a commit was made after running `hg merge`, the commit did not
include the merged file in the list of files for the commit, which differs from
Mercurial's behavior.

Most of the implementation of `hg status` on the Eden side is done by
`EdenMount.diff()`. However, in this case, `diff()` does not categorize the
merged file by invoking one of the methods of `InodeDiffCallback` because
as far as `EdenMount` is concerned, the file has not changed because `EdenMount`
is unaware of the `Dirstate`. We already have some existing cases where we have
to do some post-processing on the result of `EdenMount.diff()` using information
in the `Dirstate` (e.g., files that are marked for addition or removal), so the fix was
to add a check for the case when the file is flagged as "needs merging" and
then including it as modified in the `hg status` output, as appropriate.

Reviewed By: wez

Differential Revision: D6005603

fbshipit-source-id: 7d4dd80e1a2e9f4b98243da80989e0e9119a566d

eden/fs/inodes/Dirstate.cpp

@@ -26,14 +26,14 @@
 #include "eden/fs/inodes/TreeInode.h"
 #include "eden/fs/store/ObjectStore.h"
 
+using facebook::eden::hgdirstate::DirstateMergeState;
+using facebook::eden::hgdirstate::DirstateNonnormalFileStatus;
+using facebook::eden::hgdirstate::DirstateTuple;
 using folly::Future;
-using folly::makeFuture;
 using folly::StringKeyedUnorderedMap;
 using folly::StringPiece;
 using folly::Unit;
-using facebook::eden::hgdirstate::DirstateNonnormalFileStatus;
-using facebook::eden::hgdirstate::DirstateMergeState;
-using facebook::eden::hgdirstate::DirstateTuple;
+using folly::makeFuture;
 using std::string;
 
 namespace facebook {
@@ -140,17 +140,30 @@ class ThriftStatusCallback : public InodeDiffCallback {
       // - UserStatusDirective::Remove, not on disk, not in source control:
       //   -> skip
       for (const auto& entry : data->hgDirstateTuples) {
-        auto nnFileStatus = entry.second.get_status();
-        if (nnFileStatus != DirstateNonnormalFileStatus::MarkedForAddition &&
-            nnFileStatus != DirstateNonnormalFileStatus::MarkedForRemoval) {
-          // TODO(mbolin): Handle this case.
-          continue;
+        switch (entry.second.get_status()) {
+          case DirstateNonnormalFileStatus::NotTracked:
+          case DirstateNonnormalFileStatus::Normal:
+            continue;
+          case DirstateNonnormalFileStatus::NeedsMerging:
+            if (entry.second.get_mergeState() ==
+                DirstateMergeState::NotApplicable) {
+              XLOG(ERR) << "Unexpected Nonnormal file " << entry.first
+                        << " has a merge state of NotApplicable while its "
+                        << "NonnormalFileStatus is NeedsMerging.";
+            } else {
+              status.entries.emplace(entry.first.str(), StatusCode::MODIFIED);
+            }
+            break;
+          case DirstateNonnormalFileStatus::MarkedForAddition:
+            status.entries.emplace(entry.first.str(), StatusCode::MISSING);
+            break;
+          case DirstateNonnormalFileStatus::MarkedForRemoval:
+            status.entries.emplace(entry.first.str(), StatusCode::REMOVED);
+            break;
+          default:
+            throw std::runtime_error(folly::to<string>(
+                "Unexpected file status: ", entry.second.get_status()));
         }
-        auto hgStatusCode =
-            (nnFileStatus == DirstateNonnormalFileStatus::MarkedForAddition)
-            ? StatusCode::MISSING
-            : StatusCode::REMOVED;
-        status.entries.emplace(entry.first.str(), hgStatusCode);
       }
     }
 
@@ -223,7 +236,7 @@ static bool isMagicPath(RelativePathPiece path) {
   }
   return false;
 }
-}
+} // namespace
 
 Future<Unit> Dirstate::onSnapshotChanged(const Tree* rootTree) {
   XLOG(INFO) << "Dirstate::onSnapshotChanged(" << rootTree->getHash() << ")";
@@ -431,5 +444,5 @@ std::ostream& operator<<(std::ostream& os, const ThriftHgStatus& status) {
   os << "}";
   return os;
 }
-}
-}
+} // namespace eden
+} // namespace facebook

eden/integration/hg/merge_test.py

@@ -0,0 +1,56 @@
+#!/usr/bin/env python3
+#
+# Copyright (c) 2016-present, Facebook, Inc.
+# All rights reserved.
+#
+# This source code is licensed under the BSD-style license found in the
+# LICENSE file in the root directory of this source tree. An additional grant
+# of patent rights can be found in the PATENTS file in the same directory.
+
+from .lib.hg_extension_test_base import hg_test
+
+
+@hg_test
+class MergeTest:
+    '''Note that Mercurial has a number of built-in merge tools:
+    https://www.mercurial-scm.org/repo/hg/help/merge-tools
+    '''
+    def populate_backing_repo(self, repo):
+        repo.write_file('foo', 'original')
+        self.commit0 = repo.commit('root commit')
+
+        repo.write_file('foo', '1')
+        self.commit1 = repo.commit('commit1')
+        repo.update(self.commit0)
+
+        repo.write_file('foo', '2')
+        self.commit2 = repo.commit('commit2')
+
+    def test_merge_local(self):
+        self._do_merge_and_commit(':local')
+        self._verify_tip('2')
+
+    def test_merge_other(self):
+        self._do_merge_and_commit(':other')
+        self._verify_tip('1')
+
+    def test_merge_union(self):
+        self._do_merge_and_commit(':union')
+        self._verify_tip('21')
+
+    def _do_merge_and_commit(self, tool):
+        self.hg('merge', '--tool', tool, '-r', self.commit1)
+        self.assert_status({'foo': 'M'})
+        self.repo.commit('merge commit1 into commit2')
+        self.assert_status_empty()
+
+    def _verify_tip(self, expected_contents):
+        files = self.repo.log(template='{files}', revset='tip')[0]
+        self.assertEqual('foo', files)
+
+        p1, p2 = self.repo.log(
+            template='{p1node}\n{p2node}', revset='tip'
+        )[0].split('\n')
+        self.assertEqual(self.commit2, p1)
+        self.assertEqual(self.commit1, p2)
+        self.assertEqual(expected_contents, self.read_file('foo'))