diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java index 1b551f2b18..a70c2355c1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java @@ -31,6 +31,7 @@ import org.eclipse.jgit.transport.PackParser; import java.io.IOException; import java.io.InputStream; import java.util.Collection; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -108,9 +109,16 @@ public class InMemoryInserter extends ObjectInserter { } @Override - public Collection resolve(AbbreviatedObjectId id) { - // This method should be unused by ChangeRebuilder. - throw new UnsupportedOperationException(); + public Collection resolve(AbbreviatedObjectId id) + throws IOException { + Set result = new HashSet<>(); + for (ObjectId insId : inserted.keySet()) { + if (id.prefixCompare(insId) == 0) { + result.add(insId); + } + } + result.addAll(reader.resolve(id)); + return result; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/AutoMerger.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/AutoMerger.java index 7cbf741209..c4af9fdf51 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/AutoMerger.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/AutoMerger.java @@ -57,6 +57,10 @@ import java.util.Map; public class AutoMerger { private static final Logger log = LoggerFactory.getLogger(AutoMerger.class); + static boolean cacheAutomerge(Config cfg) { + return cfg.getBoolean("change", null, "cacheAutomerge", true); + } + private final PersonIdent gerritIdent; private final boolean save; @@ -64,7 +68,7 @@ public class AutoMerger { AutoMerger( @GerritServerConfig Config cfg, @GerritPersonIdent PersonIdent gerritIdent) { - save = cfg.getBoolean("change", null, "cacheAutomerge", true); + save = cacheAutomerge(cfg); this.gerritIdent = gerritIdent; } @@ -79,7 +83,11 @@ public class AutoMerger { throws IOException { checkArgument(rw.getObjectReader().getCreatedFromInserter() == ins); InMemoryInserter tmpIns = null; - if (!save) { + 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 @@ -105,7 +113,7 @@ public class AutoMerger { ResolveMerger m = (ResolveMerger) mergeStrategy.newMerger(repo, true); DirCache dc = DirCache.newInCore(); m.setDirCache(dc); - m.setObjectInserter(save ? new NonFlushingWrapper(ins) : tmpIns); + m.setObjectInserter(tmpIns == null ? new NonFlushingWrapper(ins) : tmpIns); boolean couldMerge; try { @@ -236,6 +244,7 @@ public class AutoMerger { } checkArgument(tmpIns == null); + checkArgument(!(ins instanceof InMemoryInserter)); ObjectId commitId = ins.insert(cb); ins.flush(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java index 0b2a7ed44c..61caf20efb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.config.ConfigUtil; 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.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; @@ -84,6 +85,7 @@ public class PatchListLoader implements Callable { private final PatchListKey key; private final Project.NameKey project; private final long timeoutMillis; + private final boolean save; @AssistedInject PatchListLoader(GitRepositoryManager mgr, @@ -104,6 +106,7 @@ public class PatchListLoader implements Callable { ConfigUtil.getTimeUnit(cfg, "cache", PatchListCacheImpl.FILE_NAME, "timeout", TimeUnit.MILLISECONDS.convert(5, TimeUnit.SECONDS), TimeUnit.MILLISECONDS); + save = AutoMerger.cacheAutomerge(cfg); } @Override @@ -131,33 +134,39 @@ public class PatchListLoader implements Callable { } } - private PatchList readPatchList(final PatchListKey key, final Repository repo) + private ObjectInserter newInserter(Repository repo) { + return save + ? repo.newObjectInserter() + : new InMemoryInserter(repo); + } + + private PatchList readPatchList(PatchListKey key, Repository repo) throws IOException, PatchListNotAvailableException { - final RawTextComparator cmp = comparatorFor(key.getWhitespace()); - try (ObjectInserter ins = repo.newObjectInserter(); + RawTextComparator cmp = comparatorFor(key.getWhitespace()); + try (ObjectInserter ins = newInserter(repo); ObjectReader reader = ins.newReader(); RevWalk rw = new RevWalk(reader); DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE)) { - final RevCommit b = rw.parseCommit(key.getNewId()); - final RevObject a = aFor(key, repo, rw, ins, b); + RevCommit b = rw.parseCommit(key.getNewId()); + RevObject a = aFor(key, repo, rw, ins, b); if (a == null) { // TODO(sop) Remove this case. // This is a merge commit, compared to its ancestor. // - final PatchListEntry[] entries = new PatchListEntry[1]; + PatchListEntry[] entries = new PatchListEntry[1]; entries[0] = newCommitMessage(cmp, reader, null, b); return new PatchList(a, b, true, entries); } - final boolean againstParent = + boolean againstParent = b.getParentCount() > 0 && b.getParent(0).equals(a); RevCommit aCommit = a instanceof RevCommit ? (RevCommit) a : null; RevTree aTree = rw.parseTree(a); RevTree bTree = b.getTree(); - df.setRepository(repo); + df.setReader(reader, repo.getConfig()); df.setDiffComparator(cmp); df.setDetectRenames(true); List diffEntries = df.scan(aTree, bTree); @@ -191,9 +200,9 @@ public class PatchListLoader implements Callable { FileHeader fh = toFileHeader(key, df, e); long oldSize = - getFileSize(repo, reader, e.getOldMode(), e.getOldPath(), aTree); + getFileSize(reader, e.getOldMode(), e.getOldPath(), aTree); long newSize = - getFileSize(repo, reader, e.getNewMode(), e.getNewPath(), bTree); + getFileSize(reader, e.getNewMode(), e.getNewPath(), bTree); entries.add(newEntry(aTree, fh, newSize, newSize - oldSize)); } } @@ -202,14 +211,14 @@ public class PatchListLoader implements Callable { } } - private static long getFileSize(Repository repo, ObjectReader reader, + private static long getFileSize(ObjectReader reader, FileMode mode, String path, RevTree t) throws IOException { if (!isBlob(mode)) { return 0; } try (TreeWalk tw = TreeWalk.forPath(reader, path, t)) { return tw != null - ? repo.open(tw.getObjectId(0), OBJ_BLOB).getSize() + ? reader.open(tw.getObjectId(0), OBJ_BLOB).getSize() : 0; } } @@ -324,7 +333,7 @@ public class PatchListLoader implements Callable { switch (b.getParentCount()) { case 0: - return rw.parseAny(emptyTree(repo)); + return rw.parseAny(emptyTree(ins)); case 1: { RevCommit r = b.getParent(0); rw.parseBody(r); @@ -343,11 +352,9 @@ public class PatchListLoader implements Callable { } } - private static ObjectId emptyTree(final Repository repo) throws IOException { - try (ObjectInserter oi = repo.newObjectInserter()) { - ObjectId id = oi.insert(Constants.OBJ_TREE, new byte[] {}); - oi.flush(); - return id; - } + private static ObjectId emptyTree(ObjectInserter ins) throws IOException { + ObjectId id = ins.insert(Constants.OBJ_TREE, new byte[] {}); + ins.flush(); + return id; } }