Adapt diff- and merge tool code for PGM and EGit usage

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

* DiffTools and MergeTools
  * store FS, gitDir and workTree for usage without
git repository (for EGit preferences)
  * add getUserDefinedToolNames() and getPredefinedToolNames()
  * replace getToolNames() with getAllToolNames() that combines the two
lists and put default tool name (diff.tool or merge.tool) as first
element (for EGit preferences)
* FileElement: refactoring of getFile() and friends to have midName
(LOCAL, REMOTE etc.) always added to the temp file name (also for EGit)
* FileElement: added directory attribute that is used in getFile() to
return path with workDir as parent
* DiffTool and MergeTool
  * added errw.flush(), because sometimes stderr is not printed in case
of die()
  * print e.getMessage() always to stderr
* Moved toolname and prompt logic into managers
* Exported internal packages required for egit.ui

Bug: 356832
Change-Id: I71e7f4dc362169a7612ca4f6546a021bc4b2b5f4
Signed-off-by: Andre Bossert <andre.bossert@siemens.com>
Signed-off-by: Tim Neumann <Tim.Neumann@advantest.com>

3 files changed, 220 insertions, 26 deletions

diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DiffToolTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DiffToolTest.java
index a258821f0c..ce4c004c09 100644
--- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DiffToolTest.java
+++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DiffToolTest.java
@@ -16,11 +16,14 @@ import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PROMPT;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_TOOL;
 import static org.junit.Assert.fail;
 
+import java.io.File;
 import java.io.InputStream;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import org.eclipse.jgit.internal.diffmergetool.DiffTools;
 import org.eclipse.jgit.internal.diffmergetool.ExternalDiffTool;
@@ -42,6 +45,58 @@ public class DiffToolTest extends ToolTestCase {
 		configureEchoTool(TOOL_NAME);
 	}
 
+	@Test(expected = Die.class)
+	public void testUndefinedTool() throws Exception {
+		String toolName = "undefined";
+		String[] conflictingFilenames = createUnstagedChanges();
+
+		List<String> expectedErrors = new ArrayList<>();
+		for (String changedFilename : conflictingFilenames) {
+			expectedErrors.add("External diff tool is not defined: " + toolName);
+			expectedErrors.add("compare of " + changedFilename + " failed");
+		}
+
+		runAndCaptureUsingInitRaw(expectedErrors, DIFF_TOOL, "--no-prompt",
+				"--tool", toolName);
+		fail("Expected exception to be thrown due to undefined external tool");
+	}
+
+	@Test(expected = Die.class)
+	public void testUserToolWithCommandNotFoundError() throws Exception {
+		String toolName = "customTool";
+
+		int errorReturnCode = 127; // command not found
+		String command = "exit " + errorReturnCode;
+
+		StoredConfig config = db.getConfig();
+		config.setString(CONFIG_DIFFTOOL_SECTION, toolName, CONFIG_KEY_CMD,
+				command);
+
+		createMergeConflict();
+		runAndCaptureUsingInitRaw(DIFF_TOOL, "--no-prompt", "--tool", toolName);
+
+		fail("Expected exception to be thrown due to external tool exiting with error code: "
+				+ errorReturnCode);
+	}
+
+	@Test
+	public void testEmptyToolName() throws Exception {
+		String emptyToolName = "";
+
+		StoredConfig config = db.getConfig();
+		// the default diff tool is configured without a subsection
+		String subsection = null;
+		config.setString(CONFIG_DIFF_SECTION, subsection, CONFIG_KEY_TOOL,
+				emptyToolName);
+
+		createUnstagedChanges();
+
+		String araxisErrorLine = "compare: unrecognized option `-wait' @ error/compare.c/CompareImageCommand/1123.";
+		String[] expectedErrorOutput = { araxisErrorLine, araxisErrorLine, };
+		runAndCaptureUsingInitRaw(Arrays.asList(expectedErrorOutput), DIFF_TOOL,
+				"--no-prompt");
+	}
+
 	@Test
 	public void testToolWithPrompt() throws Exception {
 		String[] inputLines = {
@@ -138,12 +193,12 @@ public class DiffToolTest extends ToolTestCase {
 	@Test
 	public void testToolCached() throws Exception {
 		String[] conflictingFilenames = createStagedChanges();
-		String[] expectedOutput = getExpectedToolOutputNoPrompt(conflictingFilenames);
+		Pattern[] expectedOutput = getExpectedCachedToolOutputNoPrompt(conflictingFilenames);
 
 		String[] options = { "--cached", "--staged", };
 
 		for (String option : options) {
-			assertArrayOfLinesEquals("Incorrect output for option: " + option,
+			assertArrayOfMatchingLines("Incorrect output for option: " + option,
 					expectedOutput, runAndCaptureUsingInitRaw(DIFF_TOOL,
 							option, "--tool", TOOL_NAME));
 		}
@@ -213,43 +268,76 @@ public class DiffToolTest extends ToolTestCase {
 				String.valueOf(false));
 	}
 
-	private static String[] getExpectedToolOutputNoPrompt(String[] conflictingFilenames) {
+	private String[] getExpectedToolOutputNoPrompt(String[] conflictingFilenames) {
 		String[] expectedToolOutput = new String[conflictingFilenames.length];
 		for (int i = 0; i < conflictingFilenames.length; ++i) {
 			String newPath = conflictingFilenames[i];
-			String expectedLine = newPath;
-			expectedToolOutput[i] = expectedLine;
+			Path fullPath = getFullPath(newPath);
+			expectedToolOutput[i] = fullPath.toString();
 		}
 		return expectedToolOutput;
 	}
 
-	private static String[] getExpectedCompareOutput(String[] conflictingFilenames) {
+	private Pattern[] getExpectedCachedToolOutputNoPrompt(String[] conflictingFilenames) {
+		String tmpDir = System.getProperty("java.io.tmpdir");
+		if (tmpDir.endsWith(File.separator)) {
+			tmpDir = tmpDir.substring(0, tmpDir.length() - 1);
+		}
+		Pattern emptyPattern = Pattern.compile("");
+		List<Pattern> expectedToolOutput = new ArrayList<>();
+		for (int i = 0; i < conflictingFilenames.length; ++i) {
+			String changedFilename = conflictingFilenames[i];
+			Path fullPath = getFullPath(changedFilename);
+			String filename = fullPath.getFileName().toString();
+			String regexp = tmpDir + File.separatorChar + filename
+					+ "_REMOTE_.*";
+			Pattern pattern = Pattern.compile(regexp);
+			expectedToolOutput.add(pattern);
+			expectedToolOutput.add(emptyPattern);
+		}
+		expectedToolOutput.add(emptyPattern);
+		return expectedToolOutput.toArray(new Pattern[0]);
+	}
+
+	private String[] getExpectedCompareOutput(String[] conflictingFilenames) {
 		List<String> expected = new ArrayList<>();
 		int n = conflictingFilenames.length;
 		for (int i = 0; i < n; ++i) {
-			String newPath = conflictingFilenames[i];
+			String changedFilename = conflictingFilenames[i];
 			expected.add(
-					"Viewing (" + (i + 1) + "/" + n + "): '" + newPath + "'");
+					"Viewing (" + (i + 1) + "/" + n + "): '" + changedFilename
+							+ "'");
 			expected.add("Launch '" + TOOL_NAME + "' [Y/n]?");
-			expected.add(newPath);
+			Path fullPath = getFullPath(changedFilename);
+			expected.add(fullPath.toString());
 		}
 		return expected.toArray(new String[0]);
 	}
 
-	private static String[] getExpectedAbortOutput(String[] conflictingFilenames,
+	private String[] getExpectedAbortOutput(String[] conflictingFilenames,
 			int abortIndex) {
 		List<String> expected = new ArrayList<>();
 		int n = conflictingFilenames.length;
 		for (int i = 0; i < n; ++i) {
-			String newPath = conflictingFilenames[i];
+			String changedFilename = conflictingFilenames[i];
 			expected.add(
-					"Viewing (" + (i + 1) + "/" + n + "): '" + newPath + "'");
+					"Viewing (" + (i + 1) + "/" + n + "): '" + changedFilename
+							+ "'");
 			expected.add("Launch '" + TOOL_NAME + "' [Y/n]?");
 			if (i == abortIndex) {
 				break;
 			}
-			expected.add(newPath);
+			Path fullPath = getFullPath(changedFilename);
+			expected.add(fullPath.toString());
 		}
 		return expected.toArray(new String[0]);
 	}
+
+	private static String getEchoCommand() {
+		/*
+		 * use 'REMOTE' placeholder, as it will be replaced by a file path
+		 * within the repository.
+		 */
+		return "(echo \"$REMOTE\")";
+	}
 }
diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java
index e9d559e70e..1236dd30d3 100644
--- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java
+++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java
@@ -14,8 +14,10 @@ import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PROMPT;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_TOOL;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_MERGETOOL_SECTION;
 import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_MERGE_SECTION;
+import static org.junit.Assert.fail;
 
 import java.io.InputStream;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -42,6 +44,58 @@ public class MergeToolTest extends ToolTestCase {
 	}
 
 	@Test
+	public void testUndefinedTool() throws Exception {
+		String toolName = "undefined";
+		String[] conflictingFilenames = createMergeConflict();
+
+		List<String> expectedErrors = new ArrayList<>();
+		for (String conflictingFilename : conflictingFilenames) {
+			expectedErrors.add("External merge tool is not defined: " + toolName);
+			expectedErrors.add("merge of " + conflictingFilename + " failed");
+		}
+
+		runAndCaptureUsingInitRaw(expectedErrors, MERGE_TOOL,
+				"--no-prompt", "--tool", toolName);
+	}
+
+	@Test(expected = Die.class)
+	public void testUserToolWithCommandNotFoundError() throws Exception {
+		String toolName = "customTool";
+
+		int errorReturnCode = 127; // command not found
+		String command = "exit " + errorReturnCode;
+
+		StoredConfig config = db.getConfig();
+		config.setString(CONFIG_MERGETOOL_SECTION, toolName, CONFIG_KEY_CMD,
+				command);
+
+		createMergeConflict();
+		runAndCaptureUsingInitRaw(MERGE_TOOL, "--no-prompt", "--tool",
+				toolName);
+
+		fail("Expected exception to be thrown due to external tool exiting with error code: "
+				+ errorReturnCode);
+	}
+
+	@Test
+	public void testEmptyToolName() throws Exception {
+		String emptyToolName = "";
+
+		StoredConfig config = db.getConfig();
+		// the default merge tool is configured without a subsection
+		String subsection = null;
+		config.setString(CONFIG_MERGE_SECTION, subsection, CONFIG_KEY_TOOL,
+				emptyToolName);
+
+		createMergeConflict();
+
+		String araxisErrorLine = "compare: unrecognized option `-wait' @ error/compare.c/CompareImageCommand/1123.";
+		String[] expectedErrorOutput = { araxisErrorLine, araxisErrorLine, };
+		runAndCaptureUsingInitRaw(Arrays.asList(expectedErrorOutput),
+				MERGE_TOOL, "--no-prompt");
+	}
+
+	@Test
 	public void testAbortMerge() throws Exception {
 		String[] inputLines = {
 				"y", // start tool for merge resolution
@@ -220,7 +274,7 @@ public class MergeToolTest extends ToolTestCase {
 				String.valueOf(false));
 	}
 
-	private static String[] getExpectedMergeConflictOutputNoPrompt(
+	private String[] getExpectedMergeConflictOutputNoPrompt(
 			String[] conflictFilenames) {
 		List<String> expected = new ArrayList<>();
 		expected.add("Merging:");
@@ -232,7 +286,8 @@ public class MergeToolTest extends ToolTestCase {
 					+ "':");
 			expected.add("{local}: modified file");
 			expected.add("{remote}: modified file");
-			expected.add(conflictFilename);
+			Path filePath = getFullPath(conflictFilename);
+			expected.add(filePath.toString());
 			expected.add(conflictFilename + " seems unchanged.");
 		}
 		return expected.toArray(new String[0]);
@@ -257,7 +312,7 @@ public class MergeToolTest extends ToolTestCase {
 		return expected.toArray(new String[0]);
 	}
 
-	private static String[] getExpectedAbortMergeOutput(
+	private String[] getExpectedAbortMergeOutput(
 			String[] conflictFilenames, int abortIndex) {
 		List<String> expected = new ArrayList<>();
 		expected.add("Merging:");
@@ -274,8 +329,9 @@ public class MergeToolTest extends ToolTestCase {
 					"Normal merge conflict for '" + conflictFilename + "':");
 			expected.add("{local}: modified file");
 			expected.add("{remote}: modified file");
+			Path fullPath = getFullPath(conflictFilename);
 			expected.add("Hit return to start merge resolution tool ("
-					+ TOOL_NAME + "): " + conflictFilename);
+					+ TOOL_NAME + "): " + fullPath);
 			expected.add(conflictFilename + " seems unchanged.");
 			expected.add("Was the merge successful [y/n]?");
 			if (i < conflictFilenames.length - 1) {
@@ -286,7 +342,7 @@ public class MergeToolTest extends ToolTestCase {
 		return expected.toArray(new String[0]);
 	}
 
-	private static String[] getExpectedMergeConflictOutput(
+	private String[] getExpectedMergeConflictOutput(
 			String[] conflictFilenames) {
 		List<String> expected = new ArrayList<>();
 		expected.add("Merging:");
@@ -299,8 +355,9 @@ public class MergeToolTest extends ToolTestCase {
 					+ "':");
 			expected.add("{local}: modified file");
 			expected.add("{remote}: modified file");
+			Path filePath = getFullPath(conflictFilename);
 			expected.add("Hit return to start merge resolution tool ("
-					+ TOOL_NAME + "): " + conflictFilename);
+					+ TOOL_NAME + "): " + filePath);
 			expected.add(conflictFilename + " seems unchanged.");
 			expected.add("Was the merge successful [y/n]?");
 			if (i < conflictFilenames.length - 1) {
@@ -327,4 +384,12 @@ public class MergeToolTest extends ToolTestCase {
 		}
 		return expected.toArray(new String[0]);
 	}
+
+	private static String getEchoCommand() {
+		/*
+		 * use 'MERGED' placeholder, as both 'LOCAL' and 'REMOTE' will be
+		 * replaced with full paths to a temporary file during some of the tests
+		 */
+		return "(echo \"$MERGED\")";
+	}
 }
diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ToolTestCase.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ToolTestCase.java
index 933f19bcc4..a3c41f0fed 100644
--- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ToolTestCase.java
+++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ToolTestCase.java
@@ -10,13 +10,18 @@
 package org.eclipse.jgit.pgm;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import org.eclipse.jgit.api.Git;
@@ -29,6 +34,7 @@ import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.junit.Before;
 import org.kohsuke.args4j.Argument;
+import org.kohsuke.args4j.CmdLineException;
 
 /**
  * Base test case for the {@code difftool} and {@code mergetool} commands.
@@ -64,8 +70,23 @@ public abstract class ToolTestCase extends CLIRepositoryTestCase {
 		return runAndCaptureUsingInitRaw(inputStream, args);
 	}
 
+	protected String[] runAndCaptureUsingInitRaw(
+			List<String> expectedErrorOutput, String... args) throws Exception {
+		InputStream inputStream = null; // no input stream
+		return runAndCaptureUsingInitRaw(inputStream, expectedErrorOutput,
+				args);
+	}
+
 	protected String[] runAndCaptureUsingInitRaw(InputStream inputStream,
 			String... args) throws Exception {
+		List<String> expectedErrorOutput = Collections.emptyList();
+		return runAndCaptureUsingInitRaw(inputStream, expectedErrorOutput,
+				args);
+	}
+
+	protected String[] runAndCaptureUsingInitRaw(InputStream inputStream,
+			List<String> expectedErrorOutput, String... args)
+			throws CmdLineException, Exception, IOException {
 		CLIGitCommand.Result result = new CLIGitCommand.Result();
 
 		GitCliJGitWrapperParser bean = new GitCliJGitWrapperParser();
@@ -86,7 +107,7 @@ public abstract class ToolTestCase extends CLIRepositoryTestCase {
 				.filter(l -> !l.isBlank()) // we care only about error messages
 				.collect(Collectors.toList());
 		assertEquals("Expected no standard error output from tool",
-				Collections.EMPTY_LIST.toString(), errLines.toString());
+				expectedErrorOutput.toString(), errLines.toString());
 
 		return result.outLines().toArray(new String[0]);
 	}
@@ -177,6 +198,13 @@ public abstract class ToolTestCase extends CLIRepositoryTestCase {
 		return changes;
 	}
 
+	protected Path getFullPath(String repositoryFilename) {
+		Path dotGitPath = db.getDirectory().toPath();
+		Path repositoryRoot = dotGitPath.getParent();
+		Path repositoryFilePath = repositoryRoot.resolve(repositoryFilename);
+		return repositoryFilePath;
+	}
+
 	protected static InputStream createInputStream(String[] inputLines) {
 		return createInputStream(Arrays.asList(inputLines));
 	}
@@ -192,11 +220,24 @@ public abstract class ToolTestCase extends CLIRepositoryTestCase {
 		assertEquals(failMessage, toString(expected), toString(actual));
 	}
 
-	protected static String getEchoCommand() {
-		/*
-		 * use 'MERGED' placeholder, as both 'LOCAL' and 'REMOTE' will be
-		 * replaced with full paths to a temporary file during some of the tests
-		 */
-		return "(echo \"$MERGED\")";
+	protected static void assertArrayOfMatchingLines(String failMessage,
+			Pattern[] expected, String[] actual) {
+		assertEquals(failMessage + System.lineSeparator()
+				+ "Expected and actual lines count don't match. Expected: "
+				+ Arrays.asList(expected) + ", actual: "
+				+ Arrays.asList(actual), expected.length, actual.length);
+		int n = expected.length;
+		for (int i = 0; i < n; ++i) {
+			Pattern expectedPattern = expected[i];
+			String actualLine = actual[i];
+			Matcher matcher = expectedPattern.matcher(actualLine);
+			boolean matches = matcher.matches();
+			assertTrue(failMessage + System.lineSeparator() + "Line " + i + " '"
+					+ actualLine + "' doesn't match expected pattern: "
+					+ expectedPattern + System.lineSeparator() + "Expected: "
+					+ Arrays.asList(expected) + ", actual: "
+					+ Arrays.asList(actual),
+					matches);
+		}
 	}
 }