Merge "Fix expanded result rows to not react on click to expand/collapse"
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index d3bea00..cde6864 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -186,6 +186,7 @@
* `git/upload-pack/phase_compressing`: Time spent in the 'Compressing...' phase.
* `git/upload-pack/phase_writing`: Time spent transferring bytes to client.
* `git/upload-pack/pack_bytes`: Distribution of sizes of packs sent to clients.
+* `git/auto-merge/num_operations`: Number of attempted auto merge operations and context.
=== BatchUpdate
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 2b49a06..fb027bd 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -64,6 +64,7 @@
import com.google.gerrit.server.mail.send.CreateChangeSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.patch.AutoMerger;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -114,6 +115,7 @@
private final ReviewerAdder reviewerAdder;
private final MessageIdGenerator messageIdGenerator;
private final DynamicItem<UrlFormatter> urlFormatter;
+ private final AutoMerger autoMerger;
private final Change.Id changeId;
private final PatchSet.Id psId;
@@ -164,6 +166,7 @@
ReviewerAdder reviewerAdder,
MessageIdGenerator messageIdGenerator,
DynamicItem<UrlFormatter> urlFormatter,
+ AutoMerger autoMerger,
@Assisted Change.Id changeId,
@Assisted ObjectId commitId,
@Assisted String refName) {
@@ -181,6 +184,7 @@
this.reviewerAdder = reviewerAdder;
this.messageIdGenerator = messageIdGenerator;
this.urlFormatter = urlFormatter;
+ this.autoMerger = autoMerger;
this.changeId = changeId;
this.psId = PatchSet.id(changeId, INITIAL_PATCH_SET_ID);
@@ -378,6 +382,15 @@
return;
}
ctx.addRefUpdate(cmd);
+ Optional<ReceiveCommand> autoMerge =
+ autoMerger.createAutoMergeCommitIfNecessary(
+ ctx.getRepoView(),
+ ctx.getRevWalk(),
+ ctx.getInserter(),
+ ctx.getRevWalk().parseCommit(commitId));
+ if (autoMerge.isPresent()) {
+ ctx.addRefUpdate(autoMerge.get());
+ }
}
@Override
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index 4c1e9fb..d2bf3fe 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.patch.AutoMerger;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -60,6 +61,7 @@
import java.io.IOException;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.transport.ReceiveCommand;
@@ -82,6 +84,7 @@
private final PatchSetUtil psUtil;
private final WorkInProgressStateChanged wipStateChanged;
private final MessageIdGenerator messageIdGenerator;
+ private final AutoMerger autoMerger;
// Assisted-injected fields.
private final PatchSet.Id psId;
@@ -124,6 +127,7 @@
ProjectCache projectCache,
WorkInProgressStateChanged wipStateChanged,
MessageIdGenerator messageIdGenerator,
+ AutoMerger autoMerger,
@Assisted ChangeNotes notes,
@Assisted PatchSet.Id psId,
@Assisted ObjectId commitId) {
@@ -138,6 +142,7 @@
this.projectCache = projectCache;
this.wipStateChanged = wipStateChanged;
this.messageIdGenerator = messageIdGenerator;
+ this.autoMerger = autoMerger;
this.origNotes = notes;
this.psId = psId;
@@ -214,6 +219,15 @@
throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
validate(ctx);
ctx.addRefUpdate(ObjectId.zeroId(), commitId, getPatchSetId().toRefName());
+ Optional<ReceiveCommand> autoMerge =
+ autoMerger.createAutoMergeCommitIfNecessary(
+ ctx.getRepoView(),
+ ctx.getRevWalk(),
+ ctx.getInserter(),
+ ctx.getRevWalk().parseCommit(commitId));
+ if (autoMerge.isPresent()) {
+ ctx.addRefUpdate(autoMerge.get());
+ }
}
@Override
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 4600f6d..f35b3dd 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -148,6 +148,7 @@
import com.google.gerrit.server.mail.MailUtil.MailRecipients;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.server.patch.AutoMerger;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.GlobalPermission;
@@ -356,6 +357,7 @@
private final SetPrivateOp.Factory setPrivateOpFactory;
private final ReplyAttentionSetUpdates replyAttentionSetUpdates;
private final DynamicItem<UrlFormatter> urlFormatter;
+ private final AutoMerger autoMerger;
// Assisted injected fields.
private final ProjectState projectState;
@@ -440,6 +442,7 @@
SetPrivateOp.Factory setPrivateOpFactory,
ReplyAttentionSetUpdates replyAttentionSetUpdates,
DynamicItem<UrlFormatter> urlFormatter,
+ AutoMerger autoMerger,
@Assisted ProjectState projectState,
@Assisted IdentifiedUser user,
@Assisted ReceivePack rp,
@@ -491,6 +494,7 @@
this.setPrivateOpFactory = setPrivateOpFactory;
this.replyAttentionSetUpdates = replyAttentionSetUpdates;
this.urlFormatter = urlFormatter;
+ this.autoMerger = autoMerger;
// Assisted injected fields.
this.projectState = projectState;
@@ -3063,6 +3067,25 @@
if (progress != null) {
bu.addOp(notes.getChangeId(), new ChangeProgressOp(progress));
}
+ bu.addRepoOnlyOp(
+ new RepoOnlyOp() {
+ @Override
+ public void updateRepo(RepoContext ctx) throws Exception {
+ // Create auto merge ref if the new patch set is a merge commit. This is only
+ // required for new patch sets on existing changes as these do not go through
+ // PatchSetInserter. New changes pushed via git go through ChangeInserter and have
+ // their auto merge commits created there.
+ Optional<ReceiveCommand> autoMerge =
+ autoMerger.createAutoMergeCommitIfNecessary(
+ ctx.getRepoView(),
+ ctx.getRevWalk(),
+ ctx.getInserter(),
+ ctx.getRevWalk().parseCommit(newCommitId));
+ if (autoMerge.isPresent()) {
+ ctx.addRefUpdate(autoMerge.get());
+ }
+ }
+ });
}
}
diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index ac37411..de5b1da 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -16,19 +16,22 @@
import static com.google.common.base.Preconditions.checkArgument;
-import com.google.common.base.Throwables;
-import com.google.gerrit.common.Nullable;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.git.LockFailureException;
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.MergeUtil;
-import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryableAction.ActionType;
+import com.google.gerrit.server.logging.Metadata;
+import com.google.gerrit.server.update.RepoView;
import com.google.inject.Inject;
import java.io.IOException;
+import java.util.Optional;
import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
@@ -37,13 +40,13 @@
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.merge.ResolveMerger;
import org.eclipse.jgit.merge.ThreeWayMergeStrategy;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
/**
* Utility class for creating an auto-merge commit of a merge commit.
@@ -67,6 +70,8 @@
* is that these refs should never be deleted.
*/
public class AutoMerger {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
public static final String AUTO_MERGE_MSG_PREFIX = "Auto-merge of ";
@UsedAt(UsedAt.Project.GOOGLE)
@@ -74,92 +79,108 @@
return cfg.getBoolean("change", null, "cacheAutomerge", true);
}
- private final RetryHelper retryHelper;
+ private enum OperationType {
+ CACHE_LOAD,
+ IN_MEMORY_WRITE,
+ ON_DISK_WRITE
+ }
+
+ private final Counter1<OperationType> counter;
private final PersonIdent gerritIdent;
private final boolean save;
+ private final ThreeWayMergeStrategy configuredMergeStrategy;
@Inject
AutoMerger(
- RetryHelper retryHelper,
+ MetricMaker metricMaker,
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent gerritIdent) {
- this.retryHelper = retryHelper;
- save = cacheAutomerge(cfg);
+ this.counter =
+ metricMaker.newCounter(
+ "git/auto-merge/num_operations",
+ new Description("AutoMerge computations").setRate().setUnit("auto merge computations"),
+ Field.ofEnum(OperationType.class, "type", Metadata.Builder::operationName).build());
+ this.save = cacheAutomerge(cfg);
this.gerritIdent = gerritIdent;
+ this.configuredMergeStrategy = MergeUtil.getMergeStrategy(cfg);
}
/**
- * Creates an auto-merge commit of the parents of the given merge commit.
+ * Reads or creates an auto-merge commit of the parents of the given merge commit.
*
- * <p>In case of an exception the creation of the auto-merge commit is retried a few times. E.g.
- * this allows the operation to succeed if a Git update fails due to a temporary issue.
+ * <p>The result is read from Git or computed in-memory and not written back to Git. This method
+ * exists for backwards compatibility only. All new changes have their auto-merge commits written
+ * transactionally when the change or patch set is created.
*
* @return auto-merge commit. Headers of the returned RevCommit are parsed.
*/
- public RevCommit merge(
+ public RevCommit lookupFromGitOrMergeInMemory(
Repository repo,
RevWalk rw,
- ObjectInserter ins,
- RevCommit merge,
- ThreeWayMergeStrategy mergeStrategy)
- throws IOException {
- try {
- return retryHelper
- .action(
- ActionType.GIT_UPDATE,
- "createAutoMerge",
- () -> createAutoMergeCommit(repo, rw, ins, merge, mergeStrategy))
- .defaultTimeoutMultiplier(2)
- .call();
- } catch (Exception e) {
- Throwables.throwIfUnchecked(e);
- Throwables.throwIfInstanceOf(e, IOException.class);
- throw new IllegalStateException(e);
- }
- }
-
- /**
- * Creates an auto-merge commit of the parents of the given merge commit.
- *
- * @return auto-merge commit. Headers of the returned RevCommit are parsed.
- */
- private RevCommit createAutoMergeCommit(
- Repository repo,
- RevWalk rw,
- ObjectInserter ins,
+ InMemoryInserter ins,
RevCommit merge,
ThreeWayMergeStrategy mergeStrategy)
throws IOException {
checkArgument(rw.getObjectReader().getCreatedFromInserter() == ins);
+ Optional<RevCommit> existingCommit =
+ lookupCommit(repo, rw, RefNames.refsCacheAutomerge(merge.name()));
+ if (existingCommit.isPresent()) {
+ counter.increment(OperationType.CACHE_LOAD);
+ return existingCommit.get();
+ }
+ counter.increment(OperationType.IN_MEMORY_WRITE);
+ logger.atWarning().log("Computing in-memory AutoMerge for " + merge.name());
+ return rw.parseCommit(createAutoMergeCommit(repo.getConfig(), rw, ins, merge, mergeStrategy));
+ }
- InMemoryInserter tmpIns = null;
- if (ins instanceof InMemoryInserter) {
- // Caller gave us an in-memory inserter, so ensure anything we write from
- // this method is visible to them.
- tmpIns = (InMemoryInserter) ins;
- } else if (!save) {
- // If we don't plan on saving results, use a fully in-memory inserter.
- // Using just a non-flushing wrapper is not sufficient, since in
- // particular DfsInserter might try to write to storage after exceeding an
- // internal buffer size.
- tmpIns = new InMemoryInserter(rw.getObjectReader());
+ /**
+ * Creates an auto merge commit for the provided commit in case it is a merge commit. To be used
+ * whenever Gerrit creates new patch sets.
+ *
+ * <p>Callers need to include the returned {@link ReceiveCommand} in their ref transaction.
+ *
+ * @return A {@link ReceiveCommand} wrapped in an {@link Optional} to be used in a {@link
+ * org.eclipse.jgit.lib.BatchRefUpdate}. {@link Optional#empty()} in case we don't need an
+ * auto merge commit.
+ */
+ public Optional<ReceiveCommand> createAutoMergeCommitIfNecessary(
+ RepoView repoView, RevWalk rw, ObjectInserter ins, RevCommit maybeMergeCommit)
+ throws IOException {
+ if (maybeMergeCommit.getParentCount() != 2 || !save) {
+ logger.atFine().log("AutoMerge not required");
+ return Optional.empty();
}
+ ObjectId autoMerge =
+ createAutoMergeCommit(
+ repoView.getConfig(), rw, ins, maybeMergeCommit, configuredMergeStrategy);
+ counter.increment(OperationType.ON_DISK_WRITE);
+ logger.atFine().log("Added %s AutoMerge ref update for commit", autoMerge.name());
+ return Optional.of(
+ new ReceiveCommand(
+ ObjectId.zeroId(), autoMerge, RefNames.refsCacheAutomerge(maybeMergeCommit.name())));
+ }
+
+ /**
+ * Creates an auto-merge commit of the parents of the given merge commit.
+ *
+ * @return auto-merge commit. Headers of the returned RevCommit are parsed.
+ */
+ private ObjectId createAutoMergeCommit(
+ Config repoConfig,
+ RevWalk rw,
+ ObjectInserter ins,
+ RevCommit merge,
+ ThreeWayMergeStrategy mergeStrategy)
+ throws IOException {
rw.parseHeaders(merge);
- String refName = RefNames.refsCacheAutomerge(merge.name());
- Ref ref = repo.getRefDatabase().exactRef(refName);
- if (ref != null && ref.getObjectId() != null) {
- RevObject obj = rw.parseAny(ref.getObjectId());
- if (obj instanceof RevCommit) {
- return (RevCommit) obj;
- }
- return commit(repo, rw, tmpIns, ins, refName, obj, merge);
- }
-
- ResolveMerger m = (ResolveMerger) mergeStrategy.newMerger(repo, true);
+ ResolveMerger m = (ResolveMerger) mergeStrategy.newMerger(ins, repoConfig);
DirCache dc = DirCache.newInCore();
m.setDirCache(dc);
- m.setObjectInserter(tmpIns == null ? new NonFlushingWrapper(ins) : tmpIns);
+ // If we don't plan on saving results, use a fully in-memory inserter.
+ // Using just a non-flushing wrapper is not sufficient, since in particular DfsInserter might
+ // try to write to storage after exceeding an internal buffer size.
+ m.setObjectInserter(ins instanceof InMemoryInserter ? new NonFlushingWrapper(ins) : ins);
boolean couldMerge = m.merge(merge.getParents());
@@ -179,18 +200,6 @@
m.getMergeResults());
}
- return commit(repo, rw, tmpIns, ins, refName, treeId, merge);
- }
-
- private RevCommit commit(
- Repository repo,
- RevWalk rw,
- @Nullable InMemoryInserter tmpIns,
- ObjectInserter ins,
- String refName,
- ObjectId tree,
- RevCommit merge)
- throws IOException {
rw.parseHeaders(merge);
// For maximum stability, choose a single ident using the committer time of
// the input commit, using the server name and timezone.
@@ -200,50 +209,34 @@
CommitBuilder cb = new CommitBuilder();
cb.setAuthor(ident);
cb.setCommitter(ident);
- cb.setTreeId(tree);
+ cb.setTreeId(treeId);
cb.setMessage(AUTO_MERGE_MSG_PREFIX + merge.name() + '\n');
for (RevCommit p : merge.getParents()) {
cb.addParentId(p);
}
- if (!save) {
- checkArgument(tmpIns != null);
- try (ObjectReader tmpReader = tmpIns.newReader();
+ if (ins instanceof InMemoryInserter) {
+ // When using an InMemoryInserter we need to read back the values from that inserter because
+ // they are not available.
+ try (ObjectReader tmpReader = ins.newReader();
RevWalk tmpRw = new RevWalk(tmpReader)) {
- return tmpRw.parseCommit(tmpIns.insert(cb));
+ return tmpRw.parseCommit(ins.insert(cb));
}
}
- checkArgument(tmpIns == null);
- checkArgument(!(ins instanceof InMemoryInserter));
- ObjectId commitId = ins.insert(cb);
- ins.flush();
+ return rw.parseCommit(ins.insert(cb));
+ }
- RefUpdate ru = repo.updateRef(refName);
- ru.setNewObjectId(commitId);
- ru.disableRefLog();
- switch (ru.forceUpdate()) {
- case FAST_FORWARD:
- case FORCED:
- case NEW:
- case NO_CHANGE:
- return rw.parseCommit(commitId);
- case LOCK_FAILURE:
- throw new LockFailureException(
- String.format("Failed to create auto-merge of %s", merge.name()), ru);
- case IO_FAILURE:
- case NOT_ATTEMPTED:
- case REJECTED:
- case REJECTED_CURRENT_BRANCH:
- case REJECTED_MISSING_OBJECT:
- case REJECTED_OTHER_REASON:
- case RENAMED:
- default:
- throw new IOException(
- String.format(
- "Failed to create auto-merge of %s: Cannot write %s (%s)",
- merge.name(), refName, ru.getResult()));
+ private Optional<RevCommit> lookupCommit(Repository repo, RevWalk rw, String refName)
+ throws IOException {
+ Ref ref = repo.getRefDatabase().exactRef(refName);
+ if (ref != null && ref.getObjectId() != null) {
+ RevObject obj = rw.parseAny(ref.getObjectId());
+ if (obj instanceof RevCommit) {
+ return Optional.of((RevCommit) obj);
+ }
}
+ return Optional.empty();
}
private static class NonFlushingWrapper extends ObjectInserter.Filter {
diff --git a/java/com/google/gerrit/server/patch/BaseCommitUtil.java b/java/com/google/gerrit/server/patch/BaseCommitUtil.java
index de4a10e..7c06a62 100644
--- a/java/com/google/gerrit/server/patch/BaseCommitUtil.java
+++ b/java/com/google/gerrit/server/patch/BaseCommitUtil.java
@@ -37,16 +37,11 @@
class BaseCommitUtil {
private final AutoMerger autoMerger;
private final ThreeWayMergeStrategy mergeStrategy;
-
- /** If true, auto-merge results are stored in the repository. */
- private final boolean saveAutomerge;
-
private final GitRepositoryManager repoManager;
@Inject
BaseCommitUtil(AutoMerger am, @GerritServerConfig Config cfg, GitRepositoryManager repoManager) {
this.autoMerger = am;
- this.saveAutomerge = AutoMerger.cacheAutomerge(cfg);
this.mergeStrategy = MergeUtil.getMergeStrategy(cfg);
this.repoManager = repoManager;
}
@@ -54,7 +49,7 @@
RevObject getBaseCommit(Project.NameKey project, ObjectId newCommit, @Nullable Integer parentNum)
throws IOException {
try (Repository repo = repoManager.openRepository(project);
- ObjectInserter ins = newInserter(repo);
+ InMemoryInserter ins = new InMemoryInserter(repo);
ObjectReader reader = ins.newReader();
RevWalk rw = new RevWalk(reader)) {
return getParentCommit(repo, ins, rw, parentNum, newCommit);
@@ -93,7 +88,7 @@
*/
RevObject getParentCommit(
Repository repo,
- ObjectInserter ins,
+ InMemoryInserter ins,
RevWalk rw,
@Nullable Integer parentNum,
ObjectId commitId)
@@ -112,16 +107,12 @@
}
// Only support auto-merge for 2 parents, not octopus merges
if (current.getParentCount() == 2) {
- return autoMerger.merge(repo, rw, ins, current, mergeStrategy);
+ return autoMerger.lookupFromGitOrMergeInMemory(repo, rw, ins, current, mergeStrategy);
}
return null;
}
}
- private ObjectInserter newInserter(Repository repo) {
- return saveAutomerge ? repo.newObjectInserter() : new InMemoryInserter(repo);
- }
-
private static ObjectId emptyTree(ObjectInserter ins) throws IOException {
ObjectId id = ins.insert(Constants.OBJ_TREE, new byte[] {});
ins.flush();
diff --git a/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java b/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
index d1c0b45..017e276 100644
--- a/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
+++ b/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java
@@ -107,7 +107,6 @@
private final PatchListKey key;
private final Project.NameKey project;
private final long timeoutMillis;
- private final boolean save;
@Inject
PatchListLoader(
@@ -133,13 +132,12 @@
"timeout",
TimeUnit.MILLISECONDS.convert(5, TimeUnit.SECONDS),
TimeUnit.MILLISECONDS);
- save = AutoMerger.cacheAutomerge(cfg);
}
@Override
public PatchList call() throws IOException, PatchListNotAvailableException {
try (Repository repo = repoManager.openRepository(project);
- ObjectInserter ins = newInserter(repo);
+ InMemoryInserter ins = new InMemoryInserter(repo);
ObjectReader reader = ins.newReader();
RevWalk rw = new RevWalk(reader)) {
return readPatchList(repo, rw, ins);
@@ -163,11 +161,7 @@
}
}
- private ObjectInserter newInserter(Repository repo) {
- return save ? repo.newObjectInserter() : new InMemoryInserter(repo);
- }
-
- private PatchList readPatchList(Repository repo, RevWalk rw, ObjectInserter ins)
+ private PatchList readPatchList(Repository repo, RevWalk rw, InMemoryInserter ins)
throws IOException, PatchListNotAvailableException {
ObjectReader reader = rw.getObjectReader();
checkArgument(reader.getCreatedFromInserter() == ins);
@@ -634,7 +628,7 @@
}
private RevObject aFor(
- PatchListKey key, Repository repo, RevWalk rw, ObjectInserter ins, RevCommit b)
+ PatchListKey key, Repository repo, RevWalk rw, InMemoryInserter ins, RevCommit b)
throws IOException {
if (key.getOldId() != null) {
return rw.parseAny(key.getOldId());
@@ -657,7 +651,7 @@
}
// Only support auto-merge for 2 parents, not octopus merges
if (b.getParentCount() == 2) {
- return autoMerger.merge(repo, rw, ins, b, mergeStrategy);
+ return autoMerger.lookupFromGitOrMergeInMemory(repo, rw, ins, b, mergeStrategy);
}
return null;
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetBlame.java b/java/com/google/gerrit/server/restapi/change/GetBlame.java
index 12b4d44..04828f2 100644
--- a/java/com/google/gerrit/server/restapi/change/GetBlame.java
+++ b/java/com/google/gerrit/server/restapi/change/GetBlame.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.change.FileResource;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.patch.AutoMerger;
import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -39,7 +40,6 @@
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
@@ -86,7 +86,7 @@
throws RestApiException, IOException, InvalidChangeOperationException {
Project.NameKey project = resource.getRevision().getChange().getProject();
try (Repository repository = repoManager.openRepository(project);
- ObjectInserter ins = repository.newObjectInserter();
+ InMemoryInserter ins = new InMemoryInserter(repository);
ObjectReader reader = ins.newReader();
RevWalk revWalk = new RevWalk(reader)) {
String refName =
@@ -115,7 +115,9 @@
result = blame(parents[0], path, repository, revWalk);
} else if (parents.length == 2) {
- ObjectId automerge = autoMerger.merge(repository, revWalk, ins, revCommit, mergeStrategy);
+ ObjectId automerge =
+ autoMerger.lookupFromGitOrMergeInMemory(
+ repository, revWalk, ins, revCommit, mergeStrategy);
result = blame(automerge, path, repository, revWalk);
} else {
diff --git a/javatests/com/google/gerrit/acceptance/git/AutoMergeIT.java b/javatests/com/google/gerrit/acceptance/git/AutoMergeIT.java
new file mode 100644
index 0000000..46688dd
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/git/AutoMergeIT.java
@@ -0,0 +1,189 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.git;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.eclipse.jgit.lib.Constants.HEAD;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.common.RawInputUtil;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.extensions.common.MergeInput;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Ensures that auto merge commits are created when a new patch set or change is uploaded. */
+public class AutoMergeIT extends AbstractDaemonTest {
+ private RevCommit parent1;
+ private RevCommit parent2;
+
+ @Before
+ public void setup() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+
+ PushOneCommit.Result p1 =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ testRepo,
+ "parent 1",
+ ImmutableMap.of("foo", "foo-1.2", "bar", "bar-1.2"))
+ .to("refs/for/master");
+ parent1 = p1.getCommit();
+
+ // reset HEAD in order to create a sibling of the first change
+ testRepo.reset(initial);
+
+ PushOneCommit.Result p2 =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ testRepo,
+ "parent 2",
+ ImmutableMap.of("foo", "foo-2.2", "bar", "bar-2.2"))
+ .to("refs/for/master");
+ parent2 = p2.getCommit();
+ }
+
+ @Test
+ public void autoMergeCreatedWhenPushingNewChange() throws Exception {
+ PushOneCommit m =
+ pushFactory.create(
+ admin.newIdent(), testRepo, "merge", ImmutableMap.of("foo", "foo-1", "bar", "bar-2"));
+ m.setParents(ImmutableList.of(parent1, parent2));
+ PushOneCommit.Result result = m.to("refs/for/master");
+ result.assertOkStatus();
+ assertAutoMergeCreated(result.getCommit());
+ }
+
+ @Test
+ public void autoMergeCreatedWhenPushingNewPatchSet() throws Exception {
+ PushOneCommit m =
+ pushFactory.create(
+ admin.newIdent(), testRepo, "merge", ImmutableMap.of("foo", "foo-1", "bar", "bar-2"));
+ m.setParents(ImmutableList.of(parent1, parent2));
+ PushOneCommit.Result ps1 = m.to("refs/for/master");
+ RevCommit ps2 =
+ testRepo
+ .amend(ps1.getCommit())
+ .message("PS2")
+ .insertChangeId(ps1.getChangeId().substring(1))
+ .create();
+ testRepo.reset(ps2);
+ GitUtil.pushHead(testRepo, "refs/for/master");
+ // Make sure we have two patch sets
+ assertThat(ps2.getParents().length).isEqualTo(2);
+ assertThat(gApi.changes().id(ps1.getChangeId()).get().revisions.size()).isEqualTo(2);
+ assertAutoMergeCreated(ps2);
+ }
+
+ @Test
+ public void autoMergeCreatedWhenChangeCreatedOnApi() throws Exception {
+ ChangeInput ci = new ChangeInput(project.get(), "master", "Merge commit");
+ ci.merge = new MergeInput();
+ ci.merge.source = parent1.name();
+
+ String newChangePatchSetSha1 = gApi.changes().create(ci).get().currentRevision;
+ assertAutoMergeCreated(ObjectId.fromString(newChangePatchSetSha1));
+ }
+
+ @Test
+ public void autoMergeCreatedWhenNewPatchSetCreatedOnApi() throws Exception {
+ ChangeInput ci = new ChangeInput(project.get(), "master", "Merge commit");
+ ci.merge = new MergeInput();
+ ci.merge.source = parent1.name();
+
+ String changeId = gApi.changes().create(ci).get().changeId;
+ gApi.changes().id(changeId).setMessage("New Commit Message\n\nChange-Id: " + changeId);
+ assertThat(gApi.changes().id(changeId).get().revisions.size()).isEqualTo(2);
+ String newChangePatchSetSha1 = gApi.changes().id(changeId).get().currentRevision;
+ assertAutoMergeCreated(ObjectId.fromString(newChangePatchSetSha1));
+ }
+
+ @Test
+ public void autoMergeCreatedWhenChangeEditIsPublished() throws Exception {
+ PushOneCommit m =
+ pushFactory.create(
+ admin.newIdent(), testRepo, "merge", ImmutableMap.of("foo", "foo-1", "bar", "bar-2"));
+ m.setParents(ImmutableList.of(parent1, parent2));
+ PushOneCommit.Result result = m.to("refs/for/master");
+ result.assertOkStatus();
+ assertAutoMergeCreated(result.getCommit());
+
+ gApi.changes()
+ .id(result.getChangeId())
+ .edit()
+ .modifyFile("new-file", RawInputUtil.create("content"));
+ gApi.changes().id(result.getChangeId()).edit().publish();
+ assertThat(gApi.changes().id(result.getChangeId()).get().revisions.size()).isEqualTo(2);
+ String newChangePatchSetSha1 = gApi.changes().id(result.getChangeId()).get().currentRevision;
+ assertAutoMergeCreated(ObjectId.fromString(newChangePatchSetSha1));
+ }
+
+ @Test
+ public void noAutoMergeCreatedWhenPushingNonMergeCommit() throws Exception {
+ PushOneCommit.Result change = createChange();
+ change.assertOkStatus();
+ assertNoAutoMergeCreated(change.getCommit());
+ }
+
+ @Test
+ public void autoMergeComputedInMemoryWhenMissing() throws Exception {
+ PushOneCommit m =
+ pushFactory.create(
+ admin.newIdent(), testRepo, "merge", ImmutableMap.of("foo", "foo-1", "bar", "bar-2"));
+ m.setParents(ImmutableList.of(parent1, parent2));
+ PushOneCommit.Result result = m.to("refs/for/master");
+ result.assertOkStatus();
+ assertAutoMergeCreated(result.getCommit());
+
+ // Delete auto merge branch
+ deleteAutoMergeBranch(result.getCommit());
+ // Trigger AutoMerge computation
+ assertThat(gApi.changes().id(result.getChangeId()).revision(1).file("foo").blameRequest().get())
+ .isNotEmpty();
+ assertNoAutoMergeCreated(result.getCommit());
+ }
+
+ private void assertAutoMergeCreated(ObjectId mergeCommit) throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ assertThat(repo.exactRef(RefNames.refsCacheAutomerge(mergeCommit.name()))).isNotNull();
+ }
+ }
+
+ private void assertNoAutoMergeCreated(ObjectId mergeCommit) throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ assertThat(repo.exactRef(RefNames.refsCacheAutomerge(mergeCommit.name()))).isNull();
+ }
+ }
+
+ private void deleteAutoMergeBranch(ObjectId mergeCommit) throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ RefUpdate ru = repo.updateRef(RefNames.refsCacheAutomerge(mergeCommit.name()));
+ ru.setForceUpdate(true);
+ assertThat(ru.delete()).isEqualTo(RefUpdate.Result.FORCED);
+ }
+ assertNoAutoMergeCreated(mergeCommit);
+ }
+}
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 12afddf..8fe3b24 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -377,6 +377,8 @@
--iron-overlay-backdrop: {
transition: none;
};
+ --paper-tooltip-delay-in: 200ms;
+ --paper-tooltip-delay-out: 0;
--paper-tooltip-duration-in: 0;
--paper-tooltip-duration-out: 0;
--paper-tooltip-background: var(--tooltip-background-color);
