IndexDiff/SubmoduleWalk: make the RepositoryBuilder configurable

Some applications using JGit use their own repository caching. In
such applications, it may be needlessly inefficient to create new
submodule repositories from a SubmoduleWalk or in an IndexDiff. It
can be much more efficient to use an already cached repository
instance.

Provide a way to configure a SubmoduleWalk with a factory to create
BaseRepositoryBuilders to use to create repositories, and use it in
IndexDiff. Provide new IndexDiff.diff() operations that take such an
additional factory as parameter.

An application that caches Repository instances (for instance EGit)
can use a factory that provides builders that don't create a new
Repository instance but that return the already cached instance, if
one is available. Note that in such a case, the application may need
to be prepared to deal with IndexDiff.diff() also _closing_ the
obtained repository; if the application expects its cached Repository
instances to remain open while being cached, it'll have to use
Repository.incrementOpen() to prevent that the repository instance
gets closed.

Bug: 550878
Change-Id: Icc1b34dfc4cebd8ed4739dd09d37744d41adf711
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java

@@ -384,7 +384,32 @@ public class IndexDiff {
 	 * @throws java.io.IOException
 	 */
 	public boolean diff() throws IOException {
-		return diff(null, 0, 0, ""); //$NON-NLS-1$
+		return diff(null);
+	}
+
+	/**
+	 * Run the diff operation. Until this is called, all lists will be empty.
+	 * Use
+	 * {@link #diff(ProgressMonitor, int, int, String, RepositoryBuilderFactory)}
+	 * if a progress monitor is required.
+	 * <p>
+	 * The operation may create repositories for submodules using builders
+	 * provided by the given {@code factory}, if any, and will also close these
+	 * submodule repositories again.
+	 * </p>
+	 *
+	 * @param factory
+	 *            the {@link RepositoryBuilderFactory} to use to create builders
+	 *            to create submodule repositories, if needed; if {@code null},
+	 *            submodule repositories will be built using a plain
+	 *            {@link RepositoryBuilder}.
+	 * @return if anything is different between index, tree, and workdir
+	 * @throws java.io.IOException
+	 * @since 5.6
+	 */
+	public boolean diff(RepositoryBuilderFactory factory)
+			throws IOException {
+		return diff(null, 0, 0, "", factory); //$NON-NLS-1$
 	}
 
 	/**
@@ -410,6 +435,45 @@ public class IndexDiff {
 	public boolean diff(final ProgressMonitor monitor, int estWorkTreeSize,
 			int estIndexSize, final String title)
 			throws IOException {
+		return diff(monitor, estWorkTreeSize, estIndexSize, title, null);
+	}
+
+	/**
+	 * Run the diff operation. Until this is called, all lists will be empty.
+	 * <p>
+	 * The operation may be aborted by the progress monitor. In that event it
+	 * will report what was found before the cancel operation was detected.
+	 * Callers should ignore the result if monitor.isCancelled() is true. If a
+	 * progress monitor is not needed, callers should use {@link #diff()}
+	 * instead. Progress reporting is crude and approximate and only intended
+	 * for informing the user.
+	 * </p>
+	 * <p>
+	 * The operation may create repositories for submodules using builders
+	 * provided by the given {@code factory}, if any, and will also close these
+	 * submodule repositories again.
+	 * </p>
+	 *
+	 * @param monitor
+	 *            for reporting progress, may be null
+	 * @param estWorkTreeSize
+	 *            number or estimated files in the working tree
+	 * @param estIndexSize
+	 *            number of estimated entries in the cache
+	 * @param title
+	 *            a {@link java.lang.String} object.
+	 * @param factory
+	 *            the {@link RepositoryBuilderFactory} to use to create builders
+	 *            to create submodule repositories, if needed; if {@code null},
+	 *            submodule repositories will be built using a plain
+	 *            {@link RepositoryBuilder}.
+	 * @return if anything is different between index, tree, and workdir
+	 * @throws java.io.IOException
+	 * @since 5.6
+	 */
+	public boolean diff(ProgressMonitor monitor, int estWorkTreeSize,
+			int estIndexSize, String title, RepositoryBuilderFactory factory)
+			throws IOException {
 		dirCache = repository.readDirCache();
 
 		try (TreeWalk treeWalk = new TreeWalk(repository)) {
@@ -537,6 +601,7 @@ public class IndexDiff {
 		if (ignoreSubmoduleMode != IgnoreSubmoduleMode.ALL) {
 			try (SubmoduleWalk smw = new SubmoduleWalk(repository)) {
 				smw.setTree(new DirCacheIterator(dirCache));
+				smw.setBuilderFactory(factory);
 				while (smw.next()) {
 					IgnoreSubmoduleMode localIgnoreSubmoduleMode = ignoreSubmoduleMode;
 					try {
@@ -568,7 +633,7 @@ public class IndexDiff {
 													subRepo));
 									submoduleIndexDiffs.put(subRepoPath, smid);
 								}
-								if (smid.diff()) {
+								if (smid.diff(factory)) {
 									if (localIgnoreSubmoduleMode == IgnoreSubmoduleMode.UNTRACKED
 											&& smid.getAdded().isEmpty()
 											&& smid.getChanged().isEmpty()

org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryBuilderFactory.java

@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2019, Thomas Wolf <thomas.wolf@paranor.ch> and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+package org.eclipse.jgit.lib;
+
+import java.util.function.Supplier;
+
+/**
+ * A factory for {@link BaseRepositoryBuilder}s.
+ * <p>
+ * Note that a {@link BaseRepositoryBuilder} should be used only once to build a
+ * repository. Otherwise subsequently built repositories may be built using
+ * settings made for earlier built repositories.
+ * </p>
+ *
+ * @since 5.6
+ */
+public interface RepositoryBuilderFactory extends
+		Supplier<BaseRepositoryBuilder<? extends BaseRepositoryBuilder, ? extends Repository>> {
+	// Empty
+}

org.eclipse.jgit/src/org/eclipse/jgit/submodule/SubmoduleWalk.java

@@ -57,6 +57,7 @@ import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.BaseRepositoryBuilder;
 import org.eclipse.jgit.lib.BlobBasedConfig;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ConfigConstants;
@@ -66,6 +67,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.RepositoryBuilder;
+import org.eclipse.jgit.lib.RepositoryBuilderFactory;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.treewalk.AbstractTreeIterator;
@@ -260,15 +262,41 @@ public class SubmoduleWalk implements AutoCloseable {
 	 */
 	public static Repository getSubmoduleRepository(final File parent,
 			final String path, FS fs) throws IOException {
+		return getSubmoduleRepository(parent, path, fs,
+				new RepositoryBuilder());
+	}
+
+	/**
+	 * Get submodule repository at path, using the specified file system
+	 * abstraction and the specified builder
+	 *
+	 * @param parent
+	 *            {@link Repository} that contains the submodule
+	 * @param path
+	 *            of the working tree of the submodule
+	 * @param fs
+	 *            {@link FS} to use
+	 * @param builder
+	 *            {@link BaseRepositoryBuilder} to use to build the submodule
+	 *            repository
+	 * @return the {@link Repository} of the submodule, or {@code null} if it
+	 *         doesn't exist
+	 * @throws IOException
+	 *             on errors
+	 * @since 5.6
+	 */
+	public static Repository getSubmoduleRepository(File parent, String path,
+			FS fs, BaseRepositoryBuilder<?, ? extends Repository> builder)
+			throws IOException {
 		File subWorkTree = new File(parent, path);
-		if (!subWorkTree.isDirectory())
+		if (!subWorkTree.isDirectory()) {
 			return null;
-		File workTree = new File(parent, path);
+		}
 		try {
-			return new RepositoryBuilder() //
+			return builder //
 					.setMustExist(true) //
 					.setFS(fs) //
-					.setWorkTree(workTree) //
+					.setWorkTree(subWorkTree) //
 					.build();
 		} catch (RepositoryNotFoundException e) {
 			return null;
@@ -366,6 +394,8 @@ public class SubmoduleWalk implements AutoCloseable {
 
 	private Map<String, String> pathToName;
 
+	private RepositoryBuilderFactory factory;
+
 	/**
 	 * Create submodule generator
 	 *
@@ -639,7 +669,25 @@ public class SubmoduleWalk implements AutoCloseable {
 	}
 
 	/**
-	 * The module name for the current submodule entry (used for the section name of .git/config)
+	 * Sets the {@link RepositoryBuilderFactory} to use for creating submodule
+	 * repositories. If none is set, a plain {@link RepositoryBuilder} is used.
+	 *
+	 * @param factory
+	 *            to set
+	 * @since 5.6
+	 */
+	public void setBuilderFactory(RepositoryBuilderFactory factory) {
+		this.factory = factory;
+	}
+
+	private BaseRepositoryBuilder<?, ? extends Repository> getBuilder() {
+		return factory != null ? factory.get() : new RepositoryBuilder();
+	}
+
+	/**
+	 * The module name for the current submodule entry (used for the section
+	 * name of .git/config)
+	 *
 	 * @since 4.10
 	 * @return name
 	 */
@@ -755,7 +803,8 @@ public class SubmoduleWalk implements AutoCloseable {
 	 * @throws java.io.IOException
 	 */
 	public Repository getRepository() throws IOException {
-		return getSubmoduleRepository(repository, path);
+		return getSubmoduleRepository(repository.getWorkTree(), path,
+				repository.getFS(), getBuilder());
 	}
 
 	/**