From 973e955ead1a9bf41efb3168baf5b68527e78023 Mon Sep 17 00:00:00 2001 From: Andre Bossert Date: Sun, 19 Jan 2020 20:56:28 +0100 Subject: [PATCH] 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 --- .../org/eclipse/jgit/pgm/DiffToolTest.java | 30 ++++++-- .../org/eclipse/jgit/pgm/MergeToolTest.java | 30 ++++++-- .../jgit/pgm/internal/CLIText.properties | 4 + .../src/org/eclipse/jgit/pgm/DiffTool.java | 77 ++++++++++++------- .../src/org/eclipse/jgit/pgm/MergeTool.java | 46 ++++++++--- .../eclipse/jgit/pgm/internal/CLIText.java | 4 + .../diffmergetool/ExternalDiffToolTest.java | 13 +--- .../diffmergetool/ExternalMergeToolTest.java | 17 +--- .../diffmergetool/CommandExecutor.java | 51 +++++++++++- .../diffmergetool/CommandLineDiffTool.java | 4 +- .../internal/diffmergetool/DiffTools.java | 38 +++++++-- .../diffmergetool/ExternalDiffTool.java | 6 ++ .../diffmergetool/ExternalToolUtils.java | 64 +++++++++++++-- .../internal/diffmergetool/MergeTools.java | 41 ++++++++-- .../diffmergetool/PreDefinedDiffTool.java | 2 +- .../diffmergetool/PreDefinedMergeTool.java | 2 +- .../internal/diffmergetool/ToolException.java | 6 ++ .../diffmergetool/UserDefinedDiffTool.java | 19 +++++ 18 files changed, 357 insertions(+), 97 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 8daaa6ad9..a258821f0 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 @@ -20,8 +20,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; -import org.eclipse.jgit.internal.diffmergetool.CommandLineDiffTool; +import org.eclipse.jgit.internal.diffmergetool.DiffTools; +import org.eclipse.jgit.internal.diffmergetool.ExternalDiffTool; import org.eclipse.jgit.lib.StoredConfig; import org.junit.Before; import org.junit.Test; @@ -149,20 +151,38 @@ expectedOutput, runAndCaptureUsingInitRaw(DIFF_TOOL, @Test public void testToolHelp() throws Exception { - CommandLineDiffTool[] defaultTools = CommandLineDiffTool.values(); List expectedOutput = new ArrayList<>(); + + DiffTools diffTools = new DiffTools(db); + Map predefinedTools = diffTools + .getPredefinedTools(true); + List availableTools = new ArrayList<>(); + List notAvailableTools = new ArrayList<>(); + for (ExternalDiffTool tool : predefinedTools.values()) { + if (tool.isAvailable()) { + availableTools.add(tool); + } else { + notAvailableTools.add(tool); + } + } + expectedOutput.add( "'git difftool --tool=' may be set to one of the following:"); - for (CommandLineDiffTool defaultTool : defaultTools) { - String toolName = defaultTool.name(); + for (ExternalDiffTool tool : availableTools) { + String toolName = tool.getName(); expectedOutput.add(toolName); } String customToolHelpLine = TOOL_NAME + "." + CONFIG_KEY_CMD + " " + getEchoCommand(); expectedOutput.add("user-defined:"); expectedOutput.add(customToolHelpLine); + expectedOutput.add( + "The following tools are valid, but not currently available:"); + for (ExternalDiffTool tool : notAvailableTools) { + String toolName = tool.getName(); + expectedOutput.add(toolName); + } String[] userDefinedToolsHelp = { - "The following tools are valid, but not currently available:", "Some of the tools listed above only work in a windowed", "environment. If run in a terminal-only session, they will fail.", }; 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 2e50f0908..e9d559e70 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 @@ -19,8 +19,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; -import org.eclipse.jgit.internal.diffmergetool.CommandLineMergeTool; +import org.eclipse.jgit.internal.diffmergetool.ExternalMergeTool; +import org.eclipse.jgit.internal.diffmergetool.MergeTools; import org.eclipse.jgit.lib.StoredConfig; import org.junit.Before; import org.junit.Test; @@ -157,20 +159,38 @@ expectedOutput, runAndCaptureUsingInitRaw(MERGE_TOOL, @Test public void testToolHelp() throws Exception { - CommandLineMergeTool[] defaultTools = CommandLineMergeTool.values(); List expectedOutput = new ArrayList<>(); + + MergeTools diffTools = new MergeTools(db); + Map predefinedTools = diffTools + .getPredefinedTools(true); + List availableTools = new ArrayList<>(); + List notAvailableTools = new ArrayList<>(); + for (ExternalMergeTool tool : predefinedTools.values()) { + if (tool.isAvailable()) { + availableTools.add(tool); + } else { + notAvailableTools.add(tool); + } + } + expectedOutput.add( "'git mergetool --tool=' may be set to one of the following:"); - for (CommandLineMergeTool defaultTool : defaultTools) { - String toolName = defaultTool.name(); + for (ExternalMergeTool tool : availableTools) { + String toolName = tool.getName(); expectedOutput.add(toolName); } String customToolHelpLine = TOOL_NAME + "." + CONFIG_KEY_CMD + " " + getEchoCommand(); expectedOutput.add("user-defined:"); expectedOutput.add(customToolHelpLine); + expectedOutput.add( + "The following tools are valid, but not currently available:"); + for (ExternalMergeTool tool : notAvailableTools) { + String toolName = tool.getName(); + expectedOutput.add(toolName); + } String[] userDefinedToolsHelp = { - "The following tools are valid, but not currently available:", "Some of the tools listed above only work in a windowed", "environment. If run in a terminal-only session, they will fail.", }; expectedOutput.addAll(Arrays.asList(userDefinedToolsHelp)); diff --git a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties index 674185df2..b14531a1b 100644 --- a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties +++ b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties @@ -61,6 +61,8 @@ deletedRemoteBranch=Deleted remote branch {0} diffToolHelpSetToFollowing=''git difftool --tool='' may be set to one of the following:\n{0}\n\tuser-defined:\n{1}\nThe following tools are valid, but not currently available:\n{2}\nSome of the tools listed above only work in a windowed\nenvironment. If run in a terminal-only session, they will fail. diffToolLaunch=Viewing ({0}/{1}): ''{2}''\nLaunch ''{3}'' [Y/n]? diffToolDied=external diff died, stopping at path ''{0}'' due to exception: {1} +diffToolPromptToolName=This message is displayed because 'diff.tool' is not configured.\nSee 'git difftool --tool-help' or 'git help config' for more details.\n'git difftool' will now attempt to use one of the following tools:\n{0}\n +diffToolUnknownToolName=Unknown diff tool '{0}' doesNotExist={0} does not exist dontOverwriteLocalChanges=error: Your local changes to the following file would be overwritten by merge: everythingUpToDate=Everything up-to-date @@ -107,6 +109,8 @@ mergeToolDeletedConflictByThem= {local}: modified file\n {remote}: deleted mergeToolContinueUnresolvedPaths=\nContinue merging other unresolved paths [y/n]? mergeToolWasMergeSuccessfull=Was the merge successful [y/n]? mergeToolDeletedMergeDecision=Use (m)odified or (d)eleted file, or (a)bort? +mergeToolPromptToolName=This message is displayed because 'merge.tool' is not configured.\nSee 'git mergetool --tool-help' or 'git help config' for more details.\n'git mergetool' will now attempt to use one of the following tools:\n{0}\n +mergeToolUnknownToolName=Unknown merge tool '{0}' mergeFailed=Automatic merge failed; fix conflicts and then commit the result mergeCheckoutFailed=Please, commit your changes or stash them before you can merge. mergeMadeBy=Merge made by the ''{0}'' strategy. diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java index 74d91cd3d..e5a3c53e3 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java @@ -23,29 +23,30 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; + import org.eclipse.jgit.diff.ContentSource; import org.eclipse.jgit.diff.ContentSource.Pair; import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.diff.DiffEntry.Side; -import org.eclipse.jgit.internal.diffmergetool.ToolException; -import org.eclipse.jgit.internal.diffmergetool.DiffTools; -import org.eclipse.jgit.internal.diffmergetool.FileElement; -import org.eclipse.jgit.internal.diffmergetool.ExternalDiffTool; import org.eclipse.jgit.diff.DiffFormatter; import org.eclipse.jgit.dircache.DirCacheCheckout; -import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.dircache.DirCacheCheckout.CheckoutMetadata; +import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.errors.AmbiguousObjectException; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.NoWorkTreeException; import org.eclipse.jgit.errors.RevisionSyntaxException; +import org.eclipse.jgit.internal.diffmergetool.DiffTools; +import org.eclipse.jgit.internal.diffmergetool.ExternalDiffTool; +import org.eclipse.jgit.internal.diffmergetool.FileElement; +import org.eclipse.jgit.internal.diffmergetool.ToolException; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.CoreConfig.EolStreamType; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.TextProgressMonitor; -import org.eclipse.jgit.lib.CoreConfig.EolStreamType; import org.eclipse.jgit.lib.internal.BooleanTriState; import org.eclipse.jgit.pgm.internal.CLIText; import org.eclipse.jgit.pgm.opt.PathTreeFilterHandler; @@ -58,8 +59,8 @@ import org.eclipse.jgit.treewalk.WorkingTreeOptions; import org.eclipse.jgit.treewalk.filter.PathFilterGroup; import org.eclipse.jgit.treewalk.filter.TreeFilter; -import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.util.FS.ExecutionResult; +import org.eclipse.jgit.util.StringUtils; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; @@ -144,19 +145,14 @@ protected void run() { if (prompt != BooleanTriState.UNSET) { showPrompt = prompt == BooleanTriState.TRUE; } - String toolNamePrompt = toolName; - if (showPrompt) { - if (StringUtils.isEmptyOrNull(toolNamePrompt)) { - toolNamePrompt = diffTools.getDefaultToolName(gui); - } - } + // get passed or default tool name + String toolNameToUse = promptToolName(); // get the changed files List files = getFiles(); if (files.size() > 0) { - compare(files, showPrompt, toolNamePrompt); + compare(files, showPrompt, toolNameToUse); } } - outw.flush(); } catch (RevisionSyntaxException | IOException e) { throw die(e.getMessage(), e); } finally { @@ -164,8 +160,32 @@ protected void run() { } } + private String promptToolName() throws IOException { + String toolNameToUse = toolName; + if (StringUtils.isEmptyOrNull(toolNameToUse)) { + toolNameToUse = diffTools.getDefaultToolName(gui); + } + if (StringUtils.isEmptyOrNull(toolNameToUse)) { + Map predefTools = diffTools + .getPredefinedTools(false); + StringBuilder toolNames = new StringBuilder(); + for (String name : predefTools.keySet()) { + toolNames.append(name + " "); //$NON-NLS-1$ + } + outw.println(MessageFormat.format( + CLIText.get().diffToolPromptToolName, toolNames)); + outw.flush(); + toolNameToUse = diffTools.getFirstAvailableTool(); + } + if (StringUtils.isEmptyOrNull(toolNameToUse)) { + throw new IOException(MessageFormat + .format(CLIText.get().diffToolUnknownToolName, toolName)); + } + return toolNameToUse; + } + private void compare(List files, boolean showPrompt, - String toolNamePrompt) throws IOException { + String toolNameToUse) throws IOException { ContentSource.Pair sourcePair = new ContentSource.Pair(source(oldTree), source(newTree)); try { @@ -179,7 +199,7 @@ private void compare(List files, boolean showPrompt, boolean launchCompare = true; if (showPrompt) { launchCompare = isLaunchCompare(fileIndex + 1, files.size(), - mergedFilePath, toolNamePrompt); + mergedFilePath, toolNameToUse); } if (launchCompare) { try { @@ -195,17 +215,21 @@ private void compare(List files, boolean showPrompt, // to jgit / java runtime ? // int rc =... ExecutionResult result = diffTools.compare(local, - remote, merged, toolName, prompt, gui, + remote, merged, toolNameToUse, prompt, gui, trustExitCode); outw.println(new String(result.getStdout().toByteArray())); + outw.flush(); errw.println( new String(result.getStderr().toByteArray())); + errw.flush(); } catch (ToolException e) { outw.println(e.getResultStdout()); outw.flush(); errw.println(e.getMessage()); + errw.flush(); throw die(MessageFormat.format( - CLIText.get().diffToolDied, mergedFilePath), e); + CLIText.get().diffToolDied, mergedFilePath, e), + e); } } else { break; @@ -232,16 +256,17 @@ private boolean isLaunchCompare(int fileIndex, int fileCount, } return launchCompare; } - private void showToolHelp() throws IOException { + Map predefTools = diffTools + .getPredefinedTools(true); StringBuilder availableToolNames = new StringBuilder(); - for (String name : diffTools.getAvailableTools().keySet()) { - availableToolNames.append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ - } StringBuilder notAvailableToolNames = new StringBuilder(); - for (String name : diffTools.getNotAvailableTools().keySet()) { - notAvailableToolNames - .append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ + for (String name : predefTools.keySet()) { + if (predefTools.get(name).isAvailable()) { + availableToolNames.append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ + } else { + notAvailableToolNames.append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ + } } StringBuilder userToolNames = new StringBuilder(); Map userTools = diffTools diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java index 971277075..f5884c44d 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java @@ -48,6 +48,7 @@ import org.eclipse.jgit.treewalk.filter.PathFilterGroup; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.internal.BooleanTriState; import org.eclipse.jgit.lib.CoreConfig.EolStreamType; @@ -121,14 +122,11 @@ protected void run() { showPrompt = prompt == BooleanTriState.TRUE; } // get passed or default tool name - String toolNameSelected = toolName; - if ((toolNameSelected == null) || toolNameSelected.isEmpty()) { - toolNameSelected = mergeTools.getDefaultToolName(gui); - } + String toolNameToUse = promptToolName(); // get the changed files Map files = getFiles(); if (files.size() > 0) { - merge(files, showPrompt, toolNameSelected); + merge(files, showPrompt, toolNameToUse); } else { outw.println(CLIText.get().mergeToolNoFiles); } @@ -139,6 +137,30 @@ protected void run() { } } + private String promptToolName() throws IOException { + String toolNameToUse = toolName; + if (StringUtils.isEmptyOrNull(toolNameToUse)) { + toolNameToUse = mergeTools.getDefaultToolName(gui); + } + if (StringUtils.isEmptyOrNull(toolNameToUse)) { + Map predefTools = mergeTools + .getPredefinedTools(false); + StringBuilder toolNames = new StringBuilder(); + for (String name : predefTools.keySet()) { + toolNames.append(name + " "); //$NON-NLS-1$ + } + outw.println(MessageFormat + .format(CLIText.get().mergeToolPromptToolName, toolNames)); + outw.flush(); + toolNameToUse = mergeTools.getFirstAvailableTool(); + } + if (StringUtils.isEmptyOrNull(toolNameToUse)) { + throw new IOException(MessageFormat + .format(CLIText.get().mergeToolUnknownToolName, toolName)); + } + return toolNameToUse; + } + private void merge(Map files, boolean showPrompt, String toolNamePrompt) throws Exception { // sort file names @@ -420,14 +442,16 @@ private int getDeletedMergeDecision() throws IOException { } private void showToolHelp() throws IOException { + Map predefTools = mergeTools + .getPredefinedTools(true); StringBuilder availableToolNames = new StringBuilder(); - for (String name : mergeTools.getAvailableTools().keySet()) { - availableToolNames.append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ - } StringBuilder notAvailableToolNames = new StringBuilder(); - for (String name : mergeTools.getNotAvailableTools().keySet()) { - notAvailableToolNames - .append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ + for (String name : predefTools.keySet()) { + if (predefTools.get(name).isAvailable()) { + availableToolNames.append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ + } else { + notAvailableToolNames.append(MessageFormat.format("\t\t{0}\n", name)); //$NON-NLS-1$ + } } StringBuilder userToolNames = new StringBuilder(); Map userTools = mergeTools diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java index 989e649b7..e06f150e5 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java @@ -139,6 +139,8 @@ public static String fatalError(String message) { /***/ public String diffToolHelpSetToFollowing; /***/ public String diffToolLaunch; /***/ public String diffToolDied; + /***/ public String diffToolPromptToolName; + /***/ public String diffToolUnknownToolName; /***/ public String doesNotExist; /***/ public String dontOverwriteLocalChanges; /***/ public String everythingUpToDate; @@ -185,6 +187,8 @@ public static String fatalError(String message) { /***/ public String mergeToolContinueUnresolvedPaths; /***/ public String mergeToolWasMergeSuccessfull; /***/ public String mergeToolDeletedMergeDecision; + /***/ public String mergeToolPromptToolName; + /***/ public String mergeToolUnknownToolName; /***/ public String mergeFailed; /***/ public String mergeCheckoutFailed; /***/ public String mergeMadeBy; diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalDiffToolTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalDiffToolTest.java index 4fd55c6ca..1b501c25b 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalDiffToolTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalDiffToolTest.java @@ -97,7 +97,7 @@ public void testToolNames() { @Test public void testAllTools() { DiffTools manager = new DiffTools(db); - Set actualToolNames = manager.getAvailableTools().keySet(); + Set actualToolNames = manager.getPredefinedTools(true).keySet(); Set expectedToolNames = new LinkedHashSet<>(); CommandLineDiffTool[] defaultTools = CommandLineDiffTool.values(); for (CommandLineDiffTool defaultTool : defaultTools) { @@ -152,15 +152,6 @@ public void testUserDefinedTools() { actualToolNames); } - @Test - public void testNotAvailableTools() { - DiffTools manager = new DiffTools(db); - Set actualToolNames = manager.getNotAvailableTools().keySet(); - Set expectedToolNames = Collections.emptySet(); - assertEquals("Incorrect set of not available external diff tools", - expectedToolNames, actualToolNames); - } - @Test public void testCompare() throws ToolException { String toolName = "customTool"; @@ -239,7 +230,7 @@ public void testOverridePreDefinedToolPath() { DiffTools manager = new DiffTools(db); Map availableTools = manager - .getAvailableTools(); + .getPredefinedTools(true); ExternalDiffTool externalDiffTool = availableTools .get(overridenToolName); String actualDiffToolPath = externalDiffTool.getPath(); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalMergeToolTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalMergeToolTest.java index 50576682e..305f2b470 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalMergeToolTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalMergeToolTest.java @@ -9,14 +9,14 @@ */ package org.eclipse.jgit.internal.diffmergetool; -import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_MERGETOOL_SECTION; -import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_MERGE_SECTION; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_CMD; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_GUITOOL; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PATH; 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_KEY_TRUST_EXIT_CODE; +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.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -95,7 +95,7 @@ public void testToolNames() { @Test public void testAllTools() { MergeTools manager = new MergeTools(db); - Set actualToolNames = manager.getAvailableTools().keySet(); + Set actualToolNames = manager.getPredefinedTools(true).keySet(); Set expectedToolNames = new LinkedHashSet<>(); CommandLineMergeTool[] defaultTools = CommandLineMergeTool.values(); for (CommandLineMergeTool defaultTool : defaultTools) { @@ -150,15 +150,6 @@ public void testUserDefinedTools() { actualToolNames); } - @Test - public void testNotAvailableTools() { - MergeTools manager = new MergeTools(db); - Set actualToolNames = manager.getNotAvailableTools().keySet(); - Set expectedToolNames = Collections.emptySet(); - assertEquals("Incorrect set of not available external merge tools", - expectedToolNames, actualToolNames); - } - @Test public void testCompare() throws ToolException { String toolName = "customTool"; @@ -236,7 +227,7 @@ public void testOverridePreDefinedToolPath() { MergeTools manager = new MergeTools(db); Map availableTools = manager - .getAvailableTools(); + .getPredefinedTools(true); ExternalMergeTool externalMergeTool = availableTools .get(overridenToolName); String actualMergeToolPath = externalMergeTool.getPath(); 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 ad79fe8fc..668adeab6 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.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 ExecutionResult run(String command, File workingDir, } } + /** + * @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 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 @@ private String[] createCommandArray(String command) 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 509515c37..00dec3271 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: http://vimdoc.sourceforge.net/htmldoc/diff.html */ - gvimdiff("gviewdiff", "\"$LOCAL\" \"$REMOTE\""), + gvimdiff("gvimdiff", "\"$LOCAL\" \"$REMOTE\""), /** * See: http://vimdoc.sourceforge.net/htmldoc/diff.html @@ -160,7 +160,7 @@ public enum CommandLineDiffTool { * See: http://vimdoc.sourceforge.net/htmldoc/diff.html */ - vimdiff("viewdiff", gvimdiff), + vimdiff("vimdiff", gvimdiff), /** * See: http://vimdoc.sourceforge.net/htmldoc/diff.html 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 1dcc523bf..6c1a780a3 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 Map getUserDefinedTools() { } /** - * @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 getAvailableTools() { + public Map 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 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 @@ private ExternalDiffTool guessTool(String toolName, BooleanTriState gui) 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 f2d7e828c..e01b892a5 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 3efb90c49..e10607d2f 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 static Map prepareEnvironment(Repository repo, FileElement mergedFile, FileElement baseFile) throws IOException { Map 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 9a2a8304e..d91d57f1a 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.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 Map getUserDefinedTools() { } /** - * @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 getAvailableTools() { + public Map 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 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 boolean isInteractive() { 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 092cb605b..e1169a2d6 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 void setPath(String path) { */ @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 2c64c1666..7b28d3282 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 String getCommand() { */ @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 27f7d12e6..7cc5bb50d 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 boolean isCommandExecutionError() { * @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 String getResultStderr() { * @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 012296eb3..eb72d01cd 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 @@ */ public class UserDefinedDiffTool implements ExternalDiffTool { + private boolean available; + /** * the diff tool name */ @@ -98,6 +100,23 @@ public String getCommand() { return cmd; } + /** + * @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..path}.