RepoCommand: Don't leave Git open

When the command is run on a non-bare repository, an instance of
Git is created to execute the commit, and is left open when the
command has finished.

Refactor to not use a class scope Git instance, and make sure it
gets closed before returning.

Change-Id: Ic623ae0fd8b9e264b5dfd434da0de6bb4f910984
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java

@@ -124,7 +124,6 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	private boolean ignoreRemoteFailures = false;
 
 	private List<RepoProject> bareProjects;
-	private Git git;
 	private ProgressMonitor monitor;
 
 	/**
@@ -486,58 +485,50 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	/** {@inheritDoc} */
 	@Override
 	public RevCommit call() throws GitAPIException {
-		try {
+		checkCallable();
-			checkCallable();
+		if (baseUri == null) {
-			if (baseUri == null) {
+			baseUri = ""; //$NON-NLS-1$
-				baseUri = ""; //$NON-NLS-1$
+		}
-			}
+		if (inputStream == null) {
-			if (inputStream == null) {
+			if (manifestPath == null || manifestPath.length() == 0)
-				if (manifestPath == null || manifestPath.length() == 0)
+				throw new IllegalArgumentException(
-					throw new IllegalArgumentException(
+						JGitText.get().pathNotConfigured);
-							JGitText.get().pathNotConfigured);
-				try {
-					inputStream = new FileInputStream(manifestPath);
-				} catch (IOException e) {
-					throw new IllegalArgumentException(
-							JGitText.get().pathNotConfigured);
-				}
-			}
-
-			if (repo.isBare()) {
-				bareProjects = new ArrayList<>();
-				if (author == null)
-					author = new PersonIdent(repo);
-				if (callback == null)
-					callback = new DefaultRemoteReader();
-			} else
-				git = new Git(repo);
-
-			ManifestParser parser = new ManifestParser(
-					includedReader, manifestPath, branch, baseUri, groupsParam, repo);
 			try {
-				parser.read(inputStream);
+				inputStream = new FileInputStream(manifestPath);
-				for (RepoProject proj : parser.getFilteredProjects()) {
+			} catch (IOException e) {
-					addSubmodule(proj.getUrl(),
+				throw new IllegalArgumentException(
-							proj.getPath(),
+						JGitText.get().pathNotConfigured);
-							proj.getRevision(),
-							proj.getCopyFiles(),
-							proj.getLinkFiles(),
-							proj.getGroups(),
-							proj.getRecommendShallow());
-				}
-			} catch (GitAPIException | IOException e) {
-				throw new ManifestErrorException(e);
 			}
+		}
+
+		List<RepoProject> filteredProjects;
+		try {
+			ManifestParser parser = new ManifestParser(includedReader,
+					manifestPath, branch, baseUri, groupsParam, repo);
+			parser.read(inputStream);
+			filteredProjects = parser.getFilteredProjects();
+		} catch (IOException e) {
+			throw new ManifestErrorException(e);
 		} finally {
 			try {
-				if (inputStream != null)
+				inputStream.close();
-					inputStream.close();
 			} catch (IOException e) {
 				// Just ignore it, it's not important.
 			}
 		}
 
 		if (repo.isBare()) {
+			bareProjects = new ArrayList<>();
+			if (author == null)
+				author = new PersonIdent(repo);
+			if (callback == null)
+				callback = new DefaultRemoteReader();
+			for (RepoProject proj : filteredProjects) {
+				addSubmoduleBare(proj.getUrl(), proj.getPath(),
+						proj.getRevision(), proj.getCopyFiles(),
+						proj.getLinkFiles(), proj.getGroups(),
+						proj.getRecommendShallow());
+			}
 			DirCache index = DirCache.newInCore();
 			DirCacheBuilder builder = index.builder();
 			ObjectInserter inserter = repo.newObjectInserter();
@@ -689,53 +680,62 @@ public class RepoCommand extends GitCommand<RevCommit> {
 				}
 
 				return rw.parseCommit(commitId);
-			} catch (IOException e) {
+			} catch (GitAPIException | IOException e) {
 				throw new ManifestErrorException(e);
 			}
 		} else {
-			return git
+			try (Git git = new Git(repo)) {
-				.commit()
+				for (RepoProject proj : filteredProjects) {
-				.setMessage(RepoText.get().repoCommitMessage)
+					addSubmodule(proj.getUrl(), proj.getPath(),
-				.call();
+							proj.getRevision(), proj.getCopyFiles(),
+							proj.getLinkFiles(), git);
+				}
+				return git.commit().setMessage(RepoText.get().repoCommitMessage)
+						.call();
+			} catch (GitAPIException | IOException e) {
+				throw new ManifestErrorException(e);
+			}
 		}
 	}
 
 	private void addSubmodule(String url, String path, String revision,
-			List<CopyFile> copyfiles, List<LinkFile> linkfiles,
+			List<CopyFile> copyfiles, List<LinkFile> linkfiles, Git git)
-			Set<String> groups, String recommendShallow)
 			throws GitAPIException, IOException {
-		if (repo.isBare()) {
+		assert (!repo.isBare());
-			RepoProject proj = new RepoProject(url, path, revision, null, groups, recommendShallow);
+		assert (git != null);
-			proj.addCopyFiles(copyfiles);
+		if (!linkfiles.isEmpty()) {
-			proj.addLinkFiles(linkfiles);
+			throw new UnsupportedOperationException(
-			bareProjects.add(proj);
+					JGitText.get().nonBareLinkFilesNotSupported);
-		} else {
+		}
-			if (!linkfiles.isEmpty()) {
-				throw new UnsupportedOperationException(
-						JGitText.get().nonBareLinkFilesNotSupported);
-			}
 
-			SubmoduleAddCommand add = git
+		SubmoduleAddCommand add = git.submoduleAdd().setPath(path).setURI(url);
-				.submoduleAdd()
+		if (monitor != null)
-				.setPath(path)
+			add.setProgressMonitor(monitor);
-				.setURI(url);
+
-			if (monitor != null)
+		Repository subRepo = add.call();
-				add.setProgressMonitor(monitor);
+		if (revision != null) {
-
+			try (Git sub = new Git(subRepo)) {
-			Repository subRepo = add.call();
+				sub.checkout().setName(findRef(revision, subRepo)).call();
-			if (revision != null) {
-				try (Git sub = new Git(subRepo)) {
-					sub.checkout().setName(findRef(revision, subRepo))
-							.call();
-				}
-				subRepo.close();
-				git.add().addFilepattern(path).call();
-			}
-			for (CopyFile copyfile : copyfiles) {
-				copyfile.copy();
-				git.add().addFilepattern(copyfile.dest).call();
 			}
+			subRepo.close();
+			git.add().addFilepattern(path).call();
 		}
+		for (CopyFile copyfile : copyfiles) {
+			copyfile.copy();
+			git.add().addFilepattern(copyfile.dest).call();
+		}
+	}
+
+	private void addSubmoduleBare(String url, String path, String revision,
+			List<CopyFile> copyfiles, List<LinkFile> linkfiles,
+			Set<String> groups, String recommendShallow) {
+		assert (repo.isBare());
+		assert (bareProjects != null);
+		RepoProject proj = new RepoProject(url, path, revision, null, groups,
+				recommendShallow);
+		proj.addCopyFiles(copyfiles);
+		proj.addLinkFiles(linkfiles);
+		bareProjects.add(proj);
 	}
 
 	/*