threads / rfc / 2762

[RFC] Run hooks with a cleaner environment

Subject: [RFC] Run hooks with a cleaner environment

## tl;dr

5 messages between Dec 6, 2005 and Dec 7, 2005.

replies: 4people: 3as markdown or json

Daniel Barkalow· Dec 6, 2005, 22:43 UTC · lore

Currently, hooks/post-update is run in the environment that receive-pack is run. This means that there are a number of things that are unpredictable. I'd like to make it set things up in a more predictable and useful way. The things I know are odd:

stdout and stdin are connected to send-pack, either by broken pipes (for local pushes) or an ignored socket (via ssh). stdin should probably be /dev/null, and stdout should be either a log file or /dev/null. stderr is still the push's stderr, which may or may not be desired.

GIT_DIR is set to the repository that got the push, which may surprise people who only use it in "GIT_DIR=foo git ..." form and don't expect it ever to be set from outside. Of course, it's potentially useful to know what repository is running the hook, but that doesn't have to be communicated in such a way that git programs will pick it up directly. Other environment variables could potentially be purged, too, but I don't think that's as important, since the user probably knows about them.

cwd is set to the push's cwd if it's local, maybe $HOME if it's over ssh. It should probably always be $HOME, unless we want it to be $GIT_DIR.

Is there anything else we want to regularize? Is there some sort of standard behavior we should match, like CVS or cron?

	-Daniel
*This .sig left intentionally blank*
Paul Serice· Dec 7, 2005, 00:19 UTC · re: Daniel Barkalow · lore

Re: [RFC] Run hooks with a cleaner environment

> Currently, hooks/post-update is run in the environment that
> receive-pack is run. This means that there are a number of things
> that are unpredictable. I'd like to make it set things up in a more
> predictable and useful way.

I'd like to second this. I've been bitten by two of the three issues you've raised.

Show 5 quoted lines
> stdout and stdin are connected to send-pack, either by broken pipes
> (for local pushes) or an ignored socket (via ssh). stdin should
> probably be /dev/null, and stdout should be either a log file or
> /dev/null. stderr is still the push's stderr, which may or may not
> be desired.

If there is a controlling terminal and nothing else git-related is reading from it, I'd like for stdout and stderr to be reconnected.

Paul Serice
Junio C Hamano· Dec 7, 2005, 00:39 UTC · re: Daniel Barkalow · lore

Re: [RFC] Run hooks with a cleaner environment

Daniel Barkalow <barkalow@iabervon.org> writes:
> GIT_DIR is set to the repository that got the push,

That is done by receive-pack; it chdir()s into the repository and does its thing, and the hooks are called from there; I'd expect cwd to be the repository ('.git'), GIT_DIR to be dot ('.').

I think doing the "unset GIT_DIR" to be the first thing if you want to access some other repository is documented somewhere but if not please send a patch to document it.

As to file descriptors, I think duping the output to original stderr might make sense, but I do not know what breaks, so interested parties may want to test it out and submit a tested patch for inclusion.

Daniel Barkalow· Dec 7, 2005, 17:47 UTC · re: Junio C Hamano · lore

Re: [RFC] Run hooks with a cleaner environment

On Tue, 6 Dec 2005, Junio C Hamano wrote:
Show 8 quoted lines
> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > GIT_DIR is set to the repository that got the push,
> 
> That is done by receive-pack; it chdir()s into the repository
> and does its thing, and the hooks are called from there; I'd
> expect cwd to be the repository ('.git'), GIT_DIR to be dot
> ('.').

I thought I was seeing the full path of the repository as GIT_DIR and I didn't check the cwd.

> I think doing the "unset GIT_DIR" to be the first thing if you
> want to access some other repository is documented somewhere but
> if not please send a patch to document it.

I didn't see it in the place "grep post-update Documentation/*" returned, so we need something. (Actually, the main thing is to specify that nothing else special is set, because the GIT_DIR thing was pretty obvious, but I then didn't know if my problems were due to something else undocumented.)

> As to file descriptors, I think duping the output to original
> stderr might make sense, but I do not know what breaks, so
> interested parties may want to test it out and submit a tested
> patch for inclusion.

I'll send a patch tonight which works for me, but it should probably be checked over by people who are good at this sort of stuff. I've got a "/dev/null" patch; I'll look into a version that tries to find a controlling tty (which could be really interesting, since you could then have the hook get input from the user), or at least copy stderr if possible.

For reference, the error I was getting was a broken pipe writing to stdout (as git merge does somewhere) when I've pushed locally.

	-Daniel
*This .sig left intentionally blank*
Junio C Hamano· Dec 7, 2005, 18:57 UTC · re: Daniel Barkalow · lore

Re: [RFC] Run hooks with a cleaner environment

Daniel Barkalow <barkalow@iabervon.org> writes:
Show 13 quoted lines
> On Tue, 6 Dec 2005, Junio C Hamano wrote:
>
>> Daniel Barkalow <barkalow@iabervon.org> writes:
>> 
>> > GIT_DIR is set to the repository that got the push,
>> 
>> That is done by receive-pack; it chdir()s into the repository
>> and does its thing, and the hooks are called from there; I'd
>> expect cwd to be the repository ('.git'), GIT_DIR to be dot
>> ('.').
>
> I thought I was seeing the full path of the repository as GIT_DIR and I 
> didn't check the cwd.

I do not do this myself, but I was wondering what would happen if somebody has "export GIT_DIR=/var/filfre" in ~/.profile.

Well, I know what would happen, actually --- things would not work when you do fetch/push because the tools want to use the path given from the other end but the environment overrides it with GIT_DIR.

← back to recent threads