JGit: tool execution return code: 127 when diff or merge due to error in CommandExecutor.createCommandArray() when Cygwin is on Windows Path

[JeanGarf]

Version

6.8.0.202311291450-r

Operating System

Windows

Bug description

1) Add Cygwin in your Windows Path Environment Variable

2) Launch Eclipse and Configure Eclipse eGit preferences to use External Tool for Git diff or merge

3) In Git Staging View, double click on a modified file.

Actual behavior

We get the following message :

Failed to run external merge tool. Merge aborted! JGit: tool execution return code: 127 checkExitCode: true execError: true stderr: C:\Users\xxx\AppData\Local\Temp.{}13599643848199867671{}jgittool.sh: line 1: C:UsersxxxAppDataLocalTemp.{}13599643848199867671{}_jgit_tool.sh: command not found

Expected behavior

The external tool should be launched for Git Diff or Merge without error

Relevant log output

No response

Other information

The JGit bug is located in CommandExecutor.createCommandArray() (of org.eclipse.jgit 6.8.0.202311291450-r) at line 168 : } else if (fs instanceof FS_Win32) {

...

} else if (fs instanceof FS_Win32_Cygwin) {

...

    commandArray[0] = commandFile.getCanonicalPath().replace("

", "/"); //$NON-NLS-1$ //$NON-NLS-2$ } else {

If Cygwin/bin is part of the Windows path, fs is instantiated as FS_Win32_Cygwin which is a sub class of FS_Win32.

Since JGit tests FS_Win32 before FS_Win32_Cygwin, we always end up in the FS_Win32 branch and never in the FS_Win32_Cygwin branch, even if fs is a FS_Win32_Cygwin.

As a result, the .replace("\", "/") is never done.

In CommandExecutor.run() line 78,

ProcessBuilder pb = fs.runInShell(commandArray[0],

The runInShell() call invokes the FS_Win32_Cygwin version which executes "sh.exe -c" instead of "cmd.exe /c" (like in FS_Win32).

However, in the case of "sh.exe -c", the following command should not contain any backslashes (which are interpreted as escaping characters), but slashes, which is not the case because the .replace("\", "/") hasn't been done.

[tomaswolf]

So the simple fix is to move that cygwin branch before the general win32 branch. See Gerrit change 1177880.

[msohn]

Change submitted as 819c5bcc8b2a2685c20e5b8e568f776b19f7db63

[JeanGarf]

Looks good.

Thanks a lot

[tomaswolf]

From code inspection I would expect more problems if the Windows user name contains blanks and thus the path to the temp file contains blanks. Some quoting might be needed, and perhaps FS.shellQuote() should become public.

But that would be a separate issue to tackle if and when it is reported.