Add availability check of pre-defined tools

see: https://git-scm.com/docs/git-difftool
see: https://git-scm.com/docs/git-mergetool

* now all available tools are printed with "--tool-help"
* if no diff.tool or merge.tool is defined the first available
pre-defined tool is used

TODO:
- add mergetools to difftools --> extra change or merge to this
- return the exit-code of the tool to jgit / java runtime

Bug: 356832
Change-Id: I20fb04e71ced981f5625020f461bbac24e6cec70
Signed-off-by: Andre Bossert <andre.bossert@siemens.com>

10 files changed, 207 insertions, 26 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java
index ad79fe8fc6..668adeab65 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java
@@ -14,13 +14,19 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Map;
+
+import org.eclipse.jgit.errors.NoWorkTreeException;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.FS.ExecutionResult;
 import org.eclipse.jgit.util.FS_POSIX;
 import org.eclipse.jgit.util.FS_Win32;
 import org.eclipse.jgit.util.FS_Win32_Cygwin;
+import org.eclipse.jgit.util.StringUtils;
 
 /**
  * Runs a command with help of FS.
@@ -91,6 +97,49 @@ public class CommandExecutor {
 		}
 	}
 
+	/**
+	 * @param path
+	 *            the executable path
+	 * @param workingDir
+	 *            the working directory
+	 * @param env
+	 *            the environment
+	 * @return the execution result
+	 * @throws ToolException
+	 * @throws InterruptedException
+	 * @throws IOException
+	 */
+	public boolean checkExecutable(String path, File workingDir,
+			Map<String, String> env)
+			throws ToolException, IOException, InterruptedException {
+		checkUseMsys2(path);
+		String command = null;
+		if (fs instanceof FS_Win32 && !useMsys2) {
+			Path p = Paths.get(path);
+			// Win32 (and not cygwin or MSYS2) where accepts only command / exe
+			// name as parameter
+			// so check if exists and executable in this case
+			if (p.isAbsolute() && Files.isExecutable(p)) {
+				return true;
+			}
+			// try where command for all other cases
+			command = "where " + ExternalToolUtils.quotePath(path); //$NON-NLS-1$
+		} else {
+			command = "which " + ExternalToolUtils.quotePath(path); //$NON-NLS-1$
+		}
+		boolean available = true;
+		try {
+			ExecutionResult rc = run(command, workingDir, env);
+			if (rc.getRc() != 0) {
+				available = false;
+			}
+		} catch (IOException | InterruptedException | NoWorkTreeException
+				| ToolException e) {
+			// no op: is true to not hide possible tools from user
+		}
+		return available;
+	}
+
 	private void deleteCommandArray() {
 		deleteCommandFile();
 	}
@@ -127,7 +176,7 @@ public class CommandExecutor {
 	private void checkUseMsys2(String command) {
 		useMsys2 = false;
 		String useMsys2Str = System.getProperty("jgit.usemsys2bash"); //$NON-NLS-1$
-		if (useMsys2Str != null && !useMsys2Str.isEmpty()) {
+		if (!StringUtils.isEmptyOrNull(useMsys2Str)) {
 			if (useMsys2Str.equalsIgnoreCase("auto")) { //$NON-NLS-1$
 				useMsys2 = command.contains(".sh"); //$NON-NLS-1$
 			} else {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandLineDiffTool.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandLineDiffTool.java
index 509515c37a..00dec32718 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandLineDiffTool.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandLineDiffTool.java
@@ -111,7 +111,7 @@ public enum CommandLineDiffTool {
 	 * See: <a href=
 	 * "http://vimdoc.sourceforge.net/htmldoc/diff.html">http://vimdoc.sourceforge.net/htmldoc/diff.html</a>
 	 */
-	gvimdiff("gviewdiff", "\"$LOCAL\" \"$REMOTE\""),
+	gvimdiff("gvimdiff", "\"$LOCAL\" \"$REMOTE\""),
 	/**
 	 * See: <a href=
 	 * "http://vimdoc.sourceforge.net/htmldoc/diff.html">http://vimdoc.sourceforge.net/htmldoc/diff.html</a>
@@ -160,7 +160,7 @@ public enum CommandLineDiffTool {
 	 * See: <a href=
 	 * "http://vimdoc.sourceforge.net/htmldoc/diff.html">http://vimdoc.sourceforge.net/htmldoc/diff.html</a>
 	 */
-	vimdiff("viewdiff", gvimdiff),
+	vimdiff("vimdiff", gvimdiff),
 	/**
 	 * See: <a href=
 	 * "http://vimdoc.sourceforge.net/htmldoc/diff.html">http://vimdoc.sourceforge.net/htmldoc/diff.html</a>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffTools.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffTools.java
index 1dcc523bf8..6c1a780a35 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffTools.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffTools.java
@@ -111,17 +111,38 @@ public class DiffTools {
 	}
 
 	/**
-	 * @return the available predefined tools
+	 * @param checkAvailability
+	 *            true: for checking if tools can be executed; ATTENTION: this
+	 *            check took some time, do not execute often (store the map for
+	 *            other actions); false: availability is NOT checked:
+	 *            isAvailable() returns default false is this case!
+	 * @return the predefined tools with optionally checked availability (long
+	 *         running operation)
 	 */
-	public Map<String, ExternalDiffTool> getAvailableTools() {
+	public Map<String, ExternalDiffTool> getPredefinedTools(
+			boolean checkAvailability) {
+		if (checkAvailability) {
+			for (ExternalDiffTool tool : predefinedTools.values()) {
+				PreDefinedDiffTool predefTool = (PreDefinedDiffTool) tool;
+				predefTool.setAvailable(ExternalToolUtils.isToolAvailable(repo,
+						predefTool.getPath()));
+			}
+		}
 		return Collections.unmodifiableMap(predefinedTools);
 	}
 
 	/**
-	 * @return the NOT available predefined tools
+	 * @return the name of first available predefined tool or null
 	 */
-	public Map<String, ExternalDiffTool> getNotAvailableTools() {
-		return Collections.unmodifiableMap(new TreeMap<>());
+	public String getFirstAvailableTool() {
+		String name = null;
+		for (ExternalDiffTool tool : predefinedTools.values()) {
+			if (ExternalToolUtils.isToolAvailable(repo, tool.getPath())) {
+				name = tool.getName();
+				break;
+			}
+		}
+		return name;
 	}
 
 	/**
@@ -146,9 +167,12 @@ public class DiffTools {
 		if (StringUtils.isEmptyOrNull(toolName)) {
 			toolName = getDefaultToolName(gui);
 		}
-		ExternalDiffTool tool = getTool(toolName);
+		ExternalDiffTool tool = null;
+		if (!StringUtils.isEmptyOrNull(toolName)) {
+			tool = getTool(toolName);
+		}
 		if (tool == null) {
-			throw new ToolException("Unknown diff tool " + toolName); //$NON-NLS-1$
+			throw new ToolException("Unknown diff tool '" + toolName + "'"); //$NON-NLS-1$ //$NON-NLS-2$
 		}
 		return tool;
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalDiffTool.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalDiffTool.java
index f2d7e828cb..e01b892a53 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalDiffTool.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalDiffTool.java
@@ -30,4 +30,10 @@ public interface ExternalDiffTool {
 	 */
 	String getCommand();
 
+	/**
+	 * @return availability of the tool: true if tool can be executed and false
+	 *         if not
+	 */
+	boolean isAvailable();
+
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalToolUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalToolUtils.java
index 3efb90c490..e10607d2fd 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalToolUtils.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ExternalToolUtils.java
@@ -39,9 +39,15 @@ public class ExternalToolUtils {
 	public static String prepareCommand(String command, FileElement localFile,
 			FileElement remoteFile, FileElement mergedFile,
 			FileElement baseFile) throws IOException {
-		command = localFile.replaceVariable(command);
-		command = remoteFile.replaceVariable(command);
-		command = mergedFile.replaceVariable(command);
+		if (localFile != null) {
+			command = localFile.replaceVariable(command);
+		}
+		if (remoteFile != null) {
+			command = remoteFile.replaceVariable(command);
+		}
+		if (mergedFile != null) {
+			command = mergedFile.replaceVariable(command);
+		}
 		if (baseFile != null) {
 			command = baseFile.replaceVariable(command);
 		}
@@ -69,13 +75,59 @@ public class ExternalToolUtils {
 			FileElement mergedFile, FileElement baseFile) throws IOException {
 		Map<String, String> env = new TreeMap<>();
 		env.put(Constants.GIT_DIR_KEY, repo.getDirectory().getAbsolutePath());
-		localFile.addToEnv(env);
-		remoteFile.addToEnv(env);
-		mergedFile.addToEnv(env);
+		if (localFile != null) {
+			localFile.addToEnv(env);
+		}
+		if (remoteFile != null) {
+			remoteFile.addToEnv(env);
+		}
+		if (mergedFile != null) {
+			mergedFile.addToEnv(env);
+		}
 		if (baseFile != null) {
 			baseFile.addToEnv(env);
 		}
 		return env;
 	}
 
+	/**
+	 * @param path
+	 *            the path to be quoted
+	 * @return quoted path if it contains spaces
+	 */
+	@SuppressWarnings("nls")
+	public static String quotePath(String path) {
+		// handling of spaces in path
+		if (path.contains(" ")) {
+			// add quotes before if needed
+			if (!path.startsWith("\"")) {
+				path = "\"" + path;
+			}
+			// add quotes after if needed
+			if (!path.endsWith("\"")) {
+				path = path + "\"";
+			}
+		}
+		return path;
+	}
+
+	/**
+	 * @param repo
+	 *            the repository
+	 * @param path
+	 *            the tool path
+	 * @return true if tool available and false otherwise
+	 */
+	public static boolean isToolAvailable(Repository repo, String path) {
+		boolean available = true;
+		try {
+			CommandExecutor cmdExec = new CommandExecutor(repo.getFS(), false);
+			available = cmdExec.checkExecutable(path, repo.getWorkTree(),
+					prepareEnvironment(repo, null, null, null, null));
+		} catch (Exception e) {
+			available = false;
+		}
+		return available;
+	}
+
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/MergeTools.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/MergeTools.java
index 9a2a8304eb..d91d57f1a8 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/MergeTools.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/MergeTools.java
@@ -23,6 +23,7 @@ import org.eclipse.jgit.internal.diffmergetool.FileElement.Type;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.internal.BooleanTriState;
 import org.eclipse.jgit.util.FS.ExecutionResult;
+import org.eclipse.jgit.util.StringUtils;
 
 /**
  * Manages merge tools.
@@ -161,17 +162,38 @@ public class MergeTools {
 	}
 
 	/**
-	 * @return the available predefined tools
+	 * @param checkAvailability
+	 *            true: for checking if tools can be executed; ATTENTION: this
+	 *            check took some time, do not execute often (store the map for
+	 *            other actions); false: availability is NOT checked:
+	 *            isAvailable() returns default false is this case!
+	 * @return the predefined tools with optionally checked availability (long
+	 *         running operation)
 	 */
-	public Map<String, ExternalMergeTool> getAvailableTools() {
+	public Map<String, ExternalMergeTool> getPredefinedTools(
+			boolean checkAvailability) {
+		if (checkAvailability) {
+			for (ExternalMergeTool tool : predefinedTools.values()) {
+				PreDefinedMergeTool predefTool = (PreDefinedMergeTool) tool;
+				predefTool.setAvailable(ExternalToolUtils.isToolAvailable(repo,
+						predefTool.getPath()));
+			}
+		}
 		return predefinedTools;
 	}
 
 	/**
-	 * @return the NOT available predefined tools
+	 * @return the name of first available predefined tool or null
 	 */
-	public Map<String, ExternalMergeTool> getNotAvailableTools() {
-		return new TreeMap<>();
+	public String getFirstAvailableTool() {
+		String name = null;
+		for (ExternalMergeTool tool : predefinedTools.values()) {
+			if (ExternalToolUtils.isToolAvailable(repo, tool.getPath())) {
+				name = tool.getName();
+				break;
+			}
+		}
+		return name;
 	}
 
 	/**
@@ -193,12 +215,15 @@ public class MergeTools {
 
 	private ExternalMergeTool guessTool(String toolName, BooleanTriState gui)
 			throws ToolException {
-		if ((toolName == null) || toolName.isEmpty()) {
+		if (StringUtils.isEmptyOrNull(toolName)) {
 			toolName = getDefaultToolName(gui);
 		}
-		ExternalMergeTool tool = getTool(toolName);
+		ExternalMergeTool tool = null;
+		if (!StringUtils.isEmptyOrNull(toolName)) {
+			tool = getTool(toolName);
+		}
 		if (tool == null) {
-			throw new ToolException("Unknown diff tool " + toolName); //$NON-NLS-1$
+			throw new ToolException("Unknown merge tool '" + toolName + "'"); //$NON-NLS-1$ //$NON-NLS-2$
 		}
 		return tool;
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedDiffTool.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedDiffTool.java
index 092cb605be..e1169a2d60 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedDiffTool.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedDiffTool.java
@@ -56,7 +56,7 @@ public class PreDefinedDiffTool extends UserDefinedDiffTool {
 	 */
 	@Override
 	public String getCommand() {
-		return getPath() + " " + super.getCommand(); //$NON-NLS-1$
+		return ExternalToolUtils.quotePath(getPath()) + " " + super.getCommand(); //$NON-NLS-1$
 	}
 
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedMergeTool.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedMergeTool.java
index 2c64c16667..7b28d32820 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedMergeTool.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/PreDefinedMergeTool.java
@@ -84,7 +84,7 @@ public class PreDefinedMergeTool extends UserDefinedMergeTool {
 	 */
 	@Override
 	public String getCommand(boolean withBase) {
-		return getPath() + " " //$NON-NLS-1$
+		return ExternalToolUtils.quotePath(getPath()) + " " //$NON-NLS-1$
 				+ (withBase ? super.getCommand() : parametersWithoutBase);
 	}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java
index 27f7d12e66..7cc5bb50d9 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java
@@ -110,6 +110,9 @@ public class ToolException extends Exception {
 	 * @return the result Stderr
 	 */
 	public String getResultStderr() {
+		if (result == null) {
+			return ""; //$NON-NLS-1$
+		}
 		try {
 			return new String(result.getStderr().toByteArray());
 		} catch (Exception e) {
@@ -122,6 +125,9 @@ public class ToolException extends Exception {
 	 * @return the result Stdout
 	 */
 	public String getResultStdout() {
+		if (result == null) {
+			return ""; //$NON-NLS-1$
+		}
 		try {
 			return new String(result.getStdout().toByteArray());
 		} catch (Exception e) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/UserDefinedDiffTool.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/UserDefinedDiffTool.java
index 012296eb35..eb72d01cdb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/UserDefinedDiffTool.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/UserDefinedDiffTool.java
@@ -15,6 +15,8 @@ package org.eclipse.jgit.internal.diffmergetool;
  */
 public class UserDefinedDiffTool implements ExternalDiffTool {
 
+	private boolean available;
+
 	/**
 	 * the diff tool name
 	 */
@@ -99,6 +101,23 @@ public class UserDefinedDiffTool implements ExternalDiffTool {
 	}
 
 	/**
+	 * @return availability of the tool: true if tool can be executed and false
+	 *         if not
+	 */
+	@Override
+	public boolean isAvailable() {
+		return available;
+	}
+
+	/**
+	 * @param available
+	 *            true if tool can be found and false if not
+	 */
+	public void setAvailable(boolean available) {
+		this.available = available;
+	}
+
+	/**
 	 * Overrides the path for the given tool. Equivalent to setting
 	 * {@code difftool.<tool>.path}.
 	 *