Merge "Notice merged changes even if they appear on a different branch"

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java

@@ -14,10 +14,13 @@
 
 package com.google.gerrit.acceptance.rest.change;
 
+import static com.google.gerrit.acceptance.GitUtil.pushHead;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.SubmitType;
@@ -32,6 +35,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.PushResult;
 import org.junit.Test;
 
 import java.util.Map;
@@ -156,4 +160,29 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
           .isEqualTo(rev);
     }
   }
+
+  @Test
+  public void submitSameCommitsAsInExperimentalBranch() throws Exception {
+    grant(Permission.CREATE, project, "refs/heads/*");
+    grant(Permission.PUSH, project, "refs/heads/experimental");
+
+    RevCommit c1 = commitBuilder()
+        .add("b.txt", "1")
+        .message("commit at tip")
+        .create();
+    String id1 = GitUtil.getChangeId(testRepo, c1).get();
+
+    PushResult r1 = pushHead(testRepo, "refs/for/master", false);
+    assertThat(r1.getRemoteUpdate("refs/for/master").getNewObjectId())
+        .isEqualTo(c1.getId());
+
+    PushResult r2 = pushHead(testRepo, "refs/heads/experimental", false);
+    assertThat(r2.getRemoteUpdate("refs/heads/experimental").getNewObjectId())
+        .isEqualTo(c1.getId());
+
+    submit(id1);
+
+    assertThat(getRemoteHead().getId()).isEqualTo(c1.getId());
+    assertSubmitter(id1, 1);
+  }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java

@@ -2,13 +2,18 @@ package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.api.changes.CherryPickInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.reviewdb.client.Project;
 
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.transport.RefSpec;
 import org.junit.Test;
 
 import java.util.List;
@@ -258,7 +263,7 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
   @Test
   public void submitWithOpenAncestorsOnOtherBranch() throws Exception {
-    PushOneCommit.Result change1 = createChange(testRepo,  "master",
+    PushOneCommit.Result change1 = createChange(testRepo, "master",
         "base commit",
         "a.txt", "1", "");
     submit(change1.getChangeId());
@@ -268,7 +273,7 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
         .branch("branch")
         .create(new BranchInput());
 
-    PushOneCommit.Result change2 = createChange(testRepo,  "master",
+    PushOneCommit.Result change2 = createChange(testRepo, "master",
         "We want to commit this to master first",
         "a.txt", "2", "");
 
@@ -282,7 +287,7 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
     assertThat(tip2.getShortMessage()).isEqualTo(
         change1.getCommit().getShortMessage());
 
-    PushOneCommit.Result change3a = createChange(testRepo,  "branch",
+    PushOneCommit.Result change3a = createChange(testRepo, "branch",
         "This commit is based on change2 pending for master, "
         + "but is targeted itself at branch, which doesn't include it.",
         "a.txt", "3", "a-topic-here");
@@ -310,4 +315,103 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
     assertThat(tipmaster.getShortMessage()).isEqualTo(
         initialHead.getShortMessage());
   }
+
+  @Test
+  public void testGerritWorkflow() throws Exception {
+    // We'll setup a master and a stable branch.
+    // Then we create a change to be applied to master, which is
+    // then cherry picked back to stable. The stable branch will
+    // be merged up into master again.
+    gApi.projects()
+        .name(project.get())
+        .branch("stable")
+        .create(new BranchInput());
+
+    // Push a change to master
+    PushOneCommit change2 =
+        pushFactory.create(db, user.getIdent(), testRepo,
+            "small fix", "a.txt", "2");
+    PushOneCommit.Result change2result = change2.to("refs/for/master");
+    submit(change2result.getChangeId());
+    RevCommit tipmaster = getRemoteLog(project, "master").get(0);
+    assertThat(tipmaster.getShortMessage()).isEqualTo(
+        change2result.getCommit().getShortMessage());
+
+    // Now cherry pick to stable
+    CherryPickInput in = new CherryPickInput();
+    in.destination = "stable";
+    in.message = "This goes to stable as well\n" + tipmaster.getFullMessage();
+    ChangeApi orig = gApi.changes()
+        .id(change2result.getChangeId());
+    String cherryId = orig.current().cherryPick(in).id();
+    gApi.changes().id(cherryId).current().review(ReviewInput.approve());
+    gApi.changes().id(cherryId).current().submit();
+
+    // Create the merge locally
+    RevCommit stable = getRemoteHead(project, "stable");
+    RevCommit master = getRemoteHead(project, "master");
+    testRepo.git().fetch().call();
+    testRepo.git()
+        .branchCreate()
+        .setName("stable")
+        .setStartPoint(stable)
+        .call();
+    testRepo.git()
+        .branchCreate()
+        .setName("master")
+        .setStartPoint(master)
+        .call();
+
+    RevCommit merge = testRepo.commit()
+        .parent(master)
+        .parent(stable)
+        .message("Merge stable into master")
+        .insertChangeId()
+        .create();
+
+    testRepo.branch("refs/heads/master").update(merge);
+    testRepo.git().push()
+        .setRefSpecs(new RefSpec("refs/heads/master:refs/for/master"))
+        .call();
+
+    String changeId = GitUtil.getChangeId(testRepo, merge).get();
+    approve(changeId);
+    submit(changeId);
+    tipmaster = getRemoteLog(project, "master").get(0);
+    assertThat(tipmaster.getShortMessage()).isEqualTo(merge.getShortMessage());
+  }
+
+  @Test
+  public void openChangeForTargetBranchPreventsMerge() throws Exception {
+    gApi.projects()
+        .name(project.get())
+        .branch("stable")
+        .create(new BranchInput());
+
+    // Propose a change for master, but leave it open for master!
+    PushOneCommit change2 =
+        pushFactory.create(db, user.getIdent(), testRepo,
+            "small fix", "a.txt", "2");
+    PushOneCommit.Result change2result = change2.to("refs/for/master");
+
+    // Now cherry pick to stable
+    CherryPickInput in = new CherryPickInput();
+    in.destination = "stable";
+    in.message = "it goes to stable branch";
+    ChangeApi orig = gApi.changes()
+        .id(change2result.getChangeId());
+    ChangeApi cherry = orig.current().cherryPick(in);
+    cherry.current().review(ReviewInput.approve());
+    cherry.current().submit();
+
+    // Create a commit locally
+    testRepo.git().fetch().setRefSpecs(new RefSpec("refs/heads/stable")).call();
+
+    PushOneCommit.Result change3 = createChange(testRepo, "stable",
+        "test","a.txt", "3", "");
+    submitWithConflict(change3.getChangeId(),
+        "Failed to submit 1 change due to the following problems:\n" +
+        "Change " + change3.getPatchSetId().getParentKey().get() +
+        ": depends on change that was not submitted");
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyListener.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.git.strategy;
 
 import com.google.common.base.CharMatcher;
+import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Change;
@@ -24,6 +25,8 @@ import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.IntegrationException;
 import com.google.gerrit.server.git.MergeOp.CommitStatus;
 
+import org.eclipse.jgit.revwalk.RevCommit;
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -86,8 +89,10 @@ public class SubmitStrategyListener extends BatchUpdate.Listener {
   private void markCleanMerges() throws IntegrationException {
     for (SubmitStrategy strategy : strategies) {
       SubmitStrategy.Arguments args = strategy.args;
+      RevCommit initialTip = args.mergeTip.getInitialTip();
       args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
-          args.mergeTip.getCurrentTip(), args.alreadyAccepted);
+          args.mergeTip.getCurrentTip(), initialTip == null ?
+              ImmutableSet.<RevCommit>of() : ImmutableSet.of(initialTip));
     }
   }