Fix for hg split in Eden.

Summary:
Before this change, `hg split` crashed complaining that `node` was a
`changectxwrapper` instead of a 20-byte hash when it was sent as `parent1`
of `WorkingDirectoryParents` in `resetParentCommits()`. Now we use `node()` to
get the hash from the `destctx` that we have already extracted via this line
earlier in `merge_update()`:

    destctx = repo[node]

The change to `eden/hg/eden/__init__.py` eliminated the crash, but was
not sufficient on its own to make `hg split` work correctly. There was also a fix
required in `Dirstate.cpp` where the `onSnapshotChanged()` callback was clearing out
entries of both `NotApplicable` and `BothParents` from `hgDirstateTuples`.
It appears that only `NotApplicable` entries should be cleared. (I tried leaving
`NotApplicable` entries in there, but that broke `eden/integration/hg/graft_test.py`.)

I suspected that the logic to clear out `hgDestToSourceCopyMap` in
`Dirstate::onSnapshotChanged` was also wrong, so I deleted it and all of the
integration tests still pass. Admittedly, we are pretty weak in our test coverage
for use cases that write to the `hgDestToSourceCopyMap`. In general, we should
rely on Mercurial to explicitly remove entries from `hgDestToSourceCopyMap`.
We have a Thrift API, `hgClearDirstate()`, that `eden_dirstate` can use to categorically
clear out `hgDirstateTuples` and `hgDestToSourceCopyMap`, if necessary.

Finally, creating a proper integration test for `hg split` required creating a value for
`HGEDITOR` that could write different commit messages for different commits.
To that end, I added a `create_editor_that_writes_commit_messages()` utility as a
method of `HgExtensionTestBase` and updated its `hg()` method to take `hgeditor`
as an optional parameter.

Reviewed By: wez

Differential Revision: D5758236

fbshipit-source-id: 5cb8bf4207d4e802726cd93108fae4a6d48f45ec

eden/fs/inodes/Dirstate.cpp

@@ -232,16 +232,8 @@ Future<Unit> Dirstate::onSnapshotChanged(const Tree* rootTree) {
     auto data = data_.wlock();
     bool madeChanges = false;
 
-    if (!data->hgDestToSourceCopyMap.empty()) {
-      // For now, we blindly assume that when the snapshot changes, the copymap
-      // data is no longer valid.
-      data->hgDestToSourceCopyMap.clear();
-      madeChanges = true;
-    }
-
-    // For now, we also blindly assume that when the snapshot changes, we can
-    // remove all dirstate tuples except for those that have a merge state of
-    // OtherParent.
+    // For now, we assume that when the snapshot changes, we should
+    // remove all dirstate tuples with a merge state of NotApplicable.
     auto iter = data->hgDirstateTuples.begin();
     while (iter != data->hgDirstateTuples.end()) {
       // If we need to erase this element, it will erase iterators pointing to
@@ -249,7 +241,12 @@ Future<Unit> Dirstate::onSnapshotChanged(const Tree* rootTree) {
       auto current = iter;
       ++iter;
 
-      if (current->second.get_mergeState() != DirstateMergeState::OtherParent) {
+      if (current->second.get_mergeState() ==
+          DirstateMergeState::NotApplicable) {
+        XLOG(INFO) << "Removing " << current->first.str()
+                   << " from hgDirstateTuples via onSnapshotChanged("
+                   << rootTree->getHash()
+                   << ") because it has merge state NotApplicable.";
         data->hgDirstateTuples.erase(current);
         madeChanges = true;
       }

eden/integration/hg/lib/hg_extension_test_base.py

@@ -7,6 +7,8 @@
 # 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 textwrap import dedent
+from typing import List
 from ...lib import find_executables, hgrepo, testcase
 import configparser
 import json
@@ -131,13 +133,63 @@ class HgExtensionTestBase(testcase.EdenTestCase):
         with open(edenrc, 'w') as f:
             config.write(f)
 
-    def hg(self, *args, stdout_charset='utf-8'):
+    def hg(self, *args, stdout_charset='utf-8', shell=False, hgeditor=None):
         '''Runs `hg.real` with the specified args in the Eden mount.
 
+        If hgeditor is specified, it will be used as the value of the $HGEDITOR
+        environment variable when the hg command is run. See
+        self.create_editor_that_writes_commit_messages().
+
         Returns the stdout decoded as a utf8 string. To use a different charset,
         specify the `stdout_charset` as a keyword argument.
         '''
-        return self.repo.hg(*args, stdout_charset=stdout_charset)
+        return self.repo.hg(*args, stdout_charset=stdout_charset,
+                            shell=shell, hgeditor=hgeditor)
+
+    def create_editor_that_writes_commit_messages(self,
+                                                  messages: List[str]) -> str:
+        '''
+        Creates a program that writes the next message in `messages` to the
+        file specified via $1 each time it is invoked.
+
+        Returns the path to the program. This is intended to be used as the
+        value for hgeditor in self.hg().
+        '''
+        tmp_dir = self.tmp_dir
+
+        messages_dir = os.path.join(tmp_dir, 'commit_messages')
+        os.makedirs(messages_dir)
+        for i, message in enumerate(messages):
+            file_name = '{:04d}'.format(i)
+            with open(os.path.join(messages_dir, file_name), 'w') as f:
+                f.write(message)
+
+        editor = os.path.join(tmp_dir, 'commit_message_editor')
+
+        # Each time this script runs, it takes the "first" message file that is
+        # left in messages_dir and moves it to overwrite the path that it was
+        # asked to edit. This makes it so that the next time it runs, it will
+        # use the "next" message in the queue.
+        with open(editor, 'w') as f:
+            f.write(
+                dedent(
+                    f'''\
+            #!/bin/bash
+            set -e
+
+            for entry in {messages_dir}/*
+            do
+                mv "$entry" "$1"
+                exit 0
+            done
+
+            # There was no message to write.
+            exit 1
+            '''
+                )
+            )
+        os.chmod(editor, 0o755)
+        return editor
 
     def status(self):
         '''Returns the output of `hg status` as a string.'''

eden/integration/hg/split_test.py

@@ -0,0 +1,61 @@
+#!/usr/bin/env python3
+#
+# Copyright (c) 2004-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 textwrap import dedent
+from .lib.hg_extension_test_base import hg_test
+
+
+@hg_test
+class SplitTest:
+    def populate_backing_repo(self, repo):
+        repo.write_file('letters', 'a\nb\nc\n')
+        repo.write_file('numbers', '1\n2\n3\n')
+        repo.commit('Initial commit.')
+
+    def test_split_one_commit_into_two(self):
+        '''Split one commit with two files into two commits of one file each.'''
+        commits = self.repo.log(template='{desc}')
+        self.assertEqual(['Initial commit.'], commits)
+        files = self.repo.log(template='{files}')
+        self.assertEqual(['letters numbers'], files)
+
+        editor = self.create_editor_that_writes_commit_messages(
+            [
+                'first commit',
+                'second commit',
+            ]
+        )
+
+        # The responses are for the following questions:
+        # y  examine changes to 'letters'?
+        # y  record change 1/2 to 'letters'?
+        # y  examine changes to 'numbers'?
+        # n  record change 2/2 to 'numbers'?
+        # y  Done splitting?
+        self.hg(
+            dedent(
+                '''\
+        split --config ui.interactive=true --config ui.interface=text << EOF
+        y
+        y
+        y
+        n
+        y
+        EOF
+        '''
+            ),
+            shell=True,
+            hgeditor=editor
+        )
+
+        self.assert_status_empty()
+        commits = self.repo.log(template='{desc}')
+        self.assertEqual(['first commit', 'second commit'], commits)
+        files = self.repo.log(template='{files}')
+        self.assertEqual(['letters', 'numbers'], files)

eden/integration/lib/hgrepo.py

@@ -69,13 +69,21 @@ class HgRepository(repobase.Repository):
             'hg.real') or distutils.spawn.find_executable('hg')
 
     def hg(self, *args, stdout_charset='utf-8', stdout=subprocess.PIPE,
-           stderr=subprocess.PIPE):
-        cmd = [self.hg_bin] + list(args)
+           stderr=subprocess.PIPE, shell=False, hgeditor=None):
+        if shell:
+            cmd = self.hg_bin + ' ' + args[0]
+        else:
+            cmd = [self.hg_bin] + list(args)
+        env = self.hg_environment
+        if hgeditor is not None:
+            env = dict(env)
+            env['HGEDITOR'] = hgeditor
+
         try:
             completed_process = subprocess.run(cmd, stdout=stdout,
                                                stderr=stderr,
                                                check=True, cwd=self.path,
-                                               env=self.hg_environment)
+                                               env=env, shell=shell)
         except subprocess.CalledProcessError as ex:
             raise HgError(ex) from ex
         if completed_process.stdout is not None: