Bug 573992 - [cygwin] Running hook through Eclipse changes output of cygwins Git
Summary: [cygwin] Running hook through Eclipse changes output of cygwins Git
Status: NEW
Alias: None
Product: JGit
Classification: Technology
Component: JGit (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-06-03 16:38 EDT by Fabian Pfaff CLA
Modified: 2021-06-23 11:23 EDT (History)
1 user (show)

See Also:


Attachments
Hook for step 2 in the reproduction (42 bytes, text/plain)
2021-06-03 16:38 EDT, Fabian Pfaff CLA
no flags Details
Main.java (tries to mimic FS_Win32_Cygwin#runInShell) (841 bytes, text/x-java)
2021-06-03 16:40 EDT, Fabian Pfaff CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fabian Pfaff CLA 2021-06-03 16:38:38 EDT
Created attachment 286520 [details]
Hook for step 2 in the reproduction

We're currently testing Eclipses hook support under Windows through cygwin.
I found an unexpected behavior that broke our hooks.
I'm not sure if this is caused by Eclipse or Gits C implementation or if this is even unintended behavior.

Reproduction:

1. Install Windows 10, Eclipse 03-2021, cygwin with Git
2. Create repository with a post-commit hook (see attachment)
3. Commit through Eclipse via EGit/JGit (start eclipse.exe with -debug to see stdout)
4. Commit through C Git via cygwin terminal

Expected:

Step 3 and 4 return the same path, in cygwin / POSIX format.
eg. "/cygdrive/c/Users/IEUser/eclipse-workspace/Test"

Actual:

Step 3 (Eclipse) returns a windows path, eg. "C:\Users\IEUser\eclipse-workspace\Test"
Step 4 (c git) returns a cygwin posix path: "/cygdrive/c/Users/IEUser/eclipse-workspace/Test"

I've tried to reproduce what JGit does in https://github.com/eclipse/jgit/blob/f6b9b392e7c31cee6954aedc12001de3c17731a3/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java#L115, see Main.java attachment.
But the output there matched the expected posix path.
Comment 1 Fabian Pfaff CLA 2021-06-03 16:40:08 EDT
Created attachment 286521 [details]
Main.java (tries to mimic FS_Win32_Cygwin#runInShell)
Comment 2 Thomas Wolf CLA 2021-06-04 03:28:55 EDT
This is not EGit or Eclipse changing anything. But Eclipse is not started from a cygwin terminal. Is the hook even running the same git executable? is $PATH the same?

What if you change the shebang to #!/bin/sh --login ?
Comment 3 Fabian Pfaff CLA 2021-06-09 12:19:09 EDT
Thank you for your input!

> Is the hook even running the same git executable?

Explicitly pointing to a /usr/libexec/git-core/git in the hook doesn't change the behavior.

> is $PATH the same?

$PATH when hook is started from bash:
/usr/libexec/git-core:/cygdrive/c/Program Files/AdoptOpenJDK/jdk-11.0.11.9-hotspot/bin:/cygdrive/c/Windows/system32:/cygdrive/c/Windows:/cygdrive/c/Windows/System32/Wbem:/cygdrive/c/Windows/System32/WindowsPowerShell/v1.0:/cygdrive/c/Windows/System32/OpenSSH:/cygdrive/c/ProgramData/chocolatey/bin:/cygdrive/c/Program Files/Puppet Labs/Puppet/bin:/usr/bin:/usr/local/bin:/cygdrive/c/Users/IEUser/AppData/Local/Microsoft/WindowsApps

$PATH when hook started through EGit / JGit:

/cygdrive/c/Users/IEUser/Downloads/eclipse-java-2021-03-R-win32-x86_64/plugins/org.eclipse.justj.openjdk.hotspot.jre.full.win32.x86_64_15.0.2.v20210201-0955/jre/bin/server:/cygdrive/c/Users/IEUser/Downloads/eclipse-java-2021-03-R-win32-x86_64/plugins/org.eclipse.justj.openjdk.hotspot.jre.full.win32.x86_64_15.0.2.v20210201-0955/jre/bin:/cygdrive/c/Program Files/AdoptOpenJDK/jdk-11.0.11.9-hotspot/bin:/cygdrive/c/Windows/system32:/cygdrive/c/Windows:/cygdrive/c/Windows/System32/Wbem:/cygdrive/c/Windows/System32/WindowsPowerShell/v1.0:/cygdrive/c/Windows/System32/OpenSSH:/cygdrive/c/ProgramData/chocolatey/bin:/cygdrive/c/Program Files/Puppet Labs/Puppet/bin:/usr/bin:/usr/local/bin:/cygdrive/c/Users/IEUser/AppData/Local/Microsoft/WindowsApps:/cygdrive/c/Users/IEUser/Downloads/eclipse-java-2021-03-R-win32-x86_64

> What if you change the shebang to #!/bin/sh --login ?

cygwin seems to not support this:
/bin/sh: .git/hooks/post-commit: No such file or directory

So there is definitely a difference in the environment but I don't get how this can change the output of git rev-parse --show-toplevel.
Maybe some cygwin specific magic that I don't know about.
I checked the .bashrc and .bash_profile and they don't do anything (everything is commented out).

I'm curious if you have any other ideas what I might check but feel free to close this since this doesn't seem to be EGit related.
Comment 4 Thomas Wolf CLA 2021-06-09 15:49:15 EDT
(In reply to Fabian Pfaff from comment #3)
> > What if you change the shebang to #!/bin/sh --login ?
> 
> cygwin seems to not support this:
> /bin/sh: .git/hooks/post-commit: No such file or directory

Doh! Of course -- I suppose that changes the current directory to the home dir.

> I'm curious if you have any other ideas what I might check but feel free to
> close this since this doesn't seem to be EGit related.

Make the hook print the output of "which git" and "git --version".

As far as I see, JGit/EGit just passes on the output of the hook. But perhaps I'm missing something... So here's an additional idea: make the hook write its output to a file (maybe via tee). Examine the file. If it's different already there, it's definitely not an EGit problem. If both files have the Cygwin path, we may have to dig deeper in JGit/EGit.
Comment 5 Fabian Pfaff CLA 2021-06-10 08:55:49 EDT
> Doh! Of course -- I suppose that changes the current directory to the home dir.

Your assumption is correct, it changes the current directory (which is not something thtat I expected as this doesn't happen on my linux machine). But this isn't the problem here. cygwin fails to start the hook as it can't find /bin/sh once an additional argument is passed in the shebang statement (at least that is what the error message claims).

I wrote the following hook to test with a login shell:

> #!/bin/sh
> 
> /bin/sh -l << EOF
> cd /cygdrive/c/Users/IEUser/eclipse-workspace/Test
> /usr/bin/git rev-parse --show-toplevel | tee out.txt
> echo $PATH
> EOF

Triggering this hook through Eclipse still returns the windows style path.

> Make the hook print the output of "which git" and "git --version".

bash:
git is /usr/libexec/git-core/git
git version 2.31.1

eclipse:
git is /usr/bin/git
git version 2.31.1

Sadly changing the hook to always point to either one of those binaries doesn't change the behavior.
Looking at https://cygwin.com/packages/x86/git/git-2.31.1-2 I think they do the same thing anyways and both come from the cygwin git (no other git was installed anyways).

> make the hook write its output to a file (maybe via tee). Examine the file. If it's different already there, it's definitely not an EGit problem.

The output in the file has the windows style path. I don't think that Eclipse does processing on the stdout.

My current theory is that the way JGit starts the shell breaks the cygwin detection of git in some way so it puts out a windows style path. Sadly I can't reproduce the behavior outside of JGit. (see my attempt through https://bugs.eclipse.org/bugs/attachment.cgi?id=286521)
Comment 6 Thomas Wolf CLA 2021-06-10 10:04:11 EDT
(In reply to Fabian Pfaff from comment #5)
> My current theory is that the way JGit starts the shell breaks the cygwin
> detection of git in some way so it puts out a windows style path. Sadly I
> can't reproduce the behavior outside of JGit. (see my attempt through
> https://bugs.eclipse.org/bugs/attachment.cgi?id=286521)

This test program writes the Cygwin path?

So let's try this: replace

  String cmd = "/cygdrive/c/Users/IEUser/eclipse-workspace/Test/.git/hooks/post-commit";

in your test with

  String cmd = "C:\\Users\\IEUser\\eclipse-workspace\\Test\\.git\\hooks\\post-commit";

Does it now print the Win-style path?
Comment 7 Thomas Wolf CLA 2021-06-10 10:29:45 EDT
(In reply to Thomas Wolf from comment #6)
> (In reply to Fabian Pfaff from comment #5)
> > My current theory is that the way JGit starts the shell breaks the cygwin
> > detection of git in some way so it puts out a windows style path. Sadly I
> > can't reproduce the behavior outside of JGit. (see my attempt through
> > https://bugs.eclipse.org/bugs/attachment.cgi?id=286521)
> 
> This test program writes the Cygwin path?
> 
> So let's try this: replace
> 
>   String cmd =
> "/cygdrive/c/Users/IEUser/eclipse-workspace/Test/.git/hooks/post-commit";
> 
> in your test with
> 
>   String cmd =
> "C:\\Users\\IEUser\\eclipse-workspace\\Test\\.git\\hooks\\post-commit";
> 
> Does it now print the Win-style path?

Actually, I think JGit would run as if we had

  String cmd = "C:/Users/IEUser/eclipse-workspace/Test/.git/hooks/post-commit";

See FS_Win_Cygwin.shellQuote() and FS.internalRunHookIfPresent().

If the format of the path influences whether the called script might output Cygwin paths or Win-style paths we have a bit of a problem. If we change it now, for instance by not simply replacing \ by / in shellQuote but actually using cygpath -u, users who relied (maybe implicitly) on the current behavior might be in for surprises.

(Shelling out twice -- first to cygpath, then to actually run the hook -- is also not exactly efficient, but maybe acceptable for hooks.)
Comment 8 Fabian Pfaff CLA 2021-06-23 11:17:54 EDT
(In reply to Thomas Wolf from comment #6)
> This test program writes the Cygwin path?
> 
> So let's try this: replace
> 
>   String cmd =
> "/cygdrive/c/Users/IEUser/eclipse-workspace/Test/.git/hooks/post-commit";
> 
> in your test with
> 
>   String cmd =
> "C:\\Users\\IEUser\\eclipse-workspace\\Test\\.git\\hooks\\post-commit";
> 
> Does it now print the Win-style path?

This leads to an error message:

> C:\Users\IEUser\eclipse-workspace\Test\.git\hooks\post-commit: C:UsersIEUsereclipse-workspaceTest.githookspost-commit: command not found

(In reply to Thomas Wolf from comment #7)
> Actually, I think JGit would run as if we had
> 
>   String cmd =
> "C:/Users/IEUser/eclipse-workspace/Test/.git/hooks/post-commit";
> 
> See FS_Win_Cygwin.shellQuote() and FS.internalRunHookIfPresent().

This also prints the cygwin path, not a windows path.
Comment 9 Fabian Pfaff CLA 2021-06-23 11:23:31 EDT
We fixed this for us by not relying on `git rev-parse --show-toplevel` anymore. (working directory for hooks is guaranteed to be deterministic and independent of the working directory of the calling shell, so it turns out it wasn't really necessary in the first place)

If you have any ideas that I could investigate let me know, otherwise I'm out of ideas. Thank you for all your suggestions and input! :)