Fix inline edits working on root commits

The initial commit of a repository should be mutable through
inline edits. Fix up the inline edit code to finish removing
all assumptions about "edit is commit on top of patch set".
This allows the code to just amend the commit and safely work
with 0 parents.

When restoring a path from the initial commit, treat that
as a file deletion. The file does not exist. Undoing create
would be deleting it.

When restoring a path edited during a merge commit, revert
to the version from parent 0. This is an "ours" type of merge
on that path, and is likely what the caller wants.

When restoring a path that doesn't exist in the ancestor,
the delete the path. This is very similar to the restore on
an initial commit. The path didn't exist before this change.

Change-Id: Ibab826f48575c0fdff09e4d3d0939e1f01c3baac

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java

@@ -15,6 +15,8 @@
 package com.google.gerrit.acceptance.edit;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.GitUtil.cloneProject;
+import static com.google.gerrit.acceptance.GitUtil.createProject;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -241,6 +243,26 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertThat(edit.isPresent()).isFalse();
   }
 
+  @Test
+  public void updateRootCommitMessage() throws Exception {
+    createProject(sshSession, "root-msg-test", null, false);
+    git = cloneProject(sshSession.getUrl() + "/root-msg-test");
+    changeId = newChange(git, admin.getIdent());
+    change = getChange(changeId);
+
+    assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId)))
+        .isEqualTo(RefUpdate.Result.NEW);
+    Optional<ChangeEdit> edit = editUtil.byChange(change);
+    assertThat(edit.get().getEditCommit().getParentCount()).isEqualTo(0);
+
+    String msg = String.format("New commit message\n\nChange-Id: %s",
+        change.getKey());
+    assertThat(modifier.modifyMessage(edit.get(), msg))
+        .isEqualTo(RefUpdate.Result.FORCED);
+    edit = editUtil.byChange(change);
+    assertThat(edit.get().getEditCommit().getFullMessage()).isEqualTo(msg);
+  }
+
   @Test
   public void updateMessage() throws Exception {
     assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId)))

gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditJson.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.edit;
 
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.gerrit.extensions.common.ActionInfo;
 import com.google.gerrit.extensions.common.CommitInfo;
@@ -35,6 +34,7 @@ import com.google.inject.Singleton;
 
 import org.eclipse.jgit.revwalk.RevCommit;
 
+import java.util.ArrayList;
 import java.util.Map;
 
 @Singleton
@@ -66,16 +66,18 @@ public class ChangeEditJson {
   private static CommitInfo fillCommit(RevCommit editCommit) {
     CommitInfo commit = new CommitInfo();
     commit.commit = editCommit.toObjectId().getName();
-    commit.parents = Lists.newArrayListWithCapacity(1);
     commit.author = CommonConverters.toGitPerson(editCommit.getAuthorIdent());
     commit.committer = CommonConverters.toGitPerson(
         editCommit.getCommitterIdent());
     commit.subject = editCommit.getShortMessage();
     commit.message = editCommit.getFullMessage();
 
+    commit.parents = new ArrayList<>(editCommit.getParentCount());
+    for (RevCommit p : editCommit.getParents()) {
       CommitInfo i = new CommitInfo();
-    i.commit = editCommit.getParent(0).toObjectId().getName();
+      i.commit = p.name();
       commit.parents.add(i);
+    }
 
     return commit;
   }

gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java

@@ -126,10 +126,9 @@ public class ChangeEditModifier {
       RevWalk rw = new RevWalk(repo);
       ObjectInserter inserter = repo.newObjectInserter();
       try {
-        RevCommit base = rw.parseCommit(ObjectId.fromString(
+        RevCommit revision = rw.parseCommit(ObjectId.fromString(
             ps.getRevision().get()));
-        RevCommit changeBase = base.getParent(0);
+        ObjectId commit = createCommit(me, inserter, revision, revision.getTree());
-        ObjectId commit = createCommit(me, inserter, base, changeBase, base.getTree());
         inserter.flush();
         String editRefName = editRefName(me.getAccountId(), change.getId(),
             ps.getId());
@@ -236,12 +235,8 @@ public class ChangeEditModifier {
     if (!currentUser.get().isIdentifiedUser()) {
       throw new AuthException("Authentication required");
     }
-    RevCommit prevEdit = edit.getEditCommit();
-    if (prevEdit.getParentCount() == 0) {
-      throw new InvalidChangeOperationException(
-          "Modify edit against root commit not supported");
-    }
 
+    RevCommit prevEdit = edit.getEditCommit();
     if (prevEdit.getFullMessage().equals(msg)) {
       throw new UnchangedCommitMessageException();
     }
@@ -254,7 +249,6 @@ public class ChangeEditModifier {
       try {
         String refName = edit.getRefName();
         ObjectId commit = createCommit(me, inserter, prevEdit,
-            rw.parseCommit(prevEdit.getParent(0)),
             prevEdit.getTree(),
             msg);
         inserter.flush();
@@ -332,20 +326,18 @@ public class ChangeEditModifier {
       try {
         String refName = edit.getRefName();
         RevCommit prevEdit = edit.getEditCommit();
-        if (prevEdit.getParentCount() == 0) {
+        ObjectId newTree = writeNewTree(op,
-          throw new InvalidChangeOperationException(
+            rw,
-              "Modify edit against root commit not supported");
+            inserter,
-        }
+            prevEdit,
-
+            reader,
-        RevCommit base = prevEdit.getParent(0);
+            file,
-        base = rw.parseCommit(base);
+            toBlob(inserter, content));
-        ObjectId newTree = writeNewTree(op, rw, inserter,
-            prevEdit, reader, file, toBlob(inserter, content), base);
         if (ObjectId.equals(newTree, prevEdit.getTree())) {
           throw new InvalidChangeOperationException("no changes were made");
         }
 
-        ObjectId commit = createCommit(me, inserter, prevEdit, base, newTree);
+        ObjectId commit = createCommit(me, inserter, prevEdit, newTree);
         inserter.flush();
         return update(repo, me, refName, rw, prevEdit, commit);
       } finally {
@@ -373,18 +365,18 @@ public class ChangeEditModifier {
   }
 
   private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter,
-      RevCommit prevEdit, RevCommit base, ObjectId tree) throws IOException {
+      RevCommit revision, ObjectId tree) throws IOException {
-    return createCommit(me, inserter, prevEdit, base, tree,
+    return createCommit(me, inserter, revision, tree,
-        prevEdit.getFullMessage());
+        revision.getFullMessage());
   }
 
   private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter,
-      RevCommit prevEdit, RevCommit base, ObjectId tree, String msg)
+      RevCommit revision, ObjectId tree, String msg)
       throws IOException {
     CommitBuilder builder = new CommitBuilder();
     builder.setTreeId(tree);
-    builder.setParentIds(base);
+    builder.setParentIds(revision.getParents());
-    builder.setAuthor(prevEdit.getAuthorIdent());
+    builder.setAuthor(revision.getAuthorIdent());
     builder.setCommitter(getCommitterIdent(me));
     builder.setMessage(msg);
     return inserter.insert(builder);
@@ -408,8 +400,8 @@ public class ChangeEditModifier {
 
   private static ObjectId writeNewTree(TreeOperation op, RevWalk rw,
       ObjectInserter ins, RevCommit prevEdit, ObjectReader reader,
-      String fileName, final @Nullable ObjectId content, RevCommit base)
+      String fileName, final @Nullable ObjectId content)
-      throws IOException, InvalidChangeOperationException {
+      throws IOException {
     DirCache newTree = readTree(reader, prevEdit);
     DirCacheEditor dce = newTree.editor();
     switch (op) {
@@ -431,21 +423,18 @@ public class ChangeEditModifier {
         break;
 
       case RESTORE_ENTRY:
-        TreeWalk tw = TreeWalk.forPath(
+        if (prevEdit.getParentCount() == 0) {
-            rw.getObjectReader(),
+          dce.add(new DeletePath(fileName));
-            fileName,
+          break;
-            base.getTree().getId());
-
-        // If the file does not exist in the base commit, try to restore it
-        // from the base's parent commit.
-        if (tw == null && base.getParentCount() == 1) {
-          tw = TreeWalk.forPath(rw.getObjectReader(), fileName,
-              rw.parseCommit(base.getParent(0)).getTree());
         }
+
+        RevCommit base = prevEdit.getParent(0);
+        rw.parseHeaders(base);
+        TreeWalk tw =
+            TreeWalk.forPath(rw.getObjectReader(), fileName, base.getTree());
         if (tw == null) {
-          throw new InvalidChangeOperationException(String.format(
+          dce.add(new DeletePath(fileName));
-              "cannot restore path %s: missing in base revision %s",
+          break;
-              fileName, base.abbreviate(8).name()));
         }
 
         final FileMode mode = tw.getFileMode(0);