-
Notifications
You must be signed in to change notification settings - Fork 85
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
Comments
Missing source. Anyway, there are indeed some hooks that send data via stdin. The
Right.
Which OS are you using? Hooks are handled differently on Windows compared to Linux or MacOS.
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:
And sure enough, the contents of the file 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 If you can run Git GUI from something like Rofi, then great! Your problem is solved. If not, see this. Running |
Thanks for the comprehensive response! I'll reply inline.
Oops! I meant to go back and add a link to https://git-scm.com/docs/githooks.
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 18.04.5 LTS
Release: 18.04
Codename: bionic
Ahh of course! I should've tried this 😆
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 (
Wow ..
I think
Thanks! Yeah, this feels like the most "correct" solution, but it's definitely not critical! |
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 pressingCommit
while executing a pre-commit hook. I've tracked this to a call tocat > ...
in my hook. It seems to me thatgit-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)
(expected behavior -- this is what plain
git
does)(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 Icat
, but that doesn't seem like a good long-term solution.For the record, I'm aware that
git
won't send me data onstdin
for thepre-commit
hook, but certain hooks do send data this way (source):Here are the versions of the commands I'm using:
I tried looking through the source, but I only got as far as
_open_stdout_stderr
since I don't know Tcl 😄Thanks!
The text was updated successfully, but these errors were encountered: