Add Cherry-Pick mode. Users can specify 'cherryPickMode = True' in the global section of automerger.config to enable this mode. Also, logging all exceptions thrown in event handler worker threads. Bug: 361415628 Change-Id: I7ce71d24f6f95184e6673af6286fcd363b43feb2
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java index dad77dc..346b4b9 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java
@@ -72,7 +72,7 @@ } String revision = rev.getPatchSet().commitId().name(); - MultipleDownstreamMergeInput mdsMergeInput = new MultipleDownstreamMergeInput(); + MultipleDownstreamChangeInput mdsMergeInput = new MultipleDownstreamChangeInput(); mdsMergeInput.dsBranchMap = branchMap; mdsMergeInput.changeNumber = change.getId().get(); mdsMergeInput.patchsetNumber = rev.getPatchSet().number(); @@ -85,7 +85,7 @@ logger.atFine().log("Multiple downstream merge input: %s", mdsMergeInput.dsBranchMap); try { - dsCreator.createMergesAndHandleConflicts(mdsMergeInput, config.getContextUserId()); + dsCreator.createChangesAndHandleConflicts(mdsMergeInput, config.getContextUserId()); } catch (ConfigInvalidException e) { throw new ResourceConflictException( "Automerger configuration file is invalid: " + e.getMessage());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeMode.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeMode.java new file mode 100644 index 0000000..8601f8b --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeMode.java
@@ -0,0 +1,51 @@ +package com.googlesource.gerrit.plugins.automerger; + +import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.server.config.ConfigResource; +import com.google.inject.Inject; +import java.io.IOException; +import org.eclipse.jgit.errors.ConfigInvalidException; + +/** + * AutomergeMode will return the configured mode to create downstream changes. + */ +class AutomergeMode implements RestReadView<ConfigResource> { + + protected ConfigLoader config; + + /** + * Initializer for this class that sets the config. + * + * @param config Config for this plugin. + */ + @Inject + public AutomergeMode(ConfigLoader config) { + this.config = config; + } + + /** + * Return the Automerger mode: either MERGE or CHERRY-PICK. + * + * @return Either MERGE or CHERRY-PICK. + * @throws RestApiException + * @throws IOException + */ + @Override + public Response<String> apply(ConfigResource configResource) + throws RestApiException, IOException { + + try { + if(config.changeMode() == ChangeMode.CHERRY_PICK){ + return Response.ok("CHERRY-PICK"); + } else { + return Response.ok("MERGE"); + } + } catch (ConfigInvalidException e) { + throw new ResourceConflictException( + "Automerger configuration file is invalid: " + e.getMessage()); + } + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergerModule.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergerModule.java index b588a2f..4c34126 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergerModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergerModule.java
@@ -15,6 +15,7 @@ package com.googlesource.gerrit.plugins.automerger; import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND; +import static com.google.gerrit.server.config.ConfigResource.CONFIG_KIND; import static com.google.gerrit.server.project.BranchResource.BRANCH_KIND; import com.google.gerrit.extensions.events.ChangeAbandonedListener; @@ -40,6 +41,7 @@ DynamicSet.bind(binder(), RevisionCreatedListener.class).to(DownstreamCreator.class); DynamicSet.bind(binder(), TopicEditedListener.class).to(DownstreamCreator.class); DynamicSet.bind(binder(), MergeValidationListener.class).to(MergeValidator.class); + bind(ChangeCreatorApi.class).toProvider(ChangeCreatorProvider.class); install( new RestApiModule() { @Override @@ -47,6 +49,7 @@ post(REVISION_KIND, "automerge-change").to(AutomergeChangeAction.class); post(REVISION_KIND, "config-downstream").to(ConfigDownstreamAction.class); get(BRANCH_KIND, "all-config-downstream").to(AllConfigDownstreamAction.class); + get(CONFIG_KIND, "automerge-mode").to(AutomergeMode.class); } }); DynamicSet.bind(binder(), WebUiPlugin.class).toInstance(new JavaScriptPlugin("automerger.js"));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeCreatorApi.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeCreatorApi.java new file mode 100644 index 0000000..3d3f939 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeCreatorApi.java
@@ -0,0 +1,16 @@ +package com.googlesource.gerrit.plugins.automerger; + +import com.google.gerrit.exceptions.StorageException; +import com.google.gerrit.extensions.api.changes.ChangeApi; +import com.google.gerrit.extensions.restapi.RestApiException; +import org.eclipse.jgit.errors.ConfigInvalidException; + +/** ChangeCreatorApi is the interface used to create or update downstream changes. */ +public interface ChangeCreatorApi { + ChangeApi create(SingleDownstreamChangeInput sdsChangeInput, String currentTopic) + throws RestApiException, ConfigInvalidException, InvalidQueryParameterException, + StorageException; + + void update(UpdateDownstreamChangeInput updateDownstreamChangeInput) + throws RestApiException, ConfigInvalidException, InvalidQueryParameterException; +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeCreatorProvider.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeCreatorProvider.java new file mode 100644 index 0000000..6d1d770 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeCreatorProvider.java
@@ -0,0 +1,44 @@ +package com.googlesource.gerrit.plugins.automerger; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.extensions.api.GerritApi; +import com.google.inject.Inject; +import com.google.inject.Provider; +import org.eclipse.jgit.errors.ConfigInvalidException; + +/** + * ChangeCreatorProvider provides the appropriate ChangeCreatorApi implementation based on the + * plugin's configuration. + */ +public class ChangeCreatorProvider implements Provider<ChangeCreatorApi> { + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final GerritApi gApi; + private final ConfigLoader config; + + @Inject + public ChangeCreatorProvider( + GerritApi gApi, + ConfigLoader config + ) { + this.gApi = gApi; + this.config = config; + } + + @Override + public ChangeCreatorApi get() { + ChangeMode changeMode = ChangeMode.MERGE; + try { + changeMode = config.changeMode(); + } catch (ConfigInvalidException e) { + logger.atWarning().log( + "Unable to read the config for cherryPickMode value. Defaulting to legacy merge-mode behavior"); + } + + if(changeMode == ChangeMode.CHERRY_PICK){ + return new CherryPickChangeCreator(gApi); + } else { + return new MergeChangeCreator(gApi); + } + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeMode.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeMode.java new file mode 100644 index 0000000..a4218ef --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeMode.java
@@ -0,0 +1,9 @@ +package com.googlesource.gerrit.plugins.automerger; + +/** + * ChangeMode defines the mode for creating downstream changes. + */ +public enum ChangeMode { + MERGE, + CHERRY_PICK +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeUtils.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeUtils.java new file mode 100644 index 0000000..f240498 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ChangeUtils.java
@@ -0,0 +1,214 @@ +package com.googlesource.gerrit.plugins.automerger; + +import com.google.common.base.Joiner; +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.extensions.api.GerritApi; +import com.google.gerrit.extensions.api.changes.ChangeApi; +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.ReviewInput; +import com.google.gerrit.extensions.client.ListChangesOption; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.CommitInfo; +import com.google.gerrit.extensions.common.RevisionInfo; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.RestApiException; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Map; + +/** ChangeUtils is a utility class for interacting with Gerrit changes */ +public final class ChangeUtils { + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + public static final String AUTOMERGER_TAG = "autogenerated:Automerger"; + private static final String SKIPPED_PREFIX = "skipped"; + private static final String CURRENT = "current"; + + private ChangeUtils(){ + throw new UnsupportedOperationException("ChangeUtils should not be instantiated."); + } + + public static QueryBuilder constructTopicQuery(String topic) throws InvalidQueryParameterException { + QueryBuilder queryBuilder = new QueryBuilder(); + queryBuilder.addParameter("topic", topic); + queryBuilder.addParameter("status", "open"); + return queryBuilder; + } + + public static List<ChangeInfo> getChangesInTopicAndBranch(GerritApi gApi, String topic, String downstreamBranch) + throws InvalidQueryParameterException, RestApiException { + QueryBuilder queryBuilder = constructTopicQuery(topic); + queryBuilder.addParameter("branch", downstreamBranch); + return gApi.changes() + .query(queryBuilder.get()) + .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT) + .get(); + } + + public static List<String> getChangeParents(GerritApi gApi, int changeNumber, String currentRevision) + throws RestApiException { + ChangeApi change = gApi.changes().id(changeNumber); + List<String> parents = new ArrayList<>(); + Map<String, RevisionInfo> revisionMap = + change.get(EnumSet.of(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT)) + .revisions; + List<CommitInfo> changeParents = revisionMap.get(currentRevision).commit.parents; + for (CommitInfo commit : changeParents) { + parents.add(commit.commit); + } + return parents; + } + + /** + * Create subject line for downstream change with metadata from upstream change. + * + * <p>The downstream subject will be in the format: "[subjectPrefix] upstreamSubject am: + * upstreamRevision". If it is a skip, "am" will be replaced with "skipped", and [subjectPrefix] + * replaced with [subjectPrefix skipped]. + * + * @param upstreamSubject Subject line of the upstream change + * @param upstreamRevision Commit SHA1 of the upstream change + * @param skipped Whether or not the merge is done with "-s ours" + * @return Subject line for downstream merge + */ + public static String getSubjectForDownstreamChange( + String subjectPrefix, String upstreamSubject, String upstreamRevision, boolean skipped) { + if (!upstreamSubject.startsWith("[" + subjectPrefix)) { + String prefix = "[" + subjectPrefix + "]"; + if (skipped) { + prefix = "[" + subjectPrefix + " " + SKIPPED_PREFIX + "]"; + } + upstreamSubject = Joiner.on(" ").join(prefix, upstreamSubject); + } + String denotationString = skipped ? "skipped:" : "am:"; + return Joiner.on(" ") + .join(upstreamSubject, denotationString, upstreamRevision.substring(0, 10)); + } + + public static String getTopic(GerritApi gApi, String revision) throws InvalidQueryParameterException, RestApiException { + QueryBuilder queryBuilder = new QueryBuilder(); + queryBuilder.addParameter("commit", revision); + List<ChangeInfo> changes = + gApi.changes() + .query(queryBuilder.get()) + .withOption(ListChangesOption.CURRENT_REVISION) + .get(); + if (!changes.isEmpty()) { + for (ChangeInfo change : changes) { + if (change.currentRevision.equals(revision) && !"".equals(change.topic)) { + return change.topic; + } + } + } + return null; + } + + public static void tagChange(GerritApi gApi, ChangeInfo change, String message) throws RestApiException { + ReviewInput reviewInput = new ReviewInput(); + reviewInput.message(message); + reviewInput.notify = NotifyHandling.NONE; + reviewInput.tag = AUTOMERGER_TAG; + try { + gApi.changes().id(change.id).revision(CURRENT).review(reviewInput); + } catch (AuthException e) { + logger.atSevere().withCause(e).log("Automerger could not set label, but still continuing."); + } + } + + public static String getSkipHashtag(String downstreamBranch){ + return "am_skip_" + downstreamBranch; + } + + public static boolean isDownstreamCherryPick(GerritApi gApi, String upstreamRevision, ChangeInfo downstreamChange){ + try { + ChangeInfo upstreamChange = gApi.changes().id(upstreamRevision).get(); + int upstreamPatchset = upstreamChange.revisions.get(upstreamRevision)._number; + + if(downstreamChange.cherryPickOfChange == null) + return false; + + return downstreamChange.cherryPickOfChange.equals(upstreamChange._number) && + downstreamChange.cherryPickOfPatchSet == upstreamPatchset; + } catch (RestApiException e) { + logger.atSevere().withCause(e).log( + "Unable to lookup change with revision %s", upstreamRevision); + return false; + } + } + + public static boolean isDownstreamMerge(String upstreamRevision, ChangeInfo downstreamChange){ + String changeRevision = downstreamChange.currentRevision; + RevisionInfo revision = downstreamChange.revisions.get(changeRevision); + List<CommitInfo> parents = revision.commit.parents; + if (parents.size() > 1) { + String secondParent = parents.get(1).commit; + if (secondParent.equals(upstreamRevision)) { + return true; + } + } + + return false; + } + + public static boolean isDownstreamChange(GerritApi gApi, String upstreamRevision, ChangeInfo downstreamChange, ChangeMode changeMode) { + boolean downstreamExists = (changeMode == ChangeMode.CHERRY_PICK) && isDownstreamCherryPick(gApi, upstreamRevision, downstreamChange); + downstreamExists |= (changeMode == ChangeMode.MERGE) && isDownstreamMerge(upstreamRevision, downstreamChange); + + return downstreamExists; + } + + /** + * Get the base change ID that the downstream change should be based off of, given the parents. + * + * <p>Given changes A and B where A is the first parent of B (stacked changes), and where A' is + * the downstream change autogenerated by A, and B' is the downstream change autogenerated by A, + * the first parent of B' should be A'. + * + * @param parents Parent commit SHAs of the change + * @return The base change ID that the change should be based off of, null if there is none. + * @throws InvalidQueryParameterException + * @throws RestApiException + */ + private static ChangeInfo getBaseChangeInfo(GerritApi gApi, List<String> parents, String branch, ChangeMode changeMode) + throws InvalidQueryParameterException, RestApiException { + if (parents.isEmpty()) { + logger.atInfo().log("No base change id for change with no parents."); + return null; + } + // 1) Get topic of first parent + String firstParentTopic = ChangeUtils.getTopic(gApi, parents.get(0)); + if (firstParentTopic == null) { + return null; + } + // 2) query that topic and use that to find A' + List<ChangeInfo> changesInTopic = ChangeUtils.getChangesInTopicAndBranch(gApi, firstParentTopic, branch); + String firstParent = parents.get(0); + for (ChangeInfo change : changesInTopic) { + if(isDownstreamChange(gApi, firstParent, change, changeMode)){ + return change; + } + } + return null; + } + public static String getBaseChangeIdForMerge(GerritApi gApi, List<String> parents, String branch) + throws InvalidQueryParameterException, RestApiException { + ChangeInfo change = getBaseChangeInfo(gApi, parents, branch, ChangeMode.MERGE); + + if(change == null) + return null; + + return String.valueOf(change._number); + } + + public static String getBaseChangeRevisionForCherryPick(GerritApi gApi, List<String> parents, String branch) + throws InvalidQueryParameterException, RestApiException { + ChangeInfo change = getBaseChangeInfo(gApi, parents, branch, ChangeMode.CHERRY_PICK); + + if(change == null) + return null; + + return change.currentRevision; + } + +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/CherryPickChangeCreator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/CherryPickChangeCreator.java new file mode 100644 index 0000000..6eac9f6 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/CherryPickChangeCreator.java
@@ -0,0 +1,159 @@ +package com.googlesource.gerrit.plugins.automerger; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.exceptions.StorageException; +import com.google.gerrit.extensions.api.GerritApi; +import com.google.gerrit.extensions.api.changes.AbandonInput; +import com.google.gerrit.extensions.api.changes.ChangeApi; +import com.google.gerrit.extensions.api.changes.CherryPickInput; +import com.google.gerrit.extensions.api.changes.HashtagsInput; +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.inject.Inject; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import org.eclipse.jgit.errors.ConfigInvalidException; + +/** + * CherryPickChangeCreator handles creation and updating of downstream changes as cherry-picks. + */ +public class CherryPickChangeCreator implements ChangeCreatorApi { + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final String SUBJECT_PREFIX = "autocherry"; + private final GerritApi gApi; + @Inject + public CherryPickChangeCreator( + GerritApi gApi) { + this.gApi = gApi; + } + + /** + * Create a single downstream cherry-pick. + * + * <p>On a skip, only a hashtag is applied to the upstream change.</p> + * + * @param sdsChangeInput Input containing metadata for the cherry-pick. + * @param currentTopic Current topic to create change in. + * @throws RestApiException + * @throws ConfigInvalidException + * @throws InvalidQueryParameterException + * @throws StorageException + */ + @Override + public ChangeApi create(SingleDownstreamChangeInput sdsChangeInput, String currentTopic) + throws RestApiException, ConfigInvalidException, InvalidQueryParameterException, StorageException { + + if (!sdsChangeInput.doChange) { + logger.atFine().log( + "Skipping cherry-pick for %s to %s", + sdsChangeInput.currentRevision, sdsChangeInput.downstreamBranch); + + applySkipHashtag(sdsChangeInput); + + return null; + } + + // This mirrors the MergeChangeCreator, although I don't believe it is possible with + // cherry-picks. Merge mode can encounter this scenario in diamond merges. + if (isAlreadyCherryPicked(sdsChangeInput, currentTopic)) { + logger.atInfo().log( + "Commit %s already cherry-picked into %s, not cherry-picking again.", + sdsChangeInput.currentRevision, sdsChangeInput.downstreamBranch); + return null; + } + + removeSkipHashtag(sdsChangeInput); + + CherryPickInput cherryPickInput = new CherryPickInput(); + cherryPickInput.base = + ChangeUtils.getBaseChangeRevisionForCherryPick(gApi, + ChangeUtils.getChangeParents(gApi, sdsChangeInput.changeNumber, sdsChangeInput.currentRevision), + sdsChangeInput.downstreamBranch); + cherryPickInput.message = + ChangeUtils.getSubjectForDownstreamChange(SUBJECT_PREFIX, sdsChangeInput.subject, sdsChangeInput.currentRevision, !sdsChangeInput.doChange); + cherryPickInput.destination = sdsChangeInput.downstreamBranch; + cherryPickInput.notify = NotifyHandling.NONE; + cherryPickInput.topic = currentTopic; + + return gApi.changes().id(sdsChangeInput.changeNumber).current().cherryPick(cherryPickInput); + } + + private Set<String> getSkipHashtagSet(String downstreamBranch) { + Set<String> set = new HashSet<>(); + set.add(ChangeUtils.getSkipHashtag(downstreamBranch)); + return set; + } + + private void applySkipHashtag(SingleDownstreamChangeInput sdsChangeInput) throws RestApiException { + ChangeApi originalChange = gApi.changes().id(sdsChangeInput.changeNumber); + Set<String> set = getSkipHashtagSet(sdsChangeInput.downstreamBranch); + originalChange.setHashtags(new HashtagsInput(set)); + } + + private void removeSkipHashtag(SingleDownstreamChangeInput sdsChangeInput) throws RestApiException { + ChangeApi originalChange = gApi.changes().id(sdsChangeInput.changeNumber); + Set<String> set = getSkipHashtagSet(sdsChangeInput.downstreamBranch); + + // It is safe to blindly attempt to remove the hashtag. + originalChange.setHashtags(new HashtagsInput(null, set)); + } + + @Override + public void update(UpdateDownstreamChangeInput updateDownstreamChangeInput) + throws RestApiException, ConfigInvalidException, InvalidQueryParameterException { + + // For cherry-picks, we don't update the prior existing commit with a patch application. + // Doing this will not update the 'Cherry pick of' metadata with the correct patchset. + // Instead, we abandon the old one and create a new one. + AbandonInput abandonInput = new AbandonInput(); + abandonInput.notify = NotifyHandling.NONE; + if(!updateDownstreamChangeInput.doChange){ + abandonInput.message = "The cherry-pick from upstream is now skipped"; + } else { + abandonInput.message = "The upstream patch set is no longer current"; + } + gApi.changes().id(updateDownstreamChangeInput.downstreamChangeNumber).abandon(abandonInput); + + SingleDownstreamChangeInput sdsChangeInput = getSingleDownstreamChangeInput( + updateDownstreamChangeInput); + + // We still "create" in the event of a skip to apply the appropriate hashtag. + ChangeApi newDownstream = create(sdsChangeInput, updateDownstreamChangeInput.topic); + if(newDownstream != null) { + ChangeUtils.tagChange(gApi, newDownstream.get(), "Automerger change created!"); + } + } + + private static SingleDownstreamChangeInput getSingleDownstreamChangeInput( + UpdateDownstreamChangeInput updateDownstreamChangeInput) { + + SingleDownstreamChangeInput sdsChangeInput = new SingleDownstreamChangeInput(); + sdsChangeInput.subject = + ChangeUtils.getSubjectForDownstreamChange(SUBJECT_PREFIX, + updateDownstreamChangeInput.upstreamSubject, + updateDownstreamChangeInput.upstreamRevision, !updateDownstreamChangeInput.doChange); + sdsChangeInput.changeNumber = updateDownstreamChangeInput.upstreamChangeNumber; + sdsChangeInput.patchsetNumber = updateDownstreamChangeInput.patchSetNumber; + sdsChangeInput.doChange = updateDownstreamChangeInput.doChange; + sdsChangeInput.currentRevision = updateDownstreamChangeInput.upstreamRevision; + sdsChangeInput.downstreamBranch = updateDownstreamChangeInput.downstreamBranch; + return sdsChangeInput; + } + + boolean isAlreadyCherryPicked(SingleDownstreamChangeInput sdsChangeInput, String currentTopic) + throws InvalidQueryParameterException, RestApiException { + + List<ChangeInfo> changes = + ChangeUtils.getChangesInTopicAndBranch(gApi, currentTopic, sdsChangeInput.downstreamBranch); + for (ChangeInfo change : changes) { + if(change.cherryPickOfChange.equals(sdsChangeInput.changeNumber) + && change.cherryPickOfPatchSet == sdsChangeInput.patchsetNumber) { + return true; + } + } + return false; + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java index 5845f59..d2171e5 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
@@ -400,4 +400,10 @@ projects.removeAll(ignoreProjects); return projects; } + + public ChangeMode changeMode() throws ConfigInvalidException { + boolean cherryPickMode = getConfig().getBoolean("global", "cherryPickMode", false); + + return cherryPickMode ? ChangeMode.CHERRY_PICK : ChangeMode.MERGE; + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java index d46f242..6c480c6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
@@ -23,16 +23,10 @@ import com.google.gerrit.extensions.api.changes.AbandonInput; import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.NotifyHandling; -import com.google.gerrit.extensions.api.changes.RestoreInput; import com.google.gerrit.extensions.api.changes.ReviewInput; -import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.common.ChangeInput; -import com.google.gerrit.extensions.common.CommitInfo; import com.google.gerrit.extensions.common.LabelInfo; -import com.google.gerrit.extensions.common.MergeInput; -import com.google.gerrit.extensions.common.MergePatchSetInput; import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.events.ChangeAbandonedListener; import com.google.gerrit.extensions.events.ChangeRestoredListener; @@ -45,6 +39,7 @@ import com.google.gerrit.json.OutputFormat; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.FanOutExecutor; +import com.google.gerrit.server.submit.IntegrationConflictException; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; import com.google.gson.Gson; @@ -77,10 +72,7 @@ RevisionCreatedListener, TopicEditedListener { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private static final String AUTOMERGER_TAG = "autogenerated:Automerger"; private static final String MERGE_CONFLICT_TAG = "autogenerated:MergeConflict"; - private static final String SUBJECT_PREFIX = "automerger"; - private static final String SKIPPED_PREFIX = "skipped"; private static final String CURRENT = "current"; private static final Gson GSON = OutputFormat.JSON_COMPACT.newGson(); @@ -89,6 +81,7 @@ private final ExecutorService executorService; private final OneOffRequestContext oneOffRequestContext; private final Provider<CurrentUser> user; + private final Provider<ChangeCreatorApi> changeCreator; @Inject public DownstreamCreator( @@ -96,12 +89,15 @@ ConfigLoader config, OneOffRequestContext oneOffRequestContext, @FanOutExecutor ExecutorService executorService, - Provider<CurrentUser> user) { + Provider<CurrentUser> user, + Provider<ChangeCreatorApi> changeCreator + ) { this.gApi = gApi; this.config = config; this.oneOffRequestContext = oneOffRequestContext; this.executorService = executorService; this.user = user; + this.changeCreator = changeCreator; } /** @@ -131,7 +127,7 @@ gApi.changes().id(change._number).revision(revisionNumber).commit(false).commit; logger.atFine().log("Detected revision %s abandoned on %s.", revision, change.project); abandonDownstream(change, revision, accountId); - } catch (ConfigInvalidException | StorageException | RestApiException e) { + } catch (Exception e) { logger.atSevere().withCause(e).log( "Automerger plugin failed onChangeAbandoned for %s", change.id); } @@ -187,7 +183,7 @@ return; } - // If change is empty, prevent someone breaking topic. + // If change is empty, prevent someone breaking topic by reapplying the old topic. if (isNullOrEmpty(change.topic)) { try { gApi.changes().id(change._number).topic(oldTopic); @@ -205,7 +201,7 @@ for (String downstreamBranch : downstreamBranches) { try { List<Integer> existingDownstream = - getExistingMergesOnBranch(revision, oldTopic, downstreamBranch, accountId); + getExistingChangesOnBranch(revision, oldTopic, downstreamBranch, accountId); for (Integer changeNumber : existingDownstream) { logger.atFine().log("Setting topic %s on %s", change.topic, changeNumber); gApi.changes().id(changeNumber).topic(change.topic); @@ -215,7 +211,7 @@ } } } - } catch (StorageException | ConfigInvalidException e) { + } catch (Exception e) { logger.atSevere().withCause(e).log( "Automerger plugin failed onTopicEdited for %s", eventChange.id); } @@ -270,7 +266,7 @@ for (String downstreamBranch : downstreamBranches) { try { List<Integer> existingDownstream = - getExistingMergesOnBranch(revision, change.topic, downstreamBranch, accountId); + getExistingChangesOnBranch(revision, change.topic, downstreamBranch, accountId); for (Integer changeNumber : existingDownstream) { ChangeInfo downstreamChange = gApi.changes().id(changeNumber).get(EnumSet.of(ListChangesOption.CURRENT_REVISION)); @@ -293,7 +289,7 @@ "Exception when updating downstream votes of %s", change.id); } } - } catch (StorageException | ConfigInvalidException | RestApiException | IOException e) { + } catch (Exception e) { logger.atSevere().withCause(e).log( "Automerger plugin failed onCommentAdded for %s", change.id); } @@ -323,11 +319,7 @@ private void onChangeRestoredImpl(ChangeInfo change, RevisionInfo revision, Account.Id accountId) { try (ManualRequestContext ctx = oneOffRequestContext.openAs(accountId)) { automergeChanges(change, revision, accountId); - } catch (RestApiException - | IOException - | ConfigInvalidException - | InvalidQueryParameterException - | StorageException e) { + } catch (Exception e) { logger.atSevere().withCause(e).log( "Automerger plugin failed onChangeRestored for %s", change.id); } @@ -358,18 +350,14 @@ public void onRevisionCreatedImpl(ChangeInfo change, RevisionInfo revision, Account.Id accountId) { try (ManualRequestContext ctx = oneOffRequestContext.openAs(accountId)) { automergeChanges(change, revision, accountId); - } catch (RestApiException - | IOException - | ConfigInvalidException - | InvalidQueryParameterException - | StorageException e) { + } catch (Exception e){ logger.atSevere().withCause(e).log( "Automerger plugin failed onRevisionCreated for %s", change.id); } } public String getOrSetTopic(int sourceId, String topic, Account.Id accountId) - throws RestApiException, ConfigInvalidException { + throws RestApiException { try (ManualRequestContext ctx = oneOffRequestContext.openAs(accountId)) { if (isNullOrEmpty(topic)) { topic = "am-" + UUID.randomUUID(); @@ -381,29 +369,29 @@ } /** - * Creates merges downstream, and votes on the automerge label if we have a failed merge. + * Creates changes downstream, and votes on the automerge label if we have a failed merge. * - * @param mdsMergeInput Input containing the downstream branch map and source change ID. + * @param mdsChangeInput Input containing the downstream branch map and source change ID. * @param accountId Account ID to authorize Gerrit API calls. * @throws RestApiException Throws if we fail a REST API call. * @throws ConfigInvalidException Throws if we get a malformed configuration * @throws InvalidQueryParameterException Throws if we attempt to add an invalid value to query. * @throws StorageException Throws if we fail to open the request context */ - public void createMergesAndHandleConflicts(MultipleDownstreamMergeInput mdsMergeInput, Account.Id accountId) + public void createChangesAndHandleConflicts(MultipleDownstreamChangeInput mdsChangeInput, Account.Id accountId) throws RestApiException, ConfigInvalidException, InvalidQueryParameterException, StorageException { try (ManualRequestContext ctx = oneOffRequestContext.openAs(accountId)) { ReviewInput reviewInput = new ReviewInput(); Map<String, Short> labels = new HashMap<>(); try { - createDownstreamMerges(mdsMergeInput, accountId); + createDownstreamChanges(mdsChangeInput, accountId); reviewInput.message = "Automerging change " - + mdsMergeInput.changeNumber + + mdsChangeInput.changeNumber + " to " - + Joiner.on(", ").join(mdsMergeInput.dsBranchMap.keySet()) + + Joiner.on(", ").join(mdsChangeInput.dsBranchMap.keySet()) + " succeeded!"; reviewInput.notify = NotifyHandling.NONE; } catch (FailedMergeException e) { @@ -419,7 +407,7 @@ // Make the vote on the original change ChangeInfo originalChange = - getOriginalChange(mdsMergeInput.changeNumber, mdsMergeInput.currentRevision); + getOriginalChange(mdsChangeInput.changeNumber, mdsChangeInput.currentRevision); // if this fails, i.e. -2 is restricted, catch it and still post message without a vote. try { gApi.changes().id(originalChange._number).revision(CURRENT).review(reviewInput); @@ -436,9 +424,9 @@ } /** - * Creates merge downstream. + * Creates changes downstream. * - * @param mdsMergeInput Input containing the downstream branch map and source change ID. + * @param mdsChangeInput Input containing the downstream branch map and source change ID. * @param accountId Account ID to authorize Gerrit API calls. * @throws RestApiException Throws if we fail a REST API call. * @throws FailedMergeException Throws if we get a merge conflict when merging downstream. @@ -446,7 +434,7 @@ * @throws InvalidQueryParameterException Throws if we attempt to add an invalid value to query. * @throws StorageException Throws if we fail to open the request context */ - private void createDownstreamMerges(MultipleDownstreamMergeInput mdsMergeInput, Account.Id accountId) + private void createDownstreamChanges(MultipleDownstreamChangeInput mdsChangeInput, Account.Id accountId) throws RestApiException, FailedMergeException, ConfigInvalidException, InvalidQueryParameterException, StorageException { try (ManualRequestContext ctx = oneOffRequestContext.openAs(accountId)) { @@ -454,31 +442,35 @@ Map<String, String> failedMergeBranchMap = new TreeMap<>(); List<Integer> existingDownstream; - for (String downstreamBranch : mdsMergeInput.dsBranchMap.keySet()) { - // If there are existing downstream merges, update them + for (String downstreamBranch : mdsChangeInput.dsBranchMap.keySet()) { + // If there are existing downstream changes, update them // Otherwise, create them. boolean createDownstreams = true; - if (mdsMergeInput.obsoleteRevision != null) { + if (mdsChangeInput.obsoleteRevision != null) { existingDownstream = - getExistingMergesOnBranch( - mdsMergeInput.obsoleteRevision, mdsMergeInput.topic, downstreamBranch, accountId); + getExistingChangesOnBranch( + mdsChangeInput.obsoleteRevision, mdsChangeInput.topic, downstreamBranch, accountId); if (!existingDownstream.isEmpty()) { logger.atFine().log( "Attempting to update downstream merge of %s on branch %s", - mdsMergeInput.currentRevision, downstreamBranch); + mdsChangeInput.currentRevision, downstreamBranch); // existingDownstream should almost always be of length one, but // it's possible to construct it so that it's not for (Integer dsChangeNumber : existingDownstream) { try { - updateDownstreamMerge( - mdsMergeInput.currentRevision, - mdsMergeInput.subject, - dsChangeNumber, - mdsMergeInput.dsBranchMap.get(downstreamBranch), - mdsMergeInput.changeNumber, - downstreamBranch); + UpdateDownstreamChangeInput updateDownstreamChangeInput = new UpdateDownstreamChangeInput(); + updateDownstreamChangeInput.upstreamRevision = mdsChangeInput.currentRevision; + updateDownstreamChangeInput.upstreamSubject = mdsChangeInput.subject; + updateDownstreamChangeInput.downstreamChangeNumber = dsChangeNumber; + updateDownstreamChangeInput.doChange = mdsChangeInput.dsBranchMap.get(downstreamBranch); + updateDownstreamChangeInput.upstreamChangeNumber = mdsChangeInput.changeNumber; + updateDownstreamChangeInput.patchSetNumber = mdsChangeInput.patchsetNumber; + updateDownstreamChangeInput.downstreamBranch = downstreamBranch; + updateDownstreamChangeInput.topic = mdsChangeInput.topic; + + changeCreator.get().update(updateDownstreamChangeInput); createDownstreams = false; - } catch (MergeConflictException e) { + } catch (MergeConflictException | IntegrationConflictException e) { failedMergeBranchMap.put(downstreamBranch, e.getMessage()); logger.atFine().log( "Abandoning existing, obsolete %s due to merge conflict.", dsChangeNumber); @@ -489,19 +481,20 @@ } if (createDownstreams) { logger.atFine().log( - "Attempting to create downstream merge of %s on branch %s", - mdsMergeInput.currentRevision, downstreamBranch); - SingleDownstreamMergeInput sdsMergeInput = new SingleDownstreamMergeInput(); - sdsMergeInput.currentRevision = mdsMergeInput.currentRevision; - sdsMergeInput.changeNumber = mdsMergeInput.changeNumber; - sdsMergeInput.project = mdsMergeInput.project; - sdsMergeInput.topic = mdsMergeInput.topic; - sdsMergeInput.subject = mdsMergeInput.subject; - sdsMergeInput.downstreamBranch = downstreamBranch; - sdsMergeInput.doMerge = mdsMergeInput.dsBranchMap.get(downstreamBranch); + "Attempting to create downstream change of %s on branch %s", + mdsChangeInput.currentRevision, downstreamBranch); + SingleDownstreamChangeInput sdsChangeInput = new SingleDownstreamChangeInput(); + sdsChangeInput.currentRevision = mdsChangeInput.currentRevision; + sdsChangeInput.changeNumber = mdsChangeInput.changeNumber; + sdsChangeInput.patchsetNumber = mdsChangeInput.patchsetNumber; + sdsChangeInput.project = mdsChangeInput.project; + sdsChangeInput.topic = mdsChangeInput.topic; + sdsChangeInput.subject = mdsChangeInput.subject; + sdsChangeInput.downstreamBranch = downstreamBranch; + sdsChangeInput.doChange = mdsChangeInput.dsBranchMap.get(downstreamBranch); try { - createSingleDownstreamMerge(sdsMergeInput, accountId); - } catch (MergeConflictException e) { + createSingleDownstreamChange(sdsChangeInput, accountId); + } catch (MergeConflictException | IntegrationConflictException e) { failedMergeBranchMap.put(downstreamBranch, e.getMessage()); } } @@ -509,28 +502,27 @@ if (!failedMergeBranchMap.isEmpty()) { String conflictMessage = config.getConflictMessage(); - if (mdsMergeInput.project.equals(config.getManifestProject())) { + if (mdsChangeInput.project.equals(config.getManifestProject())) { conflictMessage = config.getManifestConflictMessage(); } throw new FailedMergeException( failedMergeBranchMap, - mdsMergeInput.currentRevision, + mdsChangeInput.currentRevision, config.getHostName(), - mdsMergeInput.project, - mdsMergeInput.changeNumber, - mdsMergeInput.patchsetNumber, + mdsChangeInput.project, + mdsChangeInput.changeNumber, + mdsChangeInput.patchsetNumber, conflictMessage, - mdsMergeInput.topic); + mdsChangeInput.topic); } } } /** - * Get change IDs of the immediately downstream changes of the revision on the branch. + * Get change numbers of the immediately downstream changes of the revision on the branch. * - * @param upstreamRevision Revision of the original change. * @param topic Topic of the original change. - * @param downstreamBranch Branch to check for existing merge CLs. + * @param downstreamBranch Branch to check for existing automerger CLs. * @param accountId Account ID to authorize Gerrit API calls. * @return List of change numbers that are downstream of the given branch. * @throws RestApiException Throws when we fail a REST API call. @@ -538,25 +530,21 @@ * @throws ConfigInvalidException Throws if we fail to read the config * @throws StorageException Throws if we fail to open the request context */ - private List<Integer> getExistingMergesOnBranch( + private List<Integer> getExistingChangesOnBranch( String upstreamRevision, String topic, String downstreamBranch, Account.Id accountId) throws RestApiException, InvalidQueryParameterException, StorageException, ConfigInvalidException { try (ManualRequestContext ctx = oneOffRequestContext.openAs(accountId)) { List<Integer> downstreamChangeNumbers = new ArrayList<>(); - List<ChangeInfo> changes = getChangesInTopicAndBranch(topic, downstreamBranch); + List<ChangeInfo> changes = ChangeUtils.getChangesInTopicAndBranch(gApi, topic, downstreamBranch); + ChangeMode changeMode = config.changeMode(); for (ChangeInfo change : changes) { - String changeRevision = change.currentRevision; - RevisionInfo revision = change.revisions.get(changeRevision); - List<CommitInfo> parents = revision.commit.parents; - if (parents.size() > 1) { - String secondParent = parents.get(1).commit; - if (secondParent.equals(upstreamRevision)) { - downstreamChangeNumbers.add(change._number); - } + if(ChangeUtils.isDownstreamChange(gApi, upstreamRevision, change, changeMode)) { + downstreamChangeNumbers.add(change._number); } } + return downstreamChangeNumbers; } } @@ -564,94 +552,24 @@ /** * Create a single downstream merge. * - * @param sdsMergeInput Input containing metadata for the merge. + * @param sdsChangeInput Input containing metadata for the merge. * @param accountId Account ID to authorize Gerrit API calls. * @throws RestApiException * @throws ConfigInvalidException * @throws InvalidQueryParameterException * @throws StorageException */ - private void createSingleDownstreamMerge(SingleDownstreamMergeInput sdsMergeInput, Account.Id accountId) + private void createSingleDownstreamChange(SingleDownstreamChangeInput sdsChangeInput, Account.Id accountId) throws RestApiException, ConfigInvalidException, InvalidQueryParameterException, StorageException { try (ManualRequestContext ctx = oneOffRequestContext.openAs(accountId)) { - String currentTopic = getOrSetTopic(sdsMergeInput.changeNumber, sdsMergeInput.topic, accountId); + String currentTopic = getOrSetTopic(sdsChangeInput.changeNumber, sdsChangeInput.topic, accountId); - if (isAlreadyMerged(sdsMergeInput, currentTopic)) { - logger.atInfo().log( - "Commit %s already merged into %s, not automerging again.", - sdsMergeInput.currentRevision, sdsMergeInput.downstreamBranch); - return; - } - - MergeInput mergeInput = new MergeInput(); - mergeInput.source = sdsMergeInput.currentRevision; - mergeInput.strategy = "recursive"; - - logger.atFine().log("Creating downstream merge for %s", sdsMergeInput.currentRevision); - ChangeInput downstreamChangeInput = new ChangeInput(); - downstreamChangeInput.project = sdsMergeInput.project; - downstreamChangeInput.branch = sdsMergeInput.downstreamBranch; - downstreamChangeInput.subject = - getSubjectForDownstreamMerge(sdsMergeInput.subject, sdsMergeInput.currentRevision, false); - downstreamChangeInput.topic = currentTopic; - downstreamChangeInput.merge = mergeInput; - downstreamChangeInput.notify = NotifyHandling.NONE; - - downstreamChangeInput.baseChange = - getBaseChangeId( - getChangeParents(sdsMergeInput.changeNumber, sdsMergeInput.currentRevision), - sdsMergeInput.downstreamBranch); - - if (!sdsMergeInput.doMerge) { - mergeInput.strategy = "ours"; - downstreamChangeInput.subject = - getSubjectForDownstreamMerge( - sdsMergeInput.subject, sdsMergeInput.currentRevision, true); - logger.atFine().log( - "Skipping merge for %s to %s", - sdsMergeInput.currentRevision, sdsMergeInput.downstreamBranch); - } - - ChangeApi downstreamChange = gApi.changes().create(downstreamChangeInput); - tagChange(downstreamChange.get(), "Automerger change created!"); - } - } - - /** - * Get the base change ID that the downstream change should be based off of, given the parents. - * - * <p>Given changes A and B where A is the first parent of B, and where A' is the change whose - * second parent is A, and B' is the change whose second parent is B, the first parent of B' - * should be A'. - * - * @param parents Parent commit SHAs of the change - * @return The base change ID that the change should be based off of, null if there is none. - * @throws InvalidQueryParameterException - * @throws RestApiException - */ - private String getBaseChangeId(List<String> parents, String branch) - throws InvalidQueryParameterException, RestApiException { - if (parents.isEmpty()) { - logger.atInfo().log("No base change id for change with no parents."); - return null; - } - // 1) Get topic of first parent - String firstParentTopic = getTopic(parents.get(0)); - if (firstParentTopic == null) { - return null; - } - // 2) query that topic and use that to find A' - List<ChangeInfo> changesInTopic = getChangesInTopicAndBranch(firstParentTopic, branch); - String firstParent = parents.get(0); - for (ChangeInfo change : changesInTopic) { - List<CommitInfo> topicChangeParents = - change.revisions.get(change.currentRevision).commit.parents; - if (topicChangeParents.size() > 1 && topicChangeParents.get(1).commit.equals(firstParent)) { - return String.valueOf(change._number); + ChangeApi downstreamChange = changeCreator.get().create(sdsChangeInput, currentTopic); + if(downstreamChange != null) { + ChangeUtils.tagChange(gApi, downstreamChange.get(), "Automerger change created!"); } } - return null; } private void automergeChanges(ChangeInfo change, RevisionInfo revisionInfo, Account.Id accountId) @@ -681,7 +599,7 @@ ChangeApi currentChange = gApi.changes().id(change._number); String previousRevision = getPreviousRevision(currentChange, revisionInfo._number); - MultipleDownstreamMergeInput mdsMergeInput = new MultipleDownstreamMergeInput(); + MultipleDownstreamChangeInput mdsMergeInput = new MultipleDownstreamChangeInput(); mdsMergeInput.dsBranchMap = dsBranchMap; mdsMergeInput.changeNumber = change._number; mdsMergeInput.patchsetNumber = revisionInfo._number; @@ -691,7 +609,7 @@ mdsMergeInput.obsoleteRevision = previousRevision; mdsMergeInput.currentRevision = currentRevision; - createMergesAndHandleConflicts(mdsMergeInput, accountId); + createChangesAndHandleConflicts(mdsMergeInput, accountId); } private void abandonDownstream(ChangeInfo change, String revision, Account.Id accountId) @@ -706,7 +624,7 @@ for (String downstreamBranch : downstreamBranches) { List<Integer> existingDownstream = - getExistingMergesOnBranch(revision, change.topic, downstreamBranch, accountId); + getExistingChangesOnBranch(revision, change.topic, downstreamBranch, accountId); logger.atFine().log("Abandoning existing downstreams: %s", existingDownstream); for (Integer changeNumber : existingDownstream) { abandonChange(changeNumber); @@ -725,7 +643,7 @@ labels.put(label, vote); reviewInput.labels = labels; reviewInput.notify = NotifyHandling.NONE; - reviewInput.tag = AUTOMERGER_TAG; + reviewInput.tag = ChangeUtils.AUTOMERGER_TAG; try { gApi.changes().id(change.id).revision(CURRENT).review(reviewInput); } catch (AuthException e) { @@ -733,56 +651,6 @@ } } - private void tagChange(ChangeInfo change, String message) throws RestApiException { - ReviewInput reviewInput = new ReviewInput(); - reviewInput.message(message); - reviewInput.notify = NotifyHandling.NONE; - reviewInput.tag = AUTOMERGER_TAG; - try { - gApi.changes().id(change.id).revision(CURRENT).review(reviewInput); - } catch (AuthException e) { - logger.atSevere().withCause(e).log("Automerger could not set label, but still continuing."); - } - } - - private void updateDownstreamMerge( - String newParentRevision, - String upstreamSubject, - Integer sourceNum, - boolean doMerge, - Integer upstreamChangeNumber, - String downstreamBranch) - throws RestApiException, InvalidQueryParameterException { - MergeInput mergeInput = new MergeInput(); - mergeInput.source = newParentRevision; - - MergePatchSetInput mergePatchSetInput = new MergePatchSetInput(); - - mergePatchSetInput.subject = - getSubjectForDownstreamMerge(upstreamSubject, newParentRevision, false); - if (!doMerge) { - mergeInput.strategy = "ours"; - mergePatchSetInput.subject = - getSubjectForDownstreamMerge(upstreamSubject, newParentRevision, true); - logger.atFine().log("Skipping merge for %s on %s", newParentRevision, sourceNum); - } - mergePatchSetInput.merge = mergeInput; - - mergePatchSetInput.baseChange = - getBaseChangeId( - getChangeParents(upstreamChangeNumber, newParentRevision), downstreamBranch); - - ChangeApi originalChange = gApi.changes().id(sourceNum); - - if (originalChange.info().status == ChangeStatus.ABANDONED) { - RestoreInput restoreInput = new RestoreInput(); - restoreInput.message = "Restoring change due to upstream automerge."; - originalChange.restore(restoreInput); - } - - originalChange.createMergePatchSet(mergePatchSetInput); - } - private String getPreviousRevision(ChangeApi change, int currentPatchSetNumber) throws RestApiException { String previousRevision = null; @@ -802,9 +670,24 @@ return previousRevision; } - private ChangeInfo getOriginalChange(int changeNumber, String currentRevision) + private ChangeInfo getOriginalChangeCherryPickMode(int changeNumber) throws RestApiException, InvalidQueryParameterException { - List<String> parents = getChangeParents(changeNumber, currentRevision); + + ChangeInfo current = gApi.changes().id(changeNumber).get(); + String topic = current.topic; + List<ChangeInfo> changesInTopic = getChangesInTopic(topic); + for (ChangeInfo change : changesInTopic) { + if(ChangeUtils.isDownstreamCherryPick(gApi, change.currentRevision, current)){ + return getOriginalChangeCherryPickMode(change._number); + } + } + + return current; + } + + private ChangeInfo getOriginalChangeMergeMode(int changeNumber, String currentRevision) + throws RestApiException, InvalidQueryParameterException { + List<String> parents = ChangeUtils.getChangeParents(gApi, changeNumber, currentRevision); if (parents.size() >= 2) { String secondParentRevision = parents.get(1); String topic = gApi.changes().id(changeNumber).topic(); @@ -818,114 +701,36 @@ return gApi.changes().id(changeNumber).get(); } - private List<String> getChangeParents(int changeNumber, String currentRevision) - throws RestApiException { - ChangeApi change = gApi.changes().id(changeNumber); - List<String> parents = new ArrayList<>(); - Map<String, RevisionInfo> revisionMap = - change.get(EnumSet.of(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT)) - .revisions; - List<CommitInfo> changeParents = revisionMap.get(currentRevision).commit.parents; - for (CommitInfo commit : changeParents) { - parents.add(commit.commit); + private ChangeInfo getOriginalChange(int changeNumber, String currentRevision) + throws RestApiException, InvalidQueryParameterException { + ChangeMode changeMode = ChangeMode.MERGE; + try { + changeMode = config.changeMode(); + } catch (ConfigInvalidException e) { + logger.atSevere().withCause(e).log("Automerger could not read config, but still continuing."); } - return parents; + + if(changeMode == ChangeMode.CHERRY_PICK){ + return getOriginalChangeCherryPickMode(changeNumber); + } else { + return getOriginalChangeMergeMode(changeNumber, currentRevision); + } } private void abandonChange(Integer changeNumber) throws RestApiException { logger.atFine().log("Abandoning change: %s", changeNumber); AbandonInput abandonInput = new AbandonInput(); abandonInput.notify = NotifyHandling.NONE; - abandonInput.message = "Merge parent updated; abandoning due to upstream conflict."; + abandonInput.message = "Upstream change updated; abandoning due to upstream conflict."; gApi.changes().id(changeNumber).abandon(abandonInput); } - private String getTopic(String revision) throws InvalidQueryParameterException, RestApiException { - QueryBuilder queryBuilder = new QueryBuilder(); - queryBuilder.addParameter("commit", revision); - List<ChangeInfo> changes = - gApi.changes() - .query(queryBuilder.get()) - .withOption(ListChangesOption.CURRENT_REVISION) - .get(); - if (!changes.isEmpty()) { - for (ChangeInfo change : changes) { - if (change.currentRevision.equals(revision) && !"".equals(change.topic)) { - return change.topic; - } - } - } - return null; - } - - private QueryBuilder constructTopicQuery(String topic) throws InvalidQueryParameterException { - QueryBuilder queryBuilder = new QueryBuilder(); - queryBuilder.addParameter("topic", topic); - queryBuilder.addParameter("status", "open"); - return queryBuilder; - } - private List<ChangeInfo> getChangesInTopic(String topic) throws InvalidQueryParameterException, RestApiException { - QueryBuilder queryBuilder = constructTopicQuery(topic); + QueryBuilder queryBuilder = ChangeUtils.constructTopicQuery(topic); return gApi.changes() .query(queryBuilder.get()) .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT) .get(); } - - private List<ChangeInfo> getChangesInTopicAndBranch(String topic, String downstreamBranch) - throws InvalidQueryParameterException, RestApiException { - QueryBuilder queryBuilder = constructTopicQuery(topic); - queryBuilder.addParameter("branch", downstreamBranch); - return gApi.changes() - .query(queryBuilder.get()) - .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT) - .get(); - } - - private boolean isAlreadyMerged(SingleDownstreamMergeInput sdsMergeInput, String currentTopic) - throws InvalidQueryParameterException, RestApiException { - // If we've already merged this commit to this branch, don't do it again. - List<ChangeInfo> changes = - getChangesInTopicAndBranch(currentTopic, sdsMergeInput.downstreamBranch); - for (ChangeInfo change : changes) { - if (change.branch.equals(sdsMergeInput.downstreamBranch)) { - List<CommitInfo> parents = change.revisions.get(change.currentRevision).commit.parents; - if (parents.size() > 1) { - String secondParent = parents.get(1).commit; - if (secondParent.equals(sdsMergeInput.currentRevision)) { - return true; - } - } - } - } - return false; - } - - /** - * Create subject line for downstream merge with metadata from upstream change. - * - * <p>The downstream subject will be in the format: "[automerger] upstreamSubject am: - * upstreamRevision". If it is a skip, "am" will be replaced with "skipped", and [automerger] - * replaced with [automerger skipped]. - * - * @param upstreamSubject Subject line of the upstream change - * @param upstreamRevision Commit SHA1 of the upstream change - * @param skipped Whether or not the merge is done with "-s ours" - * @return Subject line for downstream merge - */ - private String getSubjectForDownstreamMerge( - String upstreamSubject, String upstreamRevision, boolean skipped) { - if (!upstreamSubject.startsWith("[" + SUBJECT_PREFIX)) { - String prefix = "[" + SUBJECT_PREFIX + "]"; - if (skipped) { - prefix = "[" + SUBJECT_PREFIX + " " + SKIPPED_PREFIX + "]"; - } - upstreamSubject = Joiner.on(" ").join(prefix, upstreamSubject); - } - String denotationString = skipped ? "skipped:" : "am:"; - return Joiner.on(" ") - .join(upstreamSubject, denotationString, upstreamRevision.substring(0, 10)); - } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeChangeCreator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeChangeCreator.java new file mode 100644 index 0000000..7ed8e1d --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeChangeCreator.java
@@ -0,0 +1,136 @@ +package com.googlesource.gerrit.plugins.automerger; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.exceptions.StorageException; +import com.google.gerrit.extensions.api.GerritApi; +import com.google.gerrit.extensions.api.changes.ChangeApi; +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.RestoreInput; +import com.google.gerrit.extensions.client.ChangeStatus; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.ChangeInput; +import com.google.gerrit.extensions.common.CommitInfo; +import com.google.gerrit.extensions.common.MergeInput; +import com.google.gerrit.extensions.common.MergePatchSetInput; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.inject.Inject; +import java.util.List; +import org.eclipse.jgit.errors.ConfigInvalidException; + +/** + * MergeChangeCreator handles creation and updating of downstream changes as merges. + */ +public class MergeChangeCreator implements ChangeCreatorApi { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final String SUBJECT_PREFIX = "automerger"; + private final GerritApi gApi; + @Inject + public MergeChangeCreator( + GerritApi gApi) { + this.gApi = gApi; + } + /** + * Create a single downstream merge. + * + * @param sdsChangeInput Input containing metadata for the merge. + * @param currentTopic Current topic to create change in. + * @throws RestApiException + * @throws ConfigInvalidException + * @throws InvalidQueryParameterException + * @throws StorageException + */ + @Override + public ChangeApi create(SingleDownstreamChangeInput sdsChangeInput, + String currentTopic) + throws RestApiException, ConfigInvalidException, InvalidQueryParameterException, StorageException { + + if (isAlreadyMerged(sdsChangeInput, currentTopic)) { + logger.atInfo().log( + "Commit %s already merged into %s, not automerging again.", + sdsChangeInput.currentRevision, sdsChangeInput.downstreamBranch); + return null; + } + + MergeInput mergeInput = new MergeInput(); + mergeInput.source = sdsChangeInput.currentRevision; + mergeInput.strategy = "recursive"; + + logger.atFine().log("Creating downstream merge for %s", sdsChangeInput.currentRevision); + ChangeInput downstreamChangeInput = new ChangeInput(); + downstreamChangeInput.project = sdsChangeInput.project; + downstreamChangeInput.branch = sdsChangeInput.downstreamBranch; + downstreamChangeInput.subject = + ChangeUtils.getSubjectForDownstreamChange(SUBJECT_PREFIX, sdsChangeInput.subject, sdsChangeInput.currentRevision, !sdsChangeInput.doChange); + downstreamChangeInput.topic = currentTopic; + downstreamChangeInput.merge = mergeInput; + downstreamChangeInput.notify = NotifyHandling.NONE; + + downstreamChangeInput.baseChange = + ChangeUtils.getBaseChangeIdForMerge(gApi, + ChangeUtils.getChangeParents(gApi, sdsChangeInput.changeNumber, sdsChangeInput.currentRevision), + sdsChangeInput.downstreamBranch); + + if (!sdsChangeInput.doChange) { + mergeInput.strategy = "ours"; + logger.atFine().log( + "Skipping merge for %s to %s", + sdsChangeInput.currentRevision, sdsChangeInput.downstreamBranch); + } + + return gApi.changes().create(downstreamChangeInput); + } + + @Override + public void update(UpdateDownstreamChangeInput updateDownstreamChangeInput) + throws RestApiException, ConfigInvalidException, InvalidQueryParameterException { + + MergeInput mergeInput = new MergeInput(); + mergeInput.source = updateDownstreamChangeInput.upstreamRevision; + + MergePatchSetInput mergePatchSetInput = new MergePatchSetInput(); + + mergePatchSetInput.subject = + ChangeUtils.getSubjectForDownstreamChange(SUBJECT_PREFIX, + updateDownstreamChangeInput.upstreamSubject, + updateDownstreamChangeInput.upstreamRevision, !updateDownstreamChangeInput.doChange); + if (!updateDownstreamChangeInput.doChange) { + mergeInput.strategy = "ours"; + logger.atFine().log("Skipping merge for %s on %s", + updateDownstreamChangeInput.upstreamRevision, updateDownstreamChangeInput.downstreamChangeNumber); + } + mergePatchSetInput.merge = mergeInput; + + mergePatchSetInput.baseChange = + ChangeUtils.getBaseChangeIdForMerge(gApi, + ChangeUtils.getChangeParents(gApi, updateDownstreamChangeInput.upstreamChangeNumber, + updateDownstreamChangeInput.upstreamRevision), updateDownstreamChangeInput.downstreamBranch); + + ChangeApi originalChange = gApi.changes().id(updateDownstreamChangeInput.downstreamChangeNumber); + + if (originalChange.info().status == ChangeStatus.ABANDONED) { + RestoreInput restoreInput = new RestoreInput(); + restoreInput.message = "Restoring change due to upstream automerge."; + originalChange.restore(restoreInput); + } + + originalChange.createMergePatchSet(mergePatchSetInput); + } + + boolean isAlreadyMerged(SingleDownstreamChangeInput sdsChangeInput, String currentTopic) + throws InvalidQueryParameterException, RestApiException { + // If we've already merged this commit to this branch, don't do it again. + List<ChangeInfo> changes = + ChangeUtils.getChangesInTopicAndBranch(gApi, currentTopic, sdsChangeInput.downstreamBranch); + for (ChangeInfo change : changes) { + List<CommitInfo> parents = change.revisions.get(change.currentRevision).commit.parents; + if (parents.size() > 1) { + String secondParent = parents.get(1).commit; + if (secondParent.equals(sdsChangeInput.currentRevision)) { + return true; + } + } + } + return false; + } + +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java index 16459a6..b5c13fc 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java
@@ -23,8 +23,6 @@ import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.common.CommitInfo; -import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.CodeReviewCommit; @@ -105,6 +103,7 @@ throws RestApiException, IOException, ConfigInvalidException, InvalidQueryParameterException { Set<String> missingDownstreamBranches = new HashSet<>(); + ChangeMode changeMode = config.changeMode(); Set<String> downstreamBranches = config.getDownstreamBranches(upstreamChange.branch, upstreamChange.project); for (String downstreamBranch : downstreamBranches) { @@ -115,6 +114,9 @@ missingDownstreamBranches.add(downstreamBranch); continue; } + if(cherryPickSkipped(upstreamChange, downstreamBranch)){ + continue; + } queryBuilder.addParameter("topic", upstreamChange.topic); queryBuilder.addParameter("branch", downstreamBranch); queryBuilder.addParameter("status", "open"); @@ -124,14 +126,9 @@ .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_COMMIT) .get(); for (ChangeInfo change : changes) { - RevisionInfo revision = change.revisions.get(change.currentRevision); - List<CommitInfo> parents = revision.commit.parents; - if (parents.size() > 1) { - String secondParent = parents.get(1).commit; - if (secondParent.equals(upstreamChange.currentRevision)) { - dsExists = true; - break; - } + if(ChangeUtils.isDownstreamChange(gApi, upstreamChange.currentRevision, change, changeMode)) { + dsExists = true; + break; } } if (!dsExists) { @@ -140,4 +137,16 @@ } return missingDownstreamBranches; } + + private boolean cherryPickSkipped(ChangeInfo change, String downstreamBranch){ + try { + if(config.changeMode() == ChangeMode.MERGE){ + return false; + } + } catch (ConfigInvalidException e) { + return false; + } + + return change.hashtags.contains(ChangeUtils.getSkipHashtag(downstreamBranch)); + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/MultipleDownstreamMergeInput.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/MultipleDownstreamChangeInput.java similarity index 85% rename from src/main/java/com/googlesource/gerrit/plugins/automerger/MultipleDownstreamMergeInput.java rename to src/main/java/com/googlesource/gerrit/plugins/automerger/MultipleDownstreamChangeInput.java index a62d9a7..476c5d4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/automerger/MultipleDownstreamMergeInput.java +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/MultipleDownstreamChangeInput.java
@@ -17,9 +17,10 @@ import java.util.Map; /** - * Class to hold input for a set of merges from a single source change, with associated metadata. + * Class to hold input for a set of downstream changes from a single source change, with associated + * metadata. */ -public class MultipleDownstreamMergeInput { +public class MultipleDownstreamChangeInput { public Map<String, Boolean> dsBranchMap; public int changeNumber; public int patchsetNumber;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/SingleDownstreamMergeInput.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/SingleDownstreamChangeInput.java similarity index 84% rename from src/main/java/com/googlesource/gerrit/plugins/automerger/SingleDownstreamMergeInput.java rename to src/main/java/com/googlesource/gerrit/plugins/automerger/SingleDownstreamChangeInput.java index 5a5f0e7..6804d70 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/automerger/SingleDownstreamMergeInput.java +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/SingleDownstreamChangeInput.java
@@ -14,8 +14,8 @@ package com.googlesource.gerrit.plugins.automerger; -/** Class to hold input for a merge for a single source change and destination branch. */ -public class SingleDownstreamMergeInput { +/** Class to hold input for a downstream change from a single source change. */ +public class SingleDownstreamChangeInput { public String currentRevision; public int changeNumber; public int patchsetNumber; @@ -23,5 +23,5 @@ public String topic; public String subject; public String downstreamBranch; - public boolean doMerge; + public boolean doChange; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/UpdateDownstreamChangeInput.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/UpdateDownstreamChangeInput.java new file mode 100644 index 0000000..edaa6cc --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/UpdateDownstreamChangeInput.java
@@ -0,0 +1,13 @@ +package com.googlesource.gerrit.plugins.automerger; + +/** Class to hold input for a downstream change update from a single source change. */ +public class UpdateDownstreamChangeInput { + public String upstreamRevision; + public String upstreamSubject; + public int downstreamChangeNumber; + public int upstreamChangeNumber; + public int patchSetNumber; + public String downstreamBranch; + public String topic; + public boolean doChange; +}
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md index 13f69f8..1e17aa7 100644 --- a/src/main/resources/Documentation/about.md +++ b/src/main/resources/Documentation/about.md
@@ -17,3 +17,5 @@ A UI button "Recreate automerges" has been added so that users can skip downstream merges. Unchecking a branch's checkbox will skip that branch and all automerges downstream of that branch. + +The plugin can be configured to perform cherry-picks instead of merges. \ No newline at end of file
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 6e629b3..b494fcb 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -17,6 +17,7 @@ blankMerge = .*RESTRICT AUTOMERGE.* blankMerge = .*SKIP UNLESS MERGEALL SET.* missingDownstreamsMessage = there is no ${missingDownstreams} + cherryPickMode = False [automerger "branch1:branch2"] setProjects = some/project @@ -133,6 +134,12 @@ credentials of this user ID instead of the credentials of the user doing the upstream operation. +global.cherryPickMode +: If this is true, cherry-picks will be executed instead of merges. + +When a change is skipped in cherry-pick mode, a downstream change is not created +and a "am_skip_<branch>" hashtag is added to the upstream commit. + automerger.branch1:branch2.setProjects : Projects to automerge for.
diff --git a/src/main/resources/Documentation/rest-api.md b/src/main/resources/Documentation/rest-api.md index 3b0b5b6..a25522f 100644 --- a/src/main/resources/Documentation/rest-api.md +++ b/src/main/resources/Documentation/rest-api.md
@@ -85,4 +85,25 @@ ``` HTTP/1.1 204 No Content -``` \ No newline at end of file +``` + +### <a id="config-downstream"> Cherry Pick Mode +GET /config/server/automerger~automerge-mode + +Returns either "CHERRY-PICK" or "MERGE". + +#### Request + +``` + GET /config/server/automerger~automerge-mode HTTP/1.1 +``` + +#### Response + +``` + HTTP/1.1 200 OK + Content-Disposition: attachment + Content-Type: application/json;charset=utf-8 + )]}' + "CHERRY-PICK" +```
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java index 254942f..3a5f8b3 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
@@ -18,6 +18,7 @@ import static com.google.common.truth.OptionalSubject.optionals; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow; import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel; import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.blockLabel; import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.labelPermissionKey; @@ -41,12 +42,14 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.BranchNameKey; +import com.google.gerrit.entities.Permission; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.api.accounts.AccountApi; import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.RebaseInput; import com.google.gerrit.extensions.api.changes.ReviewInput; +import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.common.ApprovalInfo; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeMessageInfo; @@ -88,8 +91,7 @@ return cfg; } - @Test - public void testExpectedFlow() throws Exception { + private void expectedFlow(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Create initial change PushOneCommit.Result result = @@ -98,7 +100,7 @@ String projectName = result.getChange().project().get(); createBranch(BranchNameKey.create(projectName, "ds_one")); createBranch(BranchNameKey.create(projectName, "ds_two")); - pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two"); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", changeMode); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId()); result.assertOkStatus(); @@ -109,7 +111,6 @@ .withOption(CURRENT_REVISION) .get(); assertThat(changesInTopic).hasSize(3); - List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic); ChangeInfo dsOneChangeInfo = sortedChanges.get(0); @@ -127,21 +128,32 @@ // Ensure that commit subjects are correct String masterSubject = masterChangeInfo.subject; String shortMasterSha = masterChangeInfo.currentRevision.substring(0, 10); - assertThat(masterChangeInfo.subject).doesNotContainMatch("automerger"); + String tag = getTag(changeMode); + assertThat(masterChangeInfo.subject).doesNotContainMatch(tag); assertThat(dsOneChangeInfo.subject) - .isEqualTo("[automerger] " + masterSubject + " am: " + shortMasterSha); + .isEqualTo("[" + tag + "] " + masterSubject + " am: " + shortMasterSha); assertThat(dsTwoChangeInfo.subject) - .isEqualTo("[automerger] " + masterSubject + " am: " + shortMasterSha); + .isEqualTo("[" + tag + "] " + masterSubject + " am: " + shortMasterSha); // +2 and submit merge(result); assertCodeReview(masterChangeInfo.id, 2, null); assertCodeReview(dsOneChangeInfo.id, 2, "autogenerated:Automerger"); assertCodeReview(dsTwoChangeInfo.id, 2, "autogenerated:Automerger"); + } @Test - public void testDiamondMerge() throws Exception { + public void testExpectedFlow() throws Exception { + expectedFlow(ChangeMode.MERGE); + } + + @Test + public void testExpectedFlowCherryPickMode() throws Exception { + expectedFlow(ChangeMode.CHERRY_PICK); + } + + private void diamondMerge(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId(); // Create initial change @@ -172,7 +184,7 @@ // For this test, right != left assertThat(leftRevision).isNotEqualTo(rightRevision); createBranch(BranchNameKey.create(projectName, "bottom")); - pushDiamondConfig(manifestNameKey.get(), projectName); + pushDiamondConfig(manifestNameKey.get(), projectName, changeMode); // After we upload our config, we upload a new patchset to create the downstreams testRepo.reset(masterWithMergedChange); PushOneCommit.Result result = @@ -210,39 +222,55 @@ assertThat(rightChangeInfo.branch).isEqualTo("right"); assertCodeReview(rightChangeInfo.id, 2, "autogenerated:Automerger"); + String tag = getTag(changeMode); + // Ensure that commit subjects are correct String masterSubject = masterChangeInfo.subject; String shortMasterSha = masterChangeInfo.currentRevision.substring(0, 10); String shortLeftSha = leftChangeInfo.currentRevision.substring(0, 10); String shortRightSha = rightChangeInfo.currentRevision.substring(0, 10); - assertThat(masterChangeInfo.subject).doesNotContainMatch("automerger"); + assertThat(masterChangeInfo.subject).doesNotContainMatch(tag); assertThat(leftChangeInfo.subject) - .isEqualTo("[automerger] " + masterSubject + " am: " + shortMasterSha); + .isEqualTo("[" + tag + "] " + masterSubject + " am: " + shortMasterSha); assertThat(rightChangeInfo.subject) - .isEqualTo("[automerger] " + masterSubject + " am: " + shortMasterSha); + .isEqualTo("[" + tag + "] " + masterSubject + " am: " + shortMasterSha); // Either bottomChangeInfoA came from left and bottomChangeInfoB came from right, or vice versa // We don't know which, so we use the if condition to check - String bottomChangeInfoASecondParent = getParent(bottomChangeInfoA, 1); - if (bottomChangeInfoASecondParent.equals(leftChangeInfo.currentRevision)) { + String bottomChangeInfoATarget = ""; + if(changeMode == ChangeMode.CHERRY_PICK){ + bottomChangeInfoATarget = getCherryPickFrom(bottomChangeInfoA); + } else { + bottomChangeInfoATarget = getParent(bottomChangeInfoA, 1); + } + if (bottomChangeInfoATarget.equals(leftChangeInfo.currentRevision)) { assertThat(bottomChangeInfoA.subject) .isEqualTo( - "[automerger] " + masterSubject + " am: " + shortMasterSha + " am: " + shortLeftSha); + "[" + tag + "] " + masterSubject + " am: " + shortMasterSha + " am: " + shortLeftSha); assertThat(bottomChangeInfoB.subject) .isEqualTo( - "[automerger] " + masterSubject + " am: " + shortMasterSha + " am: " + shortRightSha); + "[" + tag + "] " + masterSubject + " am: " + shortMasterSha + " am: " + shortRightSha); } else { assertThat(bottomChangeInfoA.subject) .isEqualTo( - "[automerger] " + masterSubject + " am: " + shortMasterSha + " am: " + shortRightSha); + "[" + tag + "] " + masterSubject + " am: " + shortMasterSha + " am: " + shortRightSha); assertThat(bottomChangeInfoB.subject) .isEqualTo( - "[automerger] " + masterSubject + " am: " + shortMasterSha + " am: " + shortLeftSha); + "[" + tag + "] " + masterSubject + " am: " + shortMasterSha + " am: " + shortLeftSha); } } @Test - public void testChangeStack() throws Exception { + public void testDiamondMerge() throws Exception { + diamondMerge(ChangeMode.MERGE); + } + + @Test + public void testDiamondMergeCherryPickMode() throws Exception { + diamondMerge(ChangeMode.CHERRY_PICK); + } + + private void changeStack(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Create initial change PushOneCommit.Result result = @@ -250,7 +278,7 @@ // Project name is scoped by test, so we need to get it from our initial change String projectName = result.getChange().project().get(); createBranch(BranchNameKey.create(projectName, "ds_one")); - pushSimpleConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one"); + pushSimpleConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", changeMode); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId()); result.assertOkStatus(); @@ -286,13 +314,23 @@ assertThat(bFirstParent).isEqualTo(a.currentRevision); // Ensure that commit subjects are correct + String tag = getTag(changeMode); String shortASha = a.currentRevision.substring(0, 10); - assertThat(a.subject).doesNotContainMatch("automerger"); - assertThat(aPrime.subject).isEqualTo("[automerger] test commit am: " + shortASha); + assertThat(a.subject).doesNotContainMatch(tag); + assertThat(aPrime.subject).isEqualTo("[" + tag + "] test commit am: " + shortASha); } @Test - public void testChangeStack_rebaseAfterUpload() throws Exception { + public void testChangeStack() throws Exception { + changeStack(ChangeMode.MERGE); + } + + @Test + public void testChangeStackCherryPickMode() throws Exception { + changeStack(ChangeMode.CHERRY_PICK); + } + + private void changeStack_rebaseAfterUpload(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Save initial ref at HEAD ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId(); @@ -302,7 +340,7 @@ // Project name is scoped by test, so we need to get it from our initial change String projectName = result.getChange().project().get(); createBranch(BranchNameKey.create(projectName, "ds_one")); - pushSimpleConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one"); + pushSimpleConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", changeMode); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId()); result.assertOkStatus(); @@ -352,12 +390,36 @@ String bAfterRebaseFirstParent = getParent(bAfterRebase, 0); assertThat(bAfterRebaseFirstParent).isEqualTo(a.currentRevision); + // In cherry-pick mode, we expect original bPrime to be removed, and a new bPrime to + // be created. + if(changeMode == ChangeMode.CHERRY_PICK){ + assertThat(gApi.changes().id(bPrime.id).get().status).isEqualTo(ChangeStatus.ABANDONED); + changesInTopic = + gApi.changes() + .query("topic: " + gApi.changes().id(result.getChangeId()).topic() + " status:open") + .withOptions(ALL_REVISIONS, CURRENT_COMMIT) + .get(); + assertThat(changesInTopic).hasSize(4); + sortedChanges = sortedChanges(changesInTopic); + bPrime = sortedChanges.get(1); + } + ChangeInfo bPrimeAfterRebase = gApi.changes().id(bPrime.changeId).get(); String bPrimeAfterRebaseFirstParent = getParent(bPrimeAfterRebase, 0); assertThat(bPrimeAfterRebaseFirstParent).isEqualTo(aPrime.currentRevision); } @Test + public void testChangeStack_rebaseAfterUpload() throws Exception { + changeStack_rebaseAfterUpload(ChangeMode.MERGE); + } + + @Test + public void testChangeStack_rebaseAfterUploadCherryPickMode() throws Exception { + changeStack_rebaseAfterUpload(ChangeMode.CHERRY_PICK); + } + + @Test public void testBlankMerge() throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Create initial change @@ -368,7 +430,7 @@ String projectName = result.getChange().project().get(); createBranch(BranchNameKey.create(projectName, "ds_one")); createBranch(BranchNameKey.create(projectName, "ds_two")); - pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two"); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", ChangeMode.MERGE); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId(), "DO NOT MERGE subject", "filename", "content"); result.assertOkStatus(); @@ -415,6 +477,52 @@ } @Test + public void testBlankMergeCherryPickMode() throws Exception { + Project.NameKey manifestNameKey = defaultSetup(); + // Create initial change + PushOneCommit.Result result = + createChange( + testRepo, "master", "DO NOT MERGE subject", "filename", "content", "testtopic"); + // Project name is scoped by test, so we need to get it from our initial change + String projectName = result.getChange().project().get(); + createBranch(BranchNameKey.create(projectName, "ds_one")); + createBranch(BranchNameKey.create(projectName, "ds_two")); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", ChangeMode.CHERRY_PICK); + // After we upload our config, we upload a new patchset to create the downstreams + amendChange(result.getChangeId(), "DO NOT MERGE subject", "filename", "content"); + result.assertOkStatus(); + + ChangeApi change = gApi.changes().id(result.getChangeId()); + BinaryResult content = change.current().file("filename").content(); + + List<ChangeInfo> changesInTopic = + gApi.changes().query("topic: " + change.topic()).withOption(CURRENT_REVISION).get(); + assertThat(changesInTopic).hasSize(2); + + List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic); + + ChangeInfo dsTwoChangeInfo = sortedChanges.get(0); + assertThat(dsTwoChangeInfo.branch).isEqualTo("ds_two"); + assertAutomergerChangeCreatedMessage(dsTwoChangeInfo.id); + // It should not skip ds_two, since it is marked with mergeAll: true + ChangeApi dsTwoChange = gApi.changes().id(dsTwoChangeInfo._number); + assertThat(dsTwoChange.get().subject).doesNotContain("skipped:"); + BinaryResult dsTwoContent = dsTwoChange.current().file("filename").content(); + assertThat(dsTwoContent.asString()).isEqualTo(content.asString()); + + ChangeInfo masterChangeInfo = sortedChanges.get(1); + assertCodeReviewMissing(masterChangeInfo.id); + assertThat(masterChangeInfo.branch).isEqualTo("master"); + + // Ensure that commit subjects are correct + String masterSubject = masterChangeInfo.subject; + String shortMasterSha = masterChangeInfo.currentRevision.substring(0, 10); + assertThat(masterChangeInfo.subject).doesNotContainMatch("autocherry"); + assertThat(dsTwoChangeInfo.subject) + .isEqualTo("[autocherry] " + masterSubject + " am: " + shortMasterSha); + } + + @Test public void testAlwaysBlankMerge() throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Create initial change @@ -430,7 +538,7 @@ String projectName = result.getChange().project().get(); createBranch(BranchNameKey.create(projectName, "ds_one")); createBranch(BranchNameKey.create(projectName, "ds_two")); - pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two"); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", ChangeMode.MERGE); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId(), "DO NOT MERGE ANYWHERE subject", "filename", "content"); result.assertOkStatus(); @@ -475,7 +583,36 @@ } @Test - public void testDownstreamMergeConflict() throws Exception { + public void testAlwaysBlankMergeCherryPickMode() throws Exception { + Project.NameKey manifestNameKey = defaultSetup(); + // Create initial change + PushOneCommit.Result result = + createChange( + testRepo, + "master", + "DO NOT MERGE ANYWHERE subject", + "filename", + "content", + "testtopic"); + // Project name is scoped by test, so we need to get it from our initial change + String projectName = result.getChange().project().get(); + createBranch(BranchNameKey.create(projectName, "ds_one")); + createBranch(BranchNameKey.create(projectName, "ds_two")); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", ChangeMode.CHERRY_PICK); + // After we upload our config, we upload a new patchset to create the downstreams + amendChange(result.getChangeId(), "DO NOT MERGE ANYWHERE subject", "filename", "content"); + result.assertOkStatus(); + + ChangeApi change = gApi.changes().id(result.getChangeId()); + List<ChangeInfo> changesInTopic = + gApi.changes().query("topic: " + change.topic()).withOption(CURRENT_REVISION).get(); + assertThat(changesInTopic).hasSize(1); + assertThat(changesInTopic.get(0).hashtags.size()).isEqualTo(2); + assertThat(changesInTopic.get(0).hashtags.contains("am_skip_ds_one")).isEqualTo(true); + assertThat(changesInTopic.get(0).hashtags.contains("am_skip_ds_two")).isEqualTo(true); + } + + private void downstreamMergeConflict(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId(); // Create initial change @@ -496,7 +633,7 @@ merge(ds1Result); // Reset to allow our merge conflict to come testRepo.reset(initial); - pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two"); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", changeMode); // After we upload our config, we upload a new change to create the downstreams PushOneCommit.Result masterResult = pushFactory @@ -532,13 +669,23 @@ // Ensure that commit subjects are correct String masterSubject = masterChangeInfo.subject; String shortMasterSha = masterChangeInfo.currentRevision.substring(0, 10); + String tag = getTag(changeMode); assertThat(masterChangeInfo.subject).doesNotContainMatch("automerger"); assertThat(dsTwoChangeInfo.subject) - .isEqualTo("[automerger] " + masterSubject + " am: " + shortMasterSha); + .isEqualTo("[" + tag + "] " + masterSubject + " am: " + shortMasterSha); } @Test - public void testRestrictedVotePermissions() throws Exception { + public void testDownstreamMergeConflict() throws Exception { + downstreamMergeConflict(ChangeMode.MERGE); + } + + @Test + public void testDownstreamMergeConflictCherryPickMode() throws Exception { + downstreamMergeConflict(ChangeMode.CHERRY_PICK); + } + + private void restrictedVotePermissions(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId(); // Create initial change @@ -559,7 +706,7 @@ merge(ds1Result); // Reset to allow our merge conflict to come testRepo.reset(initial); - pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two"); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", changeMode); // Block Code Review label to test restrictions projectOperations @@ -606,15 +753,25 @@ assertThat(messages).contains("Patch Set 1:\n\nMerge conflict found on ds_one"); // Ensure that commit subjects are correct + String tag = getTag(changeMode); String masterSubject = masterChangeInfo.subject; String shortMasterSha = masterChangeInfo.currentRevision.substring(0, 10); assertThat(masterChangeInfo.subject).doesNotContainMatch("automerger"); assertThat(dsTwoChangeInfo.subject) - .isEqualTo("[automerger] " + masterSubject + " am: " + shortMasterSha); + .isEqualTo("[" + tag + "] " + masterSubject + " am: " + shortMasterSha); } @Test - public void testTopicEditedListener() throws Exception { + public void testRestrictedVotePermissions() throws Exception { + restrictedVotePermissions(ChangeMode.MERGE); + } + + @Test + public void testRestrictedVotePermissionsCherryPickMode() throws Exception { + restrictedVotePermissions(ChangeMode.CHERRY_PICK); + } + + private void topicEditedListener(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Create initial change PushOneCommit.Result result = @@ -623,12 +780,47 @@ String projectName = result.getChange().project().get(); createBranch(BranchNameKey.create(projectName, "ds_one")); createBranch(BranchNameKey.create(projectName, "ds_two")); - pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two"); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", changeMode); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId()); result.assertOkStatus(); - gApi.changes().id(result.getChangeId()).topic("multiple words"); - gApi.changes().id(result.getChangeId()).topic("singlewordagain"); + gApi.changes().id(result.getChangeId()).topic(name("multiple words")); + gApi.changes().id(result.getChangeId()).topic(name("singlewordagain")); + // Check that there are the correct number of changes in the topic + List<ChangeInfo> changesInTopic = + gApi.changes() + .query("topic:\"" + gApi.changes().id(result.getChangeId()).topic() + "\"") + .get(); + assertThat(changesInTopic).hasSize(3); + // +2 and submit + merge(result); + } + + @Test + public void testTopicEditedListener() throws Exception { + topicEditedListener(ChangeMode.MERGE); + } + + @Test + public void testTopicEditedListenerCherryPickMode() throws Exception { + topicEditedListener(ChangeMode.CHERRY_PICK); + } + + private void topicEditedListener_withBraces(ChangeMode changeMode) throws Exception { + Project.NameKey manifestNameKey = defaultSetup(); + // Create initial change + PushOneCommit.Result result = + createChange(testRepo, "master", "subject", "filename", "content", "testtopic"); + // Project name is scoped by test, so we need to get it from our initial change + String projectName = result.getChange().project().get(); + createBranch(BranchNameKey.create(projectName, "ds_one")); + createBranch(BranchNameKey.create(projectName, "ds_two")); + pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two", changeMode); + // After we upload our config, we upload a new patchset to create the downstreams + amendChange(result.getChangeId()); + result.assertOkStatus(); + gApi.changes().id(result.getChangeId()).topic(name("multiple words")); + gApi.changes().id(result.getChangeId()).topic(name("with{braces}inside")); // Check that there are the correct number of changes in the topic List<ChangeInfo> changesInTopic = gApi.changes() @@ -641,32 +833,15 @@ @Test public void testTopicEditedListener_withBraces() throws Exception { - Project.NameKey manifestNameKey = defaultSetup(); - // Create initial change - PushOneCommit.Result result = - createChange(testRepo, "master", "subject", "filename", "content", "testtopic"); - // Project name is scoped by test, so we need to get it from our initial change - String projectName = result.getChange().project().get(); - createBranch(BranchNameKey.create(projectName, "ds_one")); - createBranch(BranchNameKey.create(projectName, "ds_two")); - pushDefaultConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two"); - // After we upload our config, we upload a new patchset to create the downstreams - amendChange(result.getChangeId()); - result.assertOkStatus(); - gApi.changes().id(result.getChangeId()).topic("multiple words"); - gApi.changes().id(result.getChangeId()).topic("with{braces}inside"); - // Check that there are the correct number of changes in the topic - List<ChangeInfo> changesInTopic = - gApi.changes() - .query("topic:\"" + gApi.changes().id(result.getChangeId()).topic() + "\"") - .get(); - assertThat(changesInTopic).hasSize(3); - // +2 and submit - merge(result); + topicEditedListener_withBraces(ChangeMode.MERGE); } @Test - public void testTopicEditedListener_branchWithBracesAndQuotes() throws Exception { + public void testTopicEditedListener_withBracesCherryPickMode() throws Exception { + topicEditedListener_withBraces(ChangeMode.CHERRY_PICK); + } + + private void topicEditedListener_branchWithBracesAndQuotes(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Create initial change PushOneCommit.Result result = @@ -680,7 +855,8 @@ manifestNameKey.get(), projectName, "branch{}braces", - "branch\"quotes"); + "branch\"quotes", + changeMode); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId()); result.assertOkStatus(); @@ -695,7 +871,16 @@ } @Test - public void testTopicEditedListener_emptyTopic() throws Exception { + public void testTopicEditedListener_branchWithBracesAndQuotes() throws Exception { + topicEditedListener_branchWithBracesAndQuotes(ChangeMode.MERGE); + } + + @Test + public void testTopicEditedListener_branchWithBracesAndQuotesCherryPickMode() throws Exception { + topicEditedListener_branchWithBracesAndQuotes(ChangeMode.CHERRY_PICK); + } + + private void topicEditedListener_emptyTopic(ChangeMode changeMode) throws Exception { Project.NameKey manifestNameKey = defaultSetup(); // Create initial change PushOneCommit.Result result = @@ -703,7 +888,7 @@ // Project name is scoped by test, so we need to get it from our initial change String projectName = result.getChange().project().get(); createBranch(BranchNameKey.create(projectName, "ds_one")); - pushSimpleConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one"); + pushSimpleConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", changeMode); // After we upload our config, we upload a new patchset to create the downstreams amendChange(result.getChangeId()); result.assertOkStatus(); @@ -716,7 +901,16 @@ } @Test - public void testContextUser() throws Exception { + public void testTopicEditedListener_emptyTopic() throws Exception { + topicEditedListener_emptyTopic(ChangeMode.MERGE); + } + + @Test + public void testTopicEditedListener_emptyTopicCherryPickMode() throws Exception { + topicEditedListener_emptyTopic(ChangeMode.CHERRY_PICK); + } + + private void contextUser(ChangeMode changeMode) throws Exception { // Branch flow for contextUser is master -> ds_one -> ds_two Project.NameKey manifestNameKey = defaultSetup(); // Create initial change @@ -746,9 +940,28 @@ .group(AccountGroup.UUID.parse(gApi.groups().id(contextUserGroup).get().id)) .range(-2, 2)) .setExclusiveGroup(labelPermissionKey("Code-Review").ref("refs/heads/ds_one"), true) + .add( + allow(Permission.EDIT_HASHTAGS) + .ref("refs/heads/*") + .group(AccountGroup.UUID.parse(gApi.groups().id(contextUserGroup).get().id)) + ) .update(); + + if(changeMode == ChangeMode.CHERRY_PICK) { + // Grant EDIT_HASHTAGS permission + projectOperations + .project(projectNameKey) + .forUpdate() + .add( + allow(Permission.EDIT_HASHTAGS) + .ref("refs/heads/*") + .group(AccountGroup.UUID.parse(gApi.groups().id(contextUserGroup).get().id)) + ) + .update(); + } + pushContextUserConfig( - manifestNameKey.get(), projectName, contextUserApi.get()._accountId.toString()); + manifestNameKey.get(), projectName, contextUserApi.get()._accountId.toString(), changeMode); // After we upload our config, we upload a new patchset to create the downstreams PushOneCommit.Result result = @@ -783,7 +996,16 @@ } @Test - public void testContextUser_downstreamHighestVote() throws Exception { + public void testContextUser() throws Exception { + contextUser(ChangeMode.MERGE); + } + + @Test + public void testContextUserCherryPickMode() throws Exception { + contextUser(ChangeMode.CHERRY_PICK); + } + + private void contextUser_downstreamHighestVote(ChangeMode changeMode) throws Exception { // Branch flow for contextUser is master -> ds_one -> ds_two Project.NameKey manifestNameKey = defaultSetup(); // Create initial change @@ -813,8 +1035,22 @@ .group(AccountGroup.UUID.parse(gApi.groups().id(contextUserGroup).get().id)) .range(-2, 2)) .update(); + + if(changeMode == ChangeMode.CHERRY_PICK) { + // Grant EDIT_HASHTAGS permission + projectOperations + .project(projectNameKey) + .forUpdate() + .add( + allow(Permission.EDIT_HASHTAGS) + .ref("refs/heads/*") + .group(AccountGroup.UUID.parse(gApi.groups().id(contextUserGroup).get().id)) + ) + .update(); + } + pushContextUserConfig( - manifestNameKey.get(), projectName, contextUserApi.get()._accountId.toString()); + manifestNameKey.get(), projectName, contextUserApi.get()._accountId.toString(), changeMode); // After we upload our config, we upload a new patchset to create the downstreams PushOneCommit.Result result = @@ -858,7 +1094,16 @@ } @Test - public void testContextUser_mergeConflictOnDownstreamVotesOnTopLevel() throws Exception { + public void testContextUser_downstreamHighestVote() throws Exception { + contextUser_downstreamHighestVote(ChangeMode.MERGE); + } + + @Test + public void testContextUser_downstreamHighestVoteCherryPickMode() throws Exception { + contextUser_downstreamHighestVote(ChangeMode.CHERRY_PICK); + } + + private void contextUser_mergeConflictOnDownstreamVotesOnTopLevel(ChangeMode changeMode) throws Exception { // Branch flow for contextUser is master -> ds_one -> ds_two Project.NameKey manifestNameKey = defaultSetup(); ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId(); @@ -906,8 +1151,22 @@ .group(AccountGroup.UUID.parse(gApi.groups().id(contextUserGroup).get().id)) .range(-2, 2)) .update(); + + if(changeMode == ChangeMode.CHERRY_PICK) { + // Grant EDIT_HASHTAGS permission + projectOperations + .project(projectNameKey) + .forUpdate() + .add( + allow(Permission.EDIT_HASHTAGS) + .ref("refs/heads/*") + .group(AccountGroup.UUID.parse(gApi.groups().id(contextUserGroup).get().id)) + ) + .update(); + } + pushContextUserConfig( - manifestNameKey.get(), projectName, contextUserApi.get()._accountId.toString()); + manifestNameKey.get(), projectName, contextUserApi.get()._accountId.toString(), changeMode); // After we upload our config, we upload a new patchset to create the downstreams PushOneCommit.Result result = @@ -919,15 +1178,195 @@ .query("topic: " + gApi.changes().id(result.getChangeId()).topic()) .withOptions(CURRENT_REVISION, CURRENT_COMMIT) .get(); - // There should only be two, as ds_one to ds_two should be a merge conflict - assertThat(changesInTopic).hasSize(2); + if(changeMode == ChangeMode.MERGE) { + // In merge mode, there should be a conflict + // There should only be two, as ds_one to ds_two should be a merge conflict + assertThat(changesInTopic).hasSize(2); + + List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic); + + // Check that master is at Code-Review -2 + ChangeInfo masterChangeInfo = sortedChanges.get(1); + assertThat(masterChangeInfo.branch).isEqualTo("master"); + assertCodeReview(masterChangeInfo.id, -2, "autogenerated:MergeConflict"); + } else { + // In cherry-pick mode, there will not be a conflict + assertThat(changesInTopic).hasSize(3); + + // Check that master is at Code-Review 0 + List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic); + ChangeInfo masterChangeInfo = sortedChanges.get(2); + assertThat(masterChangeInfo.branch).isEqualTo("master"); + assertCodeReviewMissing(masterChangeInfo.id); + } + } + + @Test + public void testContextUser_mergeConflictOnDownstreamVotesOnTopLevel() throws Exception { + contextUser_mergeConflictOnDownstreamVotesOnTopLevel(ChangeMode.MERGE); + } + + @Test + public void testContextUser_mergeConflictOnDownstreamVotesOnTopLevelCherryPickMode() throws Exception { + contextUser_mergeConflictOnDownstreamVotesOnTopLevel(ChangeMode.CHERRY_PICK); + } + + private void abandon(ChangeMode changeMode) throws Exception { + Project.NameKey manifestNameKey = defaultSetup(); + // Create initial change + PushOneCommit.Result result = + createChange(testRepo, "master", "subject", "filename", "content", "testtopic"); + // Project name is scoped by test, so we need to get it from our initial change + String projectName = result.getChange().project().get(); + createBranch(BranchNameKey.create(projectName, "ds_one")); + createBranch(BranchNameKey.create(projectName, "ds_two")); + createBranch(BranchNameKey.create(projectName, "ds_three")); + pushFourInChainConfig("automerger.config", manifestNameKey.get(), projectName, changeMode); + // After we upload our config, we upload a new patchset to create the downstreams + amendChange(result.getChangeId()); + result.assertOkStatus(); + // Check that there are the correct number of changes in the topic + List<ChangeInfo> changesInTopic = + gApi.changes() + .query("topic: " + gApi.changes().id(result.getChangeId()).topic()) + .withOptions(ALL_REVISIONS, CURRENT_COMMIT) + .get(); + assertThat(changesInTopic).hasSize(4); List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic); + ChangeInfo masterChangeInfo = sortedChanges.get(3); - // Check that master is at Code-Review -2 - ChangeInfo masterChangeInfo = sortedChanges.get(1); - assertThat(masterChangeInfo.branch).isEqualTo("master"); + // All changes should be abandoned after the head of the chain is abandoned. + gApi.changes().id(masterChangeInfo.id).abandon(); + changesInTopic = + gApi.changes() + .query("topic: " + gApi.changes().id(result.getChangeId()).topic() + " status:open") + .withOptions(ALL_REVISIONS, CURRENT_COMMIT) + .get(); + assertThat(changesInTopic).hasSize(0); + } + + @Test + public void testAbandon() throws Exception { + abandon(ChangeMode.MERGE); + } + + @Test + public void testAbandonCherryPickMode() throws Exception { + abandon(ChangeMode.CHERRY_PICK); + } + + private void multiAmend(ChangeMode changeMode) throws Exception { + Project.NameKey manifestNameKey = defaultSetup(); + // Create initial change + PushOneCommit.Result result = + createChange(testRepo, "master", "subject", "filename", "content", "testtopic"); + // Project name is scoped by test, so we need to get it from our initial change + String projectName = result.getChange().project().get(); + createBranch(BranchNameKey.create(projectName, "ds_one")); + createBranch(BranchNameKey.create(projectName, "ds_two")); + createBranch(BranchNameKey.create(projectName, "ds_three")); + pushFourInChainConfig("automerger.config", manifestNameKey.get(), projectName, changeMode); + // After we upload our config, we upload a new patchset to create the downstreams + amendChange(result.getChangeId()); + result.assertOkStatus(); + // Check that there are the correct number of changes in the topic + List<ChangeInfo> changesInTopic = + gApi.changes() + .query("topic: " + gApi.changes().id(result.getChangeId()).topic()) + .withOptions(ALL_REVISIONS, CURRENT_COMMIT) + .get(); + assertThat(changesInTopic).hasSize(4); + List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic); + ChangeInfo masterChangeInfo = sortedChanges.get(3); + + // Amend the change again. + // Both merge and cherry-pick mode should see 4 open changes. + amendChange(result.getChangeId()); + changesInTopic = + gApi.changes() + .query("topic: " + gApi.changes().id(result.getChangeId()).topic() + " status:open") + .withOptions(ALL_REVISIONS, CURRENT_COMMIT) + .get(); + assertThat(changesInTopic).hasSize(4); + + // Cherry-pick mode should see 3 abandoned changes. + changesInTopic = + gApi.changes() + .query("topic: " + gApi.changes().id(result.getChangeId()).topic() + " status:abandoned") + .withOptions(ALL_REVISIONS, CURRENT_COMMIT) + .get(); + + if(changeMode == ChangeMode.CHERRY_PICK) { + assertThat(changesInTopic).hasSize(3); + } else { + assertThat(changesInTopic).hasSize(0); + } + } + + @Test + public void testMultiAmend() throws Exception { + multiAmend(ChangeMode.MERGE); + } + + @Test + public void testMultiAmendCherryPick() throws Exception { + multiAmend(ChangeMode.CHERRY_PICK); + } + + private void conflictFourInChainAtTail(ChangeMode changeMode) throws Exception { + Project.NameKey manifestNameKey = defaultSetup(); + ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId(); + // Create initial change + PushOneCommit.Result result = + createChange(testRepo, "master", "subject", "filename", "content", "testtopic"); + ObjectId masterChangeId = repo().exactRef("HEAD").getLeaf().getObjectId(); + // Project name is scoped by test, so we need to get it from our initial change + String projectName = result.getChange().project().get(); + createBranch(BranchNameKey.create(projectName, "ds_one")); + createBranch(BranchNameKey.create(projectName, "ds_two")); + createBranch(BranchNameKey.create(projectName, "ds_three")); + + // Reset to create a sibling + testRepo.reset(initial); + // Create an edit in ds_three that will have a conflict + PushOneCommit.Result ds3Result = + createChange( + testRepo, "ds_three", "subject", "filename", "conflicting", "randtopic"); + ds3Result.assertOkStatus(); + merge(ds3Result); + + pushFourInChainConfig("automerger.config", manifestNameKey.get(), projectName, changeMode); + // After we upload our config, we upload a new patchset to create the downstreams and force conflict + testRepo.reset(masterChangeId); + amendChange(result.getChangeId()); + result.assertOkStatus(); + // Check that there are the correct number of changes in the topic. + List<ChangeInfo> changesInTopic = + gApi.changes() + .query("topic: " + gApi.changes().id(result.getChangeId()).topic()) + .withOptions(ALL_REVISIONS, CURRENT_COMMIT) + .get(); + assertThat(changesInTopic).hasSize(3); + List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic); + ChangeInfo dsOneChangeInfo = sortedChanges.get(0); + ChangeInfo dsTwoChangeInfo = sortedChanges.get(1); + ChangeInfo masterChangeInfo = sortedChanges.get(2); + + // All 3 should have -2. assertCodeReview(masterChangeInfo.id, -2, "autogenerated:MergeConflict"); + assertCodeReview(dsOneChangeInfo.id, -2, "autogenerated:Automerger"); + assertCodeReview(dsTwoChangeInfo.id, -2, "autogenerated:Automerger"); + } + + @Test + public void testConflictFourInChainAtTail() throws Exception { + conflictFourInChainAtTail(ChangeMode.MERGE); + } + + @Test + public void testConflictFourInChainAtTailCherryPickMode() throws Exception { + conflictFourInChainAtTail(ChangeMode.CHERRY_PICK); } private Project.NameKey defaultSetup() throws Exception { @@ -976,26 +1415,29 @@ } private void pushSimpleConfig( - String resourceName, String manifestName, String project, String branch1) throws Exception { + String resourceName, String manifestName, String project, String branch1, ChangeMode changeMode) throws Exception { List<ConfigOption> options = new ArrayList<>(); options.add(new ConfigOption("global", null, "manifestProject", manifestName)); + options.add(new ConfigOption("global", null, "cherryPickMode", cherryPickMode(changeMode))); options.add(new ConfigOption("automerger", "master:" + branch1, "setProjects", project)); pushConfig(options, resourceName); } private void pushDefaultConfig( - String resourceName, String manifestName, String project, String branch1, String branch2) + String resourceName, String manifestName, String project, String branch1, String branch2, ChangeMode changeMode) throws Exception { List<ConfigOption> options = new ArrayList<>(); options.add(new ConfigOption("global", null, "manifestProject", manifestName)); + options.add(new ConfigOption("global", null, "cherryPickMode", cherryPickMode(changeMode))); options.add(new ConfigOption("automerger", "master:" + branch1, "setProjects", project)); options.add(new ConfigOption("automerger", "master:" + branch2, "setProjects", project)); pushConfig(options, resourceName); } - private void pushDiamondConfig(String manifestName, String project) throws Exception { + private void pushDiamondConfig(String manifestName, String project, ChangeMode changeMode) throws Exception { List<ConfigOption> options = new ArrayList<>(); options.add(new ConfigOption("global", null, "manifestProject", manifestName)); + options.add(new ConfigOption("global", null, "cherryPickMode", cherryPickMode(changeMode))); options.add(new ConfigOption("automerger", "master:left", "setProjects", project)); options.add(new ConfigOption("automerger", "master:right", "setProjects", project)); options.add(new ConfigOption("automerger", "left:bottom", "setProjects", project)); @@ -1003,16 +1445,29 @@ pushConfig(options, "diamond.config"); } - private void pushContextUserConfig(String manifestName, String project, String contextUserId) + private void pushContextUserConfig(String manifestName, String project, String contextUserId, ChangeMode changeMode) throws Exception { List<ConfigOption> options = new ArrayList<>(); options.add(new ConfigOption("global", null, "manifestProject", manifestName)); options.add(new ConfigOption("global", null, "contextUserId", contextUserId)); + options.add(new ConfigOption("global", null, "cherryPickMode", cherryPickMode(changeMode))); options.add(new ConfigOption("automerger", "master:ds_one", "setProjects", project)); options.add(new ConfigOption("automerger", "ds_one:ds_two", "setProjects", project)); pushConfig(options, "context_user.config"); } + private void pushFourInChainConfig( + String resourceName, String manifestName, String project, ChangeMode changeMode) + throws Exception { + List<ConfigOption> options = new ArrayList<>(); + options.add(new ConfigOption("global", null, "manifestProject", manifestName)); + options.add(new ConfigOption("global", null, "cherryPickMode", cherryPickMode(changeMode))); + options.add(new ConfigOption("automerger", "master:" + "ds_one", "setProjects", project)); + options.add(new ConfigOption("automerger", "ds_one:" + "ds_two", "setProjects", project)); + options.add(new ConfigOption("automerger", "ds_two:" + "ds_three", "setProjects", project)); + pushConfig(options, resourceName); + } + private Optional<ApprovalInfo> getCodeReview(String id) throws RestApiException { List<ApprovalInfo> approvals = gApi.changes().id(id).get(DETAILED_LABELS).labels.get("Code-Review").all; @@ -1055,4 +1510,20 @@ public String getParent(ChangeInfo info, int number) { return info.revisions.get(info.currentRevision).commit.parents.get(number).commit; } + private String getCherryPickFrom(ChangeInfo info){ + try { + return gApi.changes().id(info.cherryPickOfChange).get().currentRevision; + } catch (RestApiException e) { + throw new RuntimeException(e); + } + } + + private String getTag(ChangeMode changeMode){ + return changeMode == ChangeMode.CHERRY_PICK ? "autocherry" : "automerger"; + } + + private String cherryPickMode(ChangeMode changeMode){ + return changeMode == ChangeMode.CHERRY_PICK ? "True" : "False"; + } + }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java index c5b37ff..9519e14 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java
@@ -24,6 +24,7 @@ import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.RefNames; +import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -33,7 +34,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; @@ -43,7 +46,7 @@ name = "automerger", sysModule = "com.googlesource.gerrit.plugins.automerger.AutomergerModule") public class MergeValidatorIT extends LightweightPluginDaemonTest { - private void pushConfig(String resourceName, String project, String branch) throws Exception { + private void pushConfig(String resourceName, String project, String branch, ChangeMode changeMode) throws Exception { TestRepository<InMemoryRepository> allProjectRepo = cloneProject(allProjects, admin); GitUtil.fetch(allProjectRepo, RefNames.REFS_CONFIG + ":config"); allProjectRepo.reset("config"); @@ -51,8 +54,10 @@ String resourceString = CharStreams.toString(new InputStreamReader(in, StandardCharsets.UTF_8)); + String cherryPickMode = (changeMode == ChangeMode.MERGE) ? "False" : "True"; Config cfg = new Config(); cfg.fromText(resourceString); + cfg.setString("global", null, "cherryPickMode", cherryPickMode); // Update manifest project path to the result of createProject(resourceName), since it is // scoped to the test method cfg.setString("automerger", "master:" + branch, "setProjects", project); @@ -63,6 +68,10 @@ } } + private void pushConfig(String resourceName, String project, String branch) throws Exception { + pushConfig(resourceName, project, branch, ChangeMode.MERGE); + } + @Test public void testNoMissingDownstreamMerges() throws Exception { // Create initial change @@ -197,6 +206,25 @@ + ": there is no ds_one"); } + @Test + public void testSkippedCherryPick() throws Exception { + // Create initial change + PushOneCommit.Result result = + createChange(testRepo, "master", "subject", "filename", "content", "testtopic"); + + // Add the skip hashtag. + Set set = new HashSet<>(); + set.add("am_skip_ds_one"); + HashtagsInput input = new HashtagsInput(set); + gApi.changes().id(result.getChangeId()).setHashtags(input); + + pushConfig("automerger.config", result.getChange().project().get(), "ds_one", ChangeMode.CHERRY_PICK); + result.assertOkStatus(); + + // Should be able to merge successfully. + merge(result); + } + private List<ChangeInfo> sortedChanges(List<ChangeInfo> changes) { List<ChangeInfo> listCopy = new ArrayList<>(changes); Collections.sort(
diff --git a/web/automerger.ts b/web/automerger.ts index e43f1f6..91ea27e 100644 --- a/web/automerger.ts +++ b/web/automerger.ts
@@ -71,6 +71,8 @@ private downstreamConfigMap: ConfigMap = {}; + private mergeMode: string = ""; + readonly plugin: PluginApi; constructor(readonly p: PluginApi) { @@ -145,7 +147,7 @@ } }; const button = document.createElement('gr-button'); - button.appendChild(document.createTextNode('Merge')); + button.appendChild(document.createTextNode(this.mergeMode)); button.addEventListener('click', onClick); return button; } @@ -193,9 +195,18 @@ }); } + private getMode() { + const url = `/config/server/automerger~automerge-mode`; + this.plugin.restApi().get<string>(url).then(resp => { + this.mergeMode = resp; + }); + } + onShowChange(change: ChangeInfo) { this.change = change; this.downstreamConfigMap = {}; + this.mergeMode = ""; + this.getMode(); this.getDownstreamConfigMap(); }