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 e7bf48417..e2ff18927 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 @@ -9,7 +9,13 @@ */ package org.eclipse.jgit.pgm; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_DIFFTOOL_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_DIFF_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_CMD; +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.assertEquals; +import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.Arrays; @@ -19,6 +25,7 @@ import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.internal.diffmergetool.CommandLineDiffTool; import org.eclipse.jgit.lib.CLIRepositoryTestCase; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.pgm.opt.CmdLineParser; import org.eclipse.jgit.pgm.opt.SubcommandHandler; import org.eclipse.jgit.revwalk.RevCommit; @@ -60,6 +67,7 @@ private String[] runAndCaptureUsingInitRaw(String... args) return result.outLines().toArray(new String[0]); } + private static final String TOOL_NAME = "some_tool"; private Git git; @Override @@ -68,6 +76,15 @@ public void setUp() throws Exception { super.setUp(); git = new Git(db); git.commit().setMessage("initial commit").call(); + configureEchoTool(TOOL_NAME); + } + + @Test(expected = Die.class) + public void testNotDefinedTool() throws Exception { + createUnstagedChanges(); + + runAndCaptureUsingInitRaw("difftool", "--tool", "undefined"); + fail("Expected exception when trying to run undefined tool"); } @Test @@ -85,7 +102,7 @@ public void testTool() throws Exception { assertArrayOfLinesEquals("Incorrect output for option: " + option, expectedOutput, runAndCaptureUsingInitRaw("difftool", option, - "some_tool")); + TOOL_NAME)); } } @@ -100,7 +117,7 @@ public void testToolTrustExitCode() throws Exception { for (String option : options) { assertArrayOfLinesEquals("Incorrect output for option: " + option, expectedOutput, runAndCaptureUsingInitRaw("difftool", - "--trust-exit-code", option, "some_tool")); + "--trust-exit-code", option, TOOL_NAME)); } } @@ -116,7 +133,7 @@ public void testToolNoGuiNoPromptNoTrustExitcode() throws Exception { assertArrayOfLinesEquals("Incorrect output for option: " + option, expectedOutput, runAndCaptureUsingInitRaw("difftool", "--no-gui", "--no-prompt", "--no-trust-exit-code", - option, "some_tool")); + option, TOOL_NAME)); } } @@ -131,7 +148,7 @@ public void testToolCached() throws Exception { for (String option : options) { assertArrayOfLinesEquals("Incorrect output for option: " + option, expectedOutput, runAndCaptureUsingInitRaw("difftool", - option, "--tool", "some_tool")); + option, "--tool", TOOL_NAME)); } } @@ -144,8 +161,11 @@ public void testToolHelp() throws Exception { String toolName = defaultTool.name(); expectedOutput.add(toolName); } + String customToolHelpLine = TOOL_NAME + "." + CONFIG_KEY_CMD + " " + + getEchoCommand(); + expectedOutput.add("user-defined:"); + expectedOutput.add(customToolHelpLine); String[] userDefinedToolsHelp = { - "user-defined:", "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.", @@ -157,6 +177,25 @@ public void testToolHelp() throws Exception { expectedOutput.toArray(new String[0]), runAndCaptureUsingInitRaw("difftool", option)); } + private void configureEchoTool(String toolName) { + 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, + toolName); + + String command = getEchoCommand(); + + config.setString(CONFIG_DIFFTOOL_SECTION, toolName, CONFIG_KEY_CMD, + command); + /* + * prevent prompts as we are running in tests and there is no user to + * interact with on the command line + */ + config.setString(CONFIG_DIFFTOOL_SECTION, toolName, CONFIG_KEY_PROMPT, + String.valueOf(false)); + } + private RevCommit createUnstagedChanges() throws Exception { writeTrashFile("a", "Hello world a"); writeTrashFile("b", "Hello world b"); @@ -188,11 +227,7 @@ private String[] getExpectedDiffToolOutput(List changes) { for (int i = 0; i < changes.size(); ++i) { DiffEntry change = changes.get(i); String newPath = change.getNewPath(); - String oldPath = change.getOldPath(); - String newIdName = change.getNewId().name(); - String oldIdName = change.getOldId().name(); - String expectedLine = "M\t" + newPath + " (" + newIdName + ")" - + "\t" + oldPath + " (" + oldIdName + ")"; + String expectedLine = newPath; expectedToolOutput[i] = expectedLine; } return expectedToolOutput; @@ -202,4 +237,12 @@ private static void assertArrayOfLinesEquals(String failMessage, String[] expected, String[] actual) { assertEquals(failMessage, toString(expected), toString(actual)); } + + 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/resources/org/eclipse/jgit/pgm/internal/CLIText.properties b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties index fda0bf6ff..3653b9d8f 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 @@ -60,7 +60,7 @@ deletedBranch=Deleted branch {0} 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 {0} +diffToolDied=external diff died, stopping at path ''{0}'' due to exception: {1} doesNotExist={0} does not exist dontOverwriteLocalChanges=error: Your local changes to the following file would be overwritten by merge: everythingUpToDate=Everything up-to-date 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 128881779..2f7417745 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 @@ -21,8 +21,10 @@ 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.diff.DiffFormatter; import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.errors.AmbiguousObjectException; @@ -30,8 +32,11 @@ 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.ObjectId; import org.eclipse.jgit.lib.ObjectReader; +import org.eclipse.jgit.lib.ObjectStream; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.TextProgressMonitor; import org.eclipse.jgit.lib.internal.BooleanTriState; @@ -40,8 +45,10 @@ import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.eclipse.jgit.treewalk.FileTreeIterator; +import org.eclipse.jgit.treewalk.WorkingTreeIterator; import org.eclipse.jgit.treewalk.filter.TreeFilter; import org.eclipse.jgit.util.StringUtils; +import org.eclipse.jgit.util.FS.ExecutionResult; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; @@ -145,40 +152,54 @@ protected void run() { private void compare(List files, boolean showPrompt, String toolNamePrompt) throws IOException { - for (int fileIndex = 0; fileIndex < files.size(); fileIndex++) { - DiffEntry ent = files.get(fileIndex); - String mergedFilePath = ent.getNewPath(); - if (mergedFilePath.equals(DiffEntry.DEV_NULL)) { - mergedFilePath = ent.getOldPath(); - } - // check if user wants to launch compare - boolean launchCompare = true; - if (showPrompt) { - launchCompare = isLaunchCompare(fileIndex + 1, files.size(), - mergedFilePath, toolNamePrompt); - } - if (launchCompare) { - switch (ent.getChangeType()) { - case MODIFY: - outw.println("M\t" + ent.getNewPath() //$NON-NLS-1$ - + " (" + ent.getNewId().name() + ")" //$NON-NLS-1$ //$NON-NLS-2$ - + "\t" + ent.getOldPath() //$NON-NLS-1$ - + " (" + ent.getOldId().name() + ")"); //$NON-NLS-1$ //$NON-NLS-2$ - int ret = diffTools.compare(ent.getNewPath(), - ent.getOldPath(), ent.getNewId().name(), - ent.getOldId().name(), toolName, prompt, gui, - trustExitCode); - if (ret != 0) { + ContentSource.Pair sourcePair = new ContentSource.Pair(source(oldTree), + source(newTree)); + try { + for (int fileIndex = 0; fileIndex < files.size(); fileIndex++) { + DiffEntry ent = files.get(fileIndex); + String mergedFilePath = ent.getNewPath(); + if (mergedFilePath.equals(DiffEntry.DEV_NULL)) { + mergedFilePath = ent.getOldPath(); + } + FileElement local = new FileElement(ent.getOldPath(), + ent.getOldId().name(), + getObjectStream(sourcePair, Side.OLD, ent)); + FileElement remote = new FileElement(ent.getNewPath(), + ent.getNewId().name(), + getObjectStream(sourcePair, Side.NEW, ent)); + // check if user wants to launch compare + boolean launchCompare = true; + if (showPrompt) { + launchCompare = isLaunchCompare(fileIndex + 1, files.size(), + mergedFilePath, toolNamePrompt); + } + if (launchCompare) { + try { + // TODO: check how to return the exit-code of + // the + // tool + // to + // jgit / java runtime ? + // int rc =... + ExecutionResult result = diffTools.compare(db, local, + remote, mergedFilePath, + toolName, prompt, gui, trustExitCode); + outw.println(new String(result.getStdout().toByteArray())); + errw.println( + new String(result.getStderr().toByteArray())); + } catch (ToolException e) { + outw.println(e.getResultStdout()); + outw.flush(); + errw.println(e.getMessage()); throw die(MessageFormat.format( - CLIText.get().diffToolDied, mergedFilePath)); + CLIText.get().diffToolDied, mergedFilePath, e)); } - break; - default: + } else { break; } - } else { - break; } + } finally { + sourcePair.close(); } } @@ -254,4 +275,23 @@ private List getFiles() return files; } + private ObjectStream getObjectStream(Pair pair, Side side, DiffEntry ent) { + ObjectStream stream = null; + if (!pair.isWorkingTreeSource(side)) { + try { + stream = pair.open(side, ent).openStream(); + } catch (Exception e) { + stream = null; + } + } + return stream; + } + + private ContentSource source(AbstractTreeIterator iterator) { + if (iterator instanceof WorkingTreeIterator) { + return ContentSource.create((WorkingTreeIterator) iterator); + } + return ContentSource.create(db.newObjectReader()); + } + } 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 c9ebec763..ebc67c81c 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 @@ -10,13 +10,17 @@ package org.eclipse.jgit.internal.diffmergetool; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_DIFFTOOL_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_DIFF_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.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.Collections; import java.util.LinkedHashSet; @@ -25,6 +29,7 @@ import org.eclipse.jgit.lib.internal.BooleanTriState; import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.util.FS.ExecutionResult; import org.junit.Test; /** @@ -32,6 +37,54 @@ */ public class ExternalDiffToolTest extends ExternalToolTestCase { + @Test(expected = ToolException.class) + public void testUserToolWithError() throws Exception { + String toolName = "customTool"; + + int errorReturnCode = 1; + String command = "exit " + errorReturnCode; + + FileBasedConfig config = db.getConfig(); + config.setString(CONFIG_DIFFTOOL_SECTION, toolName, CONFIG_KEY_CMD, + command); + + DiffTools manager = new DiffTools(db); + + BooleanTriState prompt = BooleanTriState.UNSET; + BooleanTriState gui = BooleanTriState.UNSET; + BooleanTriState trustExitCode = BooleanTriState.TRUE; + + manager.compare(db, local, remote, merged.getPath(), toolName, prompt, + gui, trustExitCode); + + fail("Expected exception to be thrown due to external tool exiting with error code: " + + errorReturnCode); + } + + @Test(expected = ToolException.class) + public void testUserToolWithCommandNotFoundError() throws Exception { + String toolName = "customTool"; + + int errorReturnCode = 127; // command not found + String command = "exit " + errorReturnCode; + + FileBasedConfig config = db.getConfig(); + config.setString(CONFIG_DIFFTOOL_SECTION, toolName, CONFIG_KEY_CMD, + command); + + DiffTools manager = new DiffTools(db); + + BooleanTriState prompt = BooleanTriState.UNSET; + BooleanTriState gui = BooleanTriState.UNSET; + BooleanTriState trustExitCode = BooleanTriState.FALSE; + + manager.compare(db, local, remote, merged.getPath(), toolName, prompt, + gui, trustExitCode); + + fail("Expected exception to be thrown due to external tool exiting with error code: " + + errorReturnCode); + } + @Test public void testToolNames() { DiffTools manager = new DiffTools(db); @@ -86,11 +139,11 @@ public void testUserDefinedTools() { config.setString(CONFIG_DIFFTOOL_SECTION, customToolname, CONFIG_KEY_PATH, "/usr/bin/echo"); config.setString(CONFIG_DIFFTOOL_SECTION, customToolname, - CONFIG_KEY_PROMPT, "--no-prompt"); + CONFIG_KEY_PROMPT, String.valueOf(false)); config.setString(CONFIG_DIFFTOOL_SECTION, customToolname, - CONFIG_KEY_GUITOOL, "--no-gui"); + CONFIG_KEY_GUITOOL, String.valueOf(false)); config.setString(CONFIG_DIFFTOOL_SECTION, customToolname, - CONFIG_KEY_TRUST_EXIT_CODE, "--no-trust-exit-code"); + CONFIG_KEY_TRUST_EXIT_CODE, String.valueOf(false)); DiffTools manager = new DiffTools(db); Set actualToolNames = manager.getUserDefinedTools().keySet(); Set expectedToolNames = new LinkedHashSet<>(); @@ -109,38 +162,50 @@ public void testNotAvailableTools() { } @Test - public void testCompare() { - DiffTools manager = new DiffTools(db); + public void testCompare() throws ToolException { + String toolName = "customTool"; + + FileBasedConfig config = db.getConfig(); + // the default diff tool is configured without a subsection + String subsection = null; + config.setString(CONFIG_DIFF_SECTION, subsection, CONFIG_KEY_TOOL, + toolName); + + String command = getEchoCommand(); + + config.setString(CONFIG_DIFFTOOL_SECTION, toolName, CONFIG_KEY_CMD, + command); - String newPath = ""; - String oldPath = ""; - String newId = ""; - String oldId = ""; - String toolName = ""; BooleanTriState prompt = BooleanTriState.UNSET; BooleanTriState gui = BooleanTriState.UNSET; BooleanTriState trustExitCode = BooleanTriState.UNSET; + DiffTools manager = new DiffTools(db); + int expectedCompareResult = 0; - int compareResult = manager.compare(newPath, oldPath, newId, oldId, - toolName, prompt, gui, trustExitCode); + ExecutionResult compareResult = manager.compare(db, local, remote, + merged.getPath(), toolName, prompt, gui, trustExitCode); assertEquals("Incorrect compare result for external diff tool", - expectedCompareResult, compareResult); + expectedCompareResult, compareResult.getRc()); } @Test public void testDefaultTool() throws Exception { + String toolName = "customTool"; + String guiToolName = "customGuiTool"; + FileBasedConfig config = db.getConfig(); // the default diff tool is configured without a subsection String subsection = null; - config.setString("diff", subsection, "tool", "customTool"); + config.setString(CONFIG_DIFF_SECTION, subsection, CONFIG_KEY_TOOL, + toolName); DiffTools manager = new DiffTools(db); BooleanTriState gui = BooleanTriState.UNSET; String defaultToolName = manager.getDefaultToolName(gui); assertEquals( "Expected configured difftool to be the default external diff tool", - "my_default_toolname", defaultToolName); + toolName, defaultToolName); gui = BooleanTriState.TRUE; String defaultGuiToolName = manager.getDefaultToolName(gui); @@ -148,11 +213,63 @@ public void testDefaultTool() throws Exception { "Expected configured difftool to be the default external diff tool", "my_gui_tool", defaultGuiToolName); - config.setString("diff", subsection, "guitool", "customGuiTool"); + config.setString(CONFIG_DIFF_SECTION, subsection, CONFIG_KEY_GUITOOL, + guiToolName); manager = new DiffTools(db); defaultGuiToolName = manager.getDefaultToolName(gui); assertEquals( "Expected configured difftool to be the default external diff guitool", "my_gui_tool", defaultGuiToolName); } + + @Test + public void testOverridePreDefinedToolPath() { + String newToolPath = "/tmp/path/"; + + CommandLineDiffTool[] defaultTools = CommandLineDiffTool.values(); + assertTrue("Expected to find pre-defined external diff tools", + defaultTools.length > 0); + + CommandLineDiffTool overridenTool = defaultTools[0]; + String overridenToolName = overridenTool.name(); + String overridenToolPath = newToolPath + overridenToolName; + FileBasedConfig config = db.getConfig(); + config.setString(CONFIG_DIFFTOOL_SECTION, overridenToolName, + CONFIG_KEY_PATH, overridenToolPath); + + DiffTools manager = new DiffTools(db); + Map availableTools = manager + .getAvailableTools(); + ExternalDiffTool externalDiffTool = availableTools + .get(overridenToolName); + String actualDiffToolPath = externalDiffTool.getPath(); + assertEquals( + "Expected pre-defined external diff tool to have overriden path", + overridenToolPath, actualDiffToolPath); + String expectedDiffToolCommand = overridenToolPath + " " + + overridenTool.getParameters(); + String actualDiffToolCommand = externalDiffTool.getCommand(); + assertEquals( + "Expected pre-defined external diff tool to have overriden command", + expectedDiffToolCommand, actualDiffToolCommand); + } + + @Test(expected = ToolException.class) + public void testUndefinedTool() throws Exception { + DiffTools manager = new DiffTools(db); + + String toolName = "undefined"; + BooleanTriState prompt = BooleanTriState.UNSET; + BooleanTriState gui = BooleanTriState.UNSET; + BooleanTriState trustExitCode = BooleanTriState.UNSET; + + manager.compare(db, local, remote, merged.getPath(), toolName, prompt, + gui, trustExitCode); + fail("Expected exception to be thrown due to not defined external diff tool"); + } + + private String getEchoCommand() { + return "(echo \"$LOCAL\" \"$REMOTE\") > " + + commandResult.getAbsolutePath(); + } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalToolTestCase.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalToolTestCase.java index 0cc12978a..6757eb463 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalToolTestCase.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/diffmergetool/ExternalToolTestCase.java @@ -36,6 +36,14 @@ public abstract class ExternalToolTestCase extends RepositoryTestCase { protected File commandResult; + protected FileElement local; + + protected FileElement remote; + + protected FileElement merged; + + protected FileElement base; + @Before @Override public void setUp() throws Exception { @@ -51,6 +59,11 @@ public void setUp() throws Exception { baseFile.deleteOnExit(); commandResult = writeTrashFile("commandResult.txt", ""); commandResult.deleteOnExit(); + + local = new FileElement(localFile.getAbsolutePath(), "LOCAL"); + remote = new FileElement(remoteFile.getAbsolutePath(), "REMOTE"); + merged = new FileElement(mergedFile.getAbsolutePath(), "MERGED"); + base = new FileElement(baseFile.getAbsolutePath(), "BASE"); } @After diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java index 1a41df3d0..64ff19c9c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010, 2020 Google Inc. and others + * Copyright (C) 2010, 2021 Google Inc. 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 @@ -91,6 +91,29 @@ public static ContentSource create(WorkingTreeIterator iterator) { public abstract ObjectLoader open(String path, ObjectId id) throws IOException; + /** + * Closes the used resources like ObjectReader, TreeWalk etc. Default + * implementation does nothing. + * + * @since 6.2 + */ + public void close() { + // Do nothing + } + + /** + * Checks if the source is from "working tree", so it can be accessed as a + * file directly. + * + * @since 6.2 + * + * @return true if working tree source and false otherwise (loader must be + * used) + */ + public boolean isWorkingTreeSource() { + return false; + } + private static class ObjectReaderSource extends ContentSource { private final ObjectReader reader; @@ -111,6 +134,16 @@ public long size(String path, ObjectId id) throws IOException { public ObjectLoader open(String path, ObjectId id) throws IOException { return reader.open(id, Constants.OBJ_BLOB); } + + @Override + public void close() { + reader.close(); + } + + @Override + public boolean isWorkingTreeSource() { + return false; + } } private static class WorkingTreeSource extends ContentSource { @@ -194,6 +227,16 @@ private void seek(String path) throws IOException { throw new FileNotFoundException(path); } } + + @Override + public void close() { + tw.close(); + } + + @Override + public boolean isWorkingTreeSource() { + return true; + } } /** A pair of sources to access the old and new sides of a DiffEntry. */ @@ -261,5 +304,37 @@ public ObjectLoader open(DiffEntry.Side side, DiffEntry ent) throw new IllegalArgumentException(); } } + + /** + * Closes used resources. + * + * @since 6.2 + */ + public void close() { + oldSource.close(); + newSource.close(); + } + + /** + * Checks if source (side) is a "working tree". + * + * @since 6.2 + * + * @param side + * which side of the entry to read (OLD or NEW). + * @return is the source a "working tree" + * + */ + public boolean isWorkingTreeSource(DiffEntry.Side side) { + switch (side) { + case OLD: + return oldSource.isWorkingTreeSource(); + case NEW: + return newSource.isWorkingTreeSource(); + default: + throw new IllegalArgumentException(); + } + } + } } 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 new file mode 100644 index 000000000..0dde9b5f3 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java @@ -0,0 +1,182 @@ +/* + * Copyright (C) 2018-2021, Andre Bossert + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.diffmergetool; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.util.Arrays; +import java.util.Map; +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; + +/** + * Runs a command with help of FS. + */ +public class CommandExecutor { + + private FS fs; + + private boolean checkExitCode; + + private File commandFile; + + private boolean useMsys2; + + /** + * @param fs + * the file system + * @param checkExitCode + * should the exit code be checked for errors ? + */ + public CommandExecutor(FS fs, boolean checkExitCode) { + this.fs = fs; + this.checkExitCode = checkExitCode; + } + + /** + * @param command + * the command string + * @param workingDir + * the working directory + * @param env + * the environment + * @return the execution result + * @throws ToolException + * @throws InterruptedException + * @throws IOException + */ + public ExecutionResult run(String command, File workingDir, + Map env) + throws ToolException, IOException, InterruptedException { + String[] commandArray = createCommandArray(command); + try { + ProcessBuilder pb = fs.runInShell(commandArray[0], + Arrays.copyOfRange(commandArray, 1, commandArray.length)); + pb.directory(workingDir); + Map envp = pb.environment(); + if (env != null) { + envp.putAll(env); + } + ExecutionResult result = fs.execute(pb, null); + int rc = result.getRc(); + if ((rc != 0) && (checkExitCode + || isCommandExecutionError(rc))) { + throw new ToolException( + new String(result.getStderr().toByteArray()), result); + } + return result; + } finally { + deleteCommandArray(); + } + } + + private void deleteCommandArray() { + deleteCommandFile(); + } + + private String[] createCommandArray(String command) + throws ToolException, IOException { + String[] commandArray = null; + checkUseMsys2(command); + createCommandFile(command); + if (fs instanceof FS_POSIX) { + commandArray = new String[1]; + commandArray[0] = commandFile.getCanonicalPath(); + } else if (fs instanceof FS_Win32) { + if (useMsys2) { + commandArray = new String[3]; + commandArray[0] = "bash.exe"; //$NON-NLS-1$ + commandArray[1] = "-c"; //$NON-NLS-1$ + commandArray[2] = commandFile.getCanonicalPath().replace("\\", //$NON-NLS-1$ + "/"); //$NON-NLS-1$ + } else { + commandArray = new String[1]; + commandArray[0] = commandFile.getCanonicalPath(); + } + } else if (fs instanceof FS_Win32_Cygwin) { + commandArray = new String[1]; + commandArray[0] = commandFile.getCanonicalPath().replace("\\", "/"); //$NON-NLS-1$ //$NON-NLS-2$ + } else { + throw new ToolException( + "JGit: file system not supported: " + fs.toString()); //$NON-NLS-1$ + } + return commandArray; + } + + private void checkUseMsys2(String command) { + useMsys2 = false; + String useMsys2Str = System.getProperty("jgit.usemsys2bash"); //$NON-NLS-1$ + if (useMsys2Str != null && !useMsys2Str.isEmpty()) { + if (useMsys2Str.equalsIgnoreCase("auto")) { //$NON-NLS-1$ + useMsys2 = command.contains(".sh"); //$NON-NLS-1$ + } else { + useMsys2 = Boolean.parseBoolean(useMsys2Str); + } + } + } + + private void createCommandFile(String command) + throws ToolException, IOException { + String fileExtension = null; + if (useMsys2 || fs instanceof FS_POSIX + || fs instanceof FS_Win32_Cygwin) { + fileExtension = ".sh"; //$NON-NLS-1$ + } else if (fs instanceof FS_Win32) { + fileExtension = ".cmd"; //$NON-NLS-1$ + command = "@echo off" + System.lineSeparator() + command //$NON-NLS-1$ + + System.lineSeparator() + "exit /B %ERRORLEVEL%"; //$NON-NLS-1$ + } else { + throw new ToolException( + "JGit: file system not supported: " + fs.toString()); //$NON-NLS-1$ + } + commandFile = File.createTempFile(".__", //$NON-NLS-1$ + "__jgit_tool" + fileExtension); //$NON-NLS-1$ + try (OutputStream outStream = new FileOutputStream(commandFile)) { + byte[] strToBytes = command.getBytes(); + outStream.write(strToBytes); + outStream.close(); + } + commandFile.setExecutable(true); + } + + private void deleteCommandFile() { + if (commandFile != null && commandFile.exists()) { + commandFile.delete(); + } + } + + private boolean isCommandExecutionError(int rc) { + if (useMsys2 || fs instanceof FS_POSIX + || fs instanceof FS_Win32_Cygwin) { + // 126: permission for executing command denied + // 127: command not found + if ((rc == 126) || (rc == 127)) { + return true; + } + } + else if (fs instanceof FS_Win32) { + // 9009, 0x2331: Program is not recognized as an internal or + // external command, operable program or batch file. Indicates that + // command, application name or path has been misspelled when + // configuring the Action. + if (rc == 9009) { + return true; + } + } + return false; + } + +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffToolConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffToolConfig.java index 551f634f2..c8b04f90f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffToolConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/DiffToolConfig.java @@ -49,9 +49,10 @@ private DiffToolConfig(Config rc) { toolName = rc.getString(CONFIG_DIFF_SECTION, null, CONFIG_KEY_TOOL); guiToolName = rc.getString(CONFIG_DIFF_SECTION, null, CONFIG_KEY_GUITOOL); - prompt = rc.getBoolean(CONFIG_DIFFTOOL_SECTION, CONFIG_KEY_PROMPT, + prompt = rc.getBoolean(CONFIG_DIFFTOOL_SECTION, toolName, + CONFIG_KEY_PROMPT, true); - String trustStr = rc.getString(CONFIG_DIFFTOOL_SECTION, null, + String trustStr = rc.getString(CONFIG_DIFFTOOL_SECTION, toolName, CONFIG_KEY_TRUST_EXIT_CODE); if (trustStr != null) { trustExitCode = Boolean.parseBoolean(trustStr) 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 39729a4ee..b15cbdc34 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 @@ -12,11 +12,16 @@ import java.util.TreeMap; import java.util.Collections; +import java.io.File; +import java.io.IOException; import java.util.Map; import java.util.Set; +import org.eclipse.jgit.lib.Constants; 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 diff tools. @@ -25,9 +30,9 @@ public class DiffTools { private final DiffToolConfig config; - private Map predefinedTools; + private final Map predefinedTools; - private Map userDefinedTools; + private final Map userDefinedTools; /** * Creates the external diff-tools manager for given repository. @@ -37,21 +42,22 @@ public class DiffTools { */ public DiffTools(Repository repo) { config = repo.getConfig().get(DiffToolConfig.KEY); - setupPredefinedTools(); - setupUserDefinedTools(); + predefinedTools = setupPredefinedTools(); + userDefinedTools = setupUserDefinedTools(config, predefinedTools); } /** * Compare two versions of a file. * - * @param newPath - * the new file path - * @param oldPath - * the old file path - * @param newId - * the new object ID - * @param oldId - * the old object ID + * @param repo + * the repository + * @param localFile + * the local file element + * @param remoteFile + * the remote file element + * @param mergedFilePath + * the path of 'merged' file, it equals local or remote path for + * difftool * @param toolName * the selected tool name (can be null) * @param prompt @@ -61,11 +67,39 @@ public DiffTools(Repository repo) { * @param trustExitCode * the "trust exit code" option * @return the return code from executed tool + * @throws ToolException */ - public int compare(String newPath, String oldPath, String newId, - String oldId, String toolName, BooleanTriState prompt, - BooleanTriState gui, BooleanTriState trustExitCode) { - return 0; + public ExecutionResult compare(Repository repo, FileElement localFile, + FileElement remoteFile, String mergedFilePath, String toolName, + BooleanTriState prompt, BooleanTriState gui, + BooleanTriState trustExitCode) throws ToolException { + ExternalDiffTool tool = guessTool(toolName, gui); + try { + File workingDir = repo.getWorkTree(); + String localFilePath = localFile.getFile().getPath(); + String remoteFilePath = remoteFile.getFile().getPath(); + String command = tool.getCommand(); + command = command.replace("$LOCAL", localFilePath); //$NON-NLS-1$ + command = command.replace("$REMOTE", remoteFilePath); //$NON-NLS-1$ + command = command.replace("$MERGED", mergedFilePath); //$NON-NLS-1$ + Map env = new TreeMap<>(); + env.put(Constants.GIT_DIR_KEY, + repo.getDirectory().getAbsolutePath()); + env.put("LOCAL", localFilePath); //$NON-NLS-1$ + env.put("REMOTE", remoteFilePath); //$NON-NLS-1$ + env.put("MERGED", mergedFilePath); //$NON-NLS-1$ + boolean trust = config.isTrustExitCode(); + if (trustExitCode != BooleanTriState.UNSET) { + trust = trustExitCode == BooleanTriState.TRUE; + } + CommandExecutor cmdExec = new CommandExecutor(repo.getFS(), trust); + return cmdExec.run(command, workingDir, env); + } catch (IOException | InterruptedException e) { + throw new ToolException(e); + } finally { + localFile.cleanTemporaries(); + remoteFile.cleanTemporaries(); + } } /** @@ -103,41 +137,64 @@ public Map getNotAvailableTools() { */ public String getDefaultToolName(BooleanTriState gui) { return gui != BooleanTriState.UNSET ? "my_gui_tool" //$NON-NLS-1$ - : "my_default_toolname"; //$NON-NLS-1$ + : config.getDefaultToolName(); } /** * @return is interactive (config prompt enabled) ? */ public boolean isInteractive() { - return false; + return config.isPrompt(); } - private void setupPredefinedTools() { - predefinedTools = new TreeMap<>(); - for (CommandLineDiffTool tool : CommandLineDiffTool.values()) { - predefinedTools.put(tool.name(), new PreDefinedDiffTool(tool)); + private ExternalDiffTool guessTool(String toolName, BooleanTriState gui) + throws ToolException { + if (StringUtils.isEmptyOrNull(toolName)) { + toolName = getDefaultToolName(gui); } + ExternalDiffTool tool = getTool(toolName); + if (tool == null) { + throw new ToolException("Unknown diff tool " + toolName); //$NON-NLS-1$ + } + return tool; } - private void setupUserDefinedTools() { - userDefinedTools = new TreeMap<>(); - Map userTools = config.getTools(); + private ExternalDiffTool getTool(final String name) { + ExternalDiffTool tool = userDefinedTools.get(name); + if (tool == null) { + tool = predefinedTools.get(name); + } + return tool; + } + + private static Map setupPredefinedTools() { + Map tools = new TreeMap<>(); + for (CommandLineDiffTool tool : CommandLineDiffTool.values()) { + tools.put(tool.name(), new PreDefinedDiffTool(tool)); + } + return tools; + } + + private static Map setupUserDefinedTools( + DiffToolConfig cfg, Map predefTools) { + Map tools = new TreeMap<>(); + Map userTools = cfg.getTools(); for (String name : userTools.keySet()) { ExternalDiffTool userTool = userTools.get(name); // if difftool..cmd is defined we have user defined tool if (userTool.getCommand() != null) { - userDefinedTools.put(name, userTool); + tools.put(name, userTool); } else if (userTool.getPath() != null) { // if difftool..path is defined we just overload the path // of predefined tool - PreDefinedDiffTool predefTool = (PreDefinedDiffTool) predefinedTools + PreDefinedDiffTool predefTool = (PreDefinedDiffTool) predefTools .get(name); if (predefTool != null) { predefTool.setPath(userTool.getPath()); } } } + return tools; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/FileElement.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/FileElement.java new file mode 100644 index 000000000..cdc8f015f --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/FileElement.java @@ -0,0 +1,160 @@ +/* + * Copyright (C) 2018-2021, Andre Bossert + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.diffmergetool; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; + +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.lib.ObjectStream; + +/** + * The element used as left or right file for compare. + * + */ +public class FileElement { + + private final String path; + + private final String id; + + private ObjectStream stream; + + private File tempFile; + + /** + * @param path + * the file path + * @param id + * the file id + */ + public FileElement(final String path, final String id) { + this(path, id, null); + } + + /** + * @param path + * the file path + * @param id + * the file id + * @param stream + * the object stream to load instead of file + */ + public FileElement(final String path, final String id, + ObjectStream stream) { + this.path = path; + this.id = id; + this.stream = stream; + } + + /** + * @return the file path + */ + public String getPath() { + return path; + } + + /** + * @return the file id + */ + public String getId() { + return id; + } + + /** + * @param stream + * the object stream + */ + public void setStream(ObjectStream stream) { + this.stream = stream; + } + + /** + * @param workingDir the working directory used if file cannot be found (e.g. /dev/null) + * @return the object stream + * @throws IOException + */ + public File getFile(File workingDir) throws IOException { + if (tempFile != null) { + return tempFile; + } + File file = new File(path); + String name = file.getName(); + if (path.equals(DiffEntry.DEV_NULL)) { + file = new File(workingDir, "nul"); //$NON-NLS-1$ + } + else if (stream != null) { + tempFile = File.createTempFile(".__", "__" + name); //$NON-NLS-1$ //$NON-NLS-2$ + try (OutputStream outStream = new FileOutputStream(tempFile)) { + int read = 0; + byte[] bytes = new byte[8 * 1024]; + while ((read = stream.read(bytes)) != -1) { + outStream.write(bytes, 0, read); + } + } finally { + // stream can only be consumed once --> close it + stream.close(); + stream = null; + } + return tempFile; + } + return file; + } + + /** + * Returns a real file from work tree or a temporary file with content if + * stream is valid or if path is "/dev/null" + * + * @return the object stream + * @throws IOException + */ + public File getFile() throws IOException { + if (tempFile != null) { + return tempFile; + } + File file = new File(path); + String name = file.getName(); + // if we have a stream or file is missing ("/dev/null") then create + // temporary file + if ((stream != null) || path.equals(DiffEntry.DEV_NULL)) { + // TODO: avoid long random file name (number generated by + // createTempFile) + tempFile = File.createTempFile(".__", "__" + name); //$NON-NLS-1$ //$NON-NLS-2$ + if (stream != null) { + try (OutputStream outStream = new FileOutputStream(tempFile)) { + int read = 0; + byte[] bytes = new byte[8 * 1024]; + while ((read = stream.read(bytes)) != -1) { + outStream.write(bytes, 0, read); + } + } finally { + // stream can only be consumed once --> close it + stream.close(); + stream = null; + } + } + return tempFile; + } + return file; + } + + /** + * Deletes and invalidates temporary file if necessary. + */ + public void cleanTemporaries() { + if (tempFile != null && tempFile.exists()) + tempFile.delete(); + tempFile = null; + } + +} 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 1c69fb491..092cb605b 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 @@ -46,17 +46,6 @@ public PreDefinedDiffTool(CommandLineDiffTool tool) { */ @Override public void setPath(String path) { - // handling of spaces in path - if (path.contains(" ")) { //$NON-NLS-1$ - // add quotes before if needed - if (!path.startsWith("\"")) { //$NON-NLS-1$ - path = "\"" + path; //$NON-NLS-1$ - } - // add quotes after if needed - if (!path.endsWith("\"")) { //$NON-NLS-1$ - path = path + "\""; //$NON-NLS-1$ - } - } super.setPath(path); } 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 new file mode 100644 index 000000000..7862cf596 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2018-2021, Andre Bossert + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.diffmergetool; + +import org.eclipse.jgit.util.FS.ExecutionResult; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Tool exception for differentiation. + * + */ +public class ToolException extends Exception { + + private final static Logger LOG = LoggerFactory + .getLogger(ToolException.class); + + private final ExecutionResult result; + + /** + * the serial version UID + */ + private static final long serialVersionUID = 1L; + + /** + * + */ + public ToolException() { + super(); + result = null; + } + + /** + * @param message + * the exception message + */ + public ToolException(String message) { + super(message); + result = null; + } + + /** + * @param message + * the exception message + * @param result + * the execution result + */ + public ToolException(String message, ExecutionResult result) { + super(message); + this.result = result; + } + + /** + * @param message + * the exception message + * @param cause + * the cause for throw + */ + public ToolException(String message, Throwable cause) { + super(message, cause); + result = null; + } + + /** + * @param cause + * the cause for throw + */ + public ToolException(Throwable cause) { + super(cause); + result = null; + } + + /** + * @return true if result is valid, false else + */ + public boolean isResult() { + return result != null; + } + + /** + * @return the execution result + */ + public ExecutionResult getResult() { + return result; + } + + /** + * @return the result Stderr + */ + public String getResultStderr() { + try { + return new String(result.getStderr().toByteArray()); + } catch (Exception e) { + LOG.warn(e.getMessage()); + } + return ""; //$NON-NLS-1$ + } + + /** + * @return the result Stdout + */ + public String getResultStdout() { + try { + return new String(result.getStdout().toByteArray()); + } catch (Exception e) { + LOG.warn(e.getMessage()); + } + return ""; //$NON-NLS-1$ + } + +}