Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git-gui doesn't close stdin when executing hooks #55

Open
kmurphy4 opened this issue Apr 16, 2021 · 2 comments
Open

git-gui doesn't close stdin when executing hooks #55

kmurphy4 opened this issue Apr 16, 2021 · 2 comments

Comments

@kmurphy4
Copy link

Hi! This is the first issue I've opened, so please let me know if you'd like any more information.

I'm running into an issue where git-gui will hang indefinitely after pressing Commit while executing a pre-commit hook. I've tracked this to a call to cat > ... in my hook. It seems to me that git-gui is not handling stdin to the child process correctly (this same hook doesn't fail on the command line).

Here's an example that hopefully makes it more clear:

(setup)

$ git init
$ cat > .git/hooks/pre-commit
#!/bin/bash
cat > x
$ chmod +x .git/hooks/pre-commit

(expected behavior -- this is what plain git does)

$ touch y
$ git add y
$ rm -f x
$ git commit -m "test"  # returns immediately
$ test -e x && echo created || echo failed
created

(actual behavior -- this is what git-gui does)

$ touch z
$ git add z
$ rm -f x
$ git gui  # Press "Commit" => hangs forever "Calling pre-commit hook..."

I don't see any way to close the stdin to the hook process, so it's impossible to commit using the GUI as-is. I've "solved" this by just adding calls to timeout when I cat, but that doesn't seem like a good long-term solution.

For the record, I'm aware that git won't send me data on stdin for the pre-commit hook, but certain hooks do send data this way (source):

Hooks can get their arguments via the environment, command-line arguments, and stdin. See the documentation for each hook below for details.

Here are the versions of the commands I'm using:

$ git --version
git version 2.31.1
$ git gui --version
git-gui version 0.21.0.99.gdf4f9e

I tried looking through the source, but I only got as far as _open_stdout_stderr since I don't know Tcl 😄

Thanks!

@prati0100
Copy link
Owner

prati0100 commented Apr 19, 2021

I don't see any way to close the stdin to the hook process, so it's impossible to commit using the GUI as-is. I've "solved" this by just adding calls to timeout when I cat, but that doesn't seem like a good long-term solution.

For the record, I'm aware that git won't send me data on stdin for the pre-commit hook, but certain hooks do send data this way (source):

Missing source. Anyway, there are indeed some hooks that send data via stdin. The post-rewrite hook is an example.

Hooks can get their arguments via the environment, command-line arguments, and stdin. See the documentation for each hook below for details.

Right.

Here are the versions of the commands I'm using:

$ git --version
git version 2.31.1
$ git gui --version
git-gui version 0.21.0.99.gdf4f9e

Which OS are you using? Hooks are handled differently on Windows compared to Linux or MacOS.

I tried looking through the source, but I only got as far as _open_stdout_stderr since I don't know Tcl smile

I went through the Tcl open docs. It has the below line:

"If read-only access is used (e.g. access is r), standard input for the pipeline is taken from the current standard input unless overridden by the command."

So I added the hook you mentioned above, opened Git GUI via a terminal, and ran commit. Sure enough, the GUI got stuck waiting for the pre-commit hook. Then I went back to the terminal which I used to open Git GUI and typed:

Hello
^D

And sure enough, the contents of the file x were "Hello". So the stdin of Git GUI is indeed being redirected to the stdin of the hook.

On one hand, the hook is being naughty by reading from stdin when it has no business doing it. My first reaction was to say "your hook is broken, fix it". But on the other hand it doesn't make much sense to redirect Git GUI's stdin to the hook. That is unexpected and surprising behaviour. The fix would be to open it as both read and write, but then immediately close the write end. Unfortunately this is not as easy as it sounds. See here. I have not spent much time looking at the problem but I see no obvious way of closing only the write end on Tcl 8.5.

This time I tried running Git GUI with Rofi (it is a program launcher, like dmenu). I assumed that a launcher like this would simply close the stdin because it won't know what to do with it anyway. And sure enough, when I run the commit the hook does not hang and creates the file x.

If you can run Git GUI from something like Rofi, then great! Your problem is solved. If not, see this. Running : | git gui from my shell also fixes this problem. I will look into actually fixing it but I don't know how long it will take me to figure out this bidirectional thing.

@kmurphy4
Copy link
Author

Thanks for the comprehensive response! I'll reply inline.

Missing source. Anyway, there are indeed some hooks that send data via stdin. The post-rewrite hook is an example.

Oops! I meant to go back and add a link to https://git-scm.com/docs/githooks.

Which OS are you using? Hooks are handled differently on Windows compared to Linux or MacOS.

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 18.04.5 LTS
Release:	18.04
Codename:	bionic

And sure enough, the contents of the file x were "Hello". So the stdin of Git GUI is indeed being redirected to the stdin of the hook.

Ahh of course! I should've tried this 😆

On one hand, the hook is being naughty by reading from stdin when it has no business doing it. My first reaction was to say "your hook is broken, fix it".

Yeah, I think this is also a reasonable position.

In my case, it's slightly convoluted because I have a single "hook" script that handles all possible hooks that git could generate and dispatches to code I keep in source control. To make that dispatcher/wrapper as simple as possible, I wanted to handle all calling conventions. Does that make sense?

If it's too much of a hassle to close the pipe, then I could just whitelist some hooks (post-rewrite, for example) to read stdin and skip that for the rest.

But on the other hand it doesn't make much sense to redirect Git GUI's stdin to the hook. That is unexpected and surprising behaviour. The fix would be to open it as both read and write, but then immediately close the write end. Unfortunately this is not as easy as it sounds. See here. I have not spent much time looking at the problem but I see no obvious way of closing only the write end on Tcl 8.5.

the ability to close only one end of a
pipe is not present in older Tcl

Wow ..

This time I tried running Git GUI with Rofi (it is a program launcher, like dmenu). I assumed that a launcher like this would simply close the stdin because it won't know what to do with it anyway. And sure enough, when I run the commit the hook does not hang and creates the file x.

If you can run Git GUI from something like Rofi, then great! Your problem is solved. If not, see this. Running : | git gui from my shell also fixes this problem.

I think : | ... is a fine (though awkward) solution, thanks! It could probably be smoothed over with a simple git alias.

I will look into actually fixing it but I don't know how long it will take me to figure out this bidirectional thing.

Thanks! Yeah, this feels like the most "correct" solution, but it's definitely not critical!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants