Re: [PATCH] Added hook in git-receive-pack
On Sunday 31 July 2005 21:17, Josef Weidendorfer wrote: Added hook in git-receive-pack Regarding the update hook: In this script, it would be nice to be able to distinguish rebasing/forced setting of a head from a regular fast forwarding. In the first case, I do not want to potentially send a list of all commits starting from project root, which currently can happen. A solution would be to call the hook simply with update refname new_sha1 if this is not a fast forward. The question is if there is a use case for the script to get the old_sha1 even in a rebase situation. So another option would be to call the script with update action refname old_sha1 new_sha1 with action being fastforward or forcedrebase or similar. Or is there already an easy way to detect the fast-forward situation in the script? Josef - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
Josef Weidendorfer [EMAIL PROTECTED] writes: Or is there already an easy way to detect the fast-forward situation in the script? Since you are given old and new, presumably you can do merge-base in the hook to see what it yields? - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
On Saturday 13 August 2005 21:27, Junio C Hamano wrote: Josef Weidendorfer [EMAIL PROTECTED] writes: Or is there already an easy way to detect the fast-forward situation in the script? Since you are given old and new, presumably you can do merge-base in the hook to see what it yields? Ah, yes, thanks. A proposed update for the example hook (one change actually is a fix): Fixed/Extended example for update hook --- diff --git a/templates/hooks--update b/templates/hooks--update --- a/templates/hooks--update +++ b/templates/hooks--update @@ -1,6 +1,7 @@ #!/bin/sh # # An example hook script to mail out commit update information. +# Called by git-receive-pack with arguments: refname sha1-old sha1-new # # To enable this hook: # (1) change the recipient e-mail address @@ -12,10 +13,15 @@ recipient=[EMAIL PROTECTED] if expr $2 : '0*$' /dev/null then echo Created a new ref, with the following commits: - git-rev-list --pretty $2 + git-rev-list --pretty $3 else - echo New commits: - git-rev-list --pretty $3 ^$2 + $base=$(git-merge-base $2 $3) + if [ $base == $2 ]; then + echo New commits: + else + echo Rebased ref, commits from common ancestor: + fi + git-rev-list --pretty $3 ^$base fi | mail -s Changes to ref $1 $recipient exit 0 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
Josef, I've committed a version that has slightly different semantics from what you originally posted. The differences are: - Instead of being post-change hook, the script is run just before each ref is updated. The script can exit with non-zero status to tell receive-pack not to update that ref if it wants to. This means that you should explicitly exit with zero status if all you want to do in the hook is to send a mail out. - The script is called once at the very end with a single parameter (i.e. $refname == ), to signal that receive-pack is about to finish. This is a good place to add any final cleanup hook. The latter change allowed me to remove the mandatory update_server_info call Linus did not like and make it optional. -jc - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
On Sun, 31 Jul 2005, Josef Weidendorfer wrote: Added hook in git-receive-pack After successful update of a ref, $GIT_DIR/hooks/update refname old-sha1 new-sha2 is called if present. This allows e.g sending of a mail with pushed commits on the remote repository. Documentation update with example hook included. This looks sane. However, I also get the strong feeling that git-update-server-info should be run as part of a hook and not be built into receive-pack.. Personally, I simply don't want to update any dumb server info stuff for my own local repositories - it's not like I'm actually serving those out anyway. Linus - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
Josef Weidendorfer [EMAIL PROTECTED] writes:
+It is assured that sha1-old is an ancestor of sha1-new (otherwise,
+the update would have not been allowed). refname is relative to
+$GIT_DIR; e.g. for the master head this is refs/heads/master.
I think this description is inaccurate; the send-pack can be run
with the --force flag and it is my understanding that receiver
would happily rewind the branch. One possibility, if we wanted
to enforce it on the receiver end, would be to add another hook
that is called before the rename happens and tell the
receive-pack to refuse that update, but that should be done with
a separate patch, I suppose.
+Using this hook, it is easy to generate mails on updates to
+the local repository. This example script sends a mail with
+the commits pushed to the repository:
+
+ #!/bin/sh
+ git-rev-list --pretty $3 ^$2 |
+mail -r $USER -s New commits on $1 [EMAIL PROTECTED]
What is the environment the hook runs in? For example, who
defines $USER used here?
We might want to describe the environment a bit more tightly
than the current patch does. This includes not just the
environment variables, but $cwd and the set of open file
descriptors among other things.
I am not saying this from the security standpoint (the fact that
you can invoke receive-pack and that you can write into the
update hooks means you already have control over that
repository), but to help hook writers to avoid making mistakes.
For example, I offhand cannot tell what happens if the hook
tries to read from its standard input. Also what happens if the
hook does not return but sleeps forever in a loop? Do we want
to somehow time it out? I think It is hooks' responsibility to
time itself out is an acceptable answer here, but if that is
the case it had better be documented.
+static void updatehook(const char *name, unsigned char *old_sha1, unsigned
char *new_sha1)
+{
+if (access(update_hook, X_OK) 0) return;
+ fprintf(stderr, executing update hook for %s\n, name);
+...
+}
I think I've seen this fork -- exec -- for loop with waitpid
pattern repeated number of times in the code. Would it be
feasible to make them into a single library-ish function and
call it from here and other existing places?
Another thing you may want to consider is to do this hook
processing before and/or after processing all the refs. A hook
might want to know what the entire set of refs are that are
being updated, and may not have enough information if it is
invoked once per ref.
Thanks for the patch; I agree with what the patch tries to
achieve in general.
-jc
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
Linus Torvalds [EMAIL PROTECTED] writes: This looks sane. However, I also get the strong feeling that git-update-server-info should be run as part of a hook and not be built into receive-pack.. Personally, I simply don't want to update any dumb server info stuff for my own local repositories - it's not like I'm actually serving those out anyway. But you are. I can run this just fine: $ git clone http://www.kernel.org/pub/scm/linux/kernel/git/torvalds/git.git/ linus I agree in principle that you should be able to disable the call to update_server_info() from there, but on the other hand once we start doing it, we need to explain people which repo is http capable and which repo is not and why. I was actually thinking about a call to git-update-server-info at the end of git-repack-script. Again, great minds think the opposite way sometimes ;-). - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
On Sun, 31 Jul 2005, Junio C Hamano wrote: But you are. I can run this just fine: No I'm not. Try all the machines behind my firewall. kernel.org is just the place I put things to when I publish them. It doesn't have any of my working directories on it. Linus - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
On Sunday 31 July 2005 22:15, Junio C Hamano wrote: Josef Weidendorfer [EMAIL PROTECTED] writes: +It is assured that sha1-old is an ancestor of sha1-new (otherwise, +the update would have not been allowed). refname is relative to +$GIT_DIR; e.g. for the master head this is refs/heads/master. I think this description is inaccurate; Thanks for the constructive comments; that patch was only a draft, and tailored for my needs. I thought it would be better to provide a patch than requesting for a feature. I am trying to convert a CVS project with a few developers to GIT; the idea is that each developer has his public branch in *the* central repository, and (s)he is allowed to merge to master him/herself; when pushing new features into the branches or merging to master, there should be send out a mail to a mailing list. the send-pack can be run with the --force flag and it is my understanding that receiver would happily rewind the branch. I didn't know this... One possibility, if we wanted to enforce it on the receiver end, I actually thought that the ancestor relationship always is given; perhaps we don't need to enforce this; but before sending out a mail, the hook script probably would like to check if there is any ancestor relationship; this would be a potential long lasting task, wouldn't it? would be to add another hook that is called before the rename happens and tell the receive-pack to refuse that update, but that should be done with a separate patch, I suppose. Sorry, I do not understand this. +Using this hook, it is easy to generate mails on updates to +the local repository. This example script sends a mail with +the commits pushed to the repository: + + #!/bin/sh + git-rev-list --pretty $3 ^$2 | +mail -r $USER -s New commits on $1 [EMAIL PROTECTED] What is the environment the hook runs in? For example, who defines $USER used here? Good question. I thought it is UNIX/POSIX behavior to set this environemt in shells (same as id -u -n). At least, ssh sets it to the user logging in (see man ssh). And I supposed git-receive-pack to be called in a users SSH environment. Hmmm... could this be called via CGI script by a web server? You are right: we should be careful here. Is there any other hook mechanism in GIT at the moment? Originally, I thought this should be a task for a porcelain, but this thing is buried too deep in GIT itself... All the issues about documenting the environment of the hooks, time out behavior and so on, are general issues for every kind of hook. I am not saying this from the security standpoint (the fact that you can invoke receive-pack and that you can write into the update hooks means you already have control over that repository), but to help hook writers to avoid making mistakes. For example, I offhand cannot tell what happens if the hook tries to read from its standard input. Also what happens if the hook does not return but sleeps forever in a loop? Do we want to somehow time it out? I think It is hooks' responsibility to time itself out is an acceptable answer here, but if that is the case it had better be documented. I think that a time out is not needed here: as the hook is called synchronous, git-receive-pack won't return without until the hook terminated. And that is visible to the user, i.e. he would see that there is something wrong. I think I've seen this fork -- exec -- for loop with waitpid pattern repeated number of times in the code. This is no coincidence ;-) I copied it from inside the same file. But the behavior is another: If the hook goes wrong, I do not want for git-receive-pack to die. Would it be feasible to make them into a single library-ish function and call it from here and other existing places? Probably. Another thing you may want to consider is to do this hook processing before and/or after processing all the refs. A hook might want to know what the entire set of refs are that are being updated, and may not have enough information if it is invoked once per ref. Do you have a use case? At least, it would make things more complex. Josef - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
Linus Torvalds [EMAIL PROTECTED] writes: No I'm not. Try all the machines behind my firewall. Ah, that's true. Do you push into them? Let's yank out the update_server_info() call when Josef's patch can handle a single hook call at the end of the run, in addition to one call per each ref getting updated. - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
On Sun, 31 Jul 2005, Junio C Hamano wrote: Linus Torvalds [EMAIL PROTECTED] writes: No I'm not. Try all the machines behind my firewall. Ah, that's true. Do you push into them? Yup, I do. I have this thing that I don't do backups, but I end up having redundancy instead, so I keep archives on my own machines and inside the private osdl network, for example. Also, I suspect that anybody who uses the CVS model with git - ie a central repository - is not likely to export that central repository any way: it's the crown jewels, after all. Open source may not have that mindset, but I'm thinking of how I was forced to use CVS at Transmeta, for example: the machine that had the CVS repo was certainly supposed to be very private indeed. In the central repo model you have another issue - you have potentially parallell pushes to different branches with no locking what-so-ever (and that's definitely _supposed_ to work), and I have this suspicion that the update for dumb servers code isn't really safe in that setting anyway. I haven't checked. Linus - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
Hi, On Sun, 31 Jul 2005, Junio C Hamano wrote: Let's yank out the update_server_info() call when Josef's patch can handle a single hook call at the end of the run, in addition to one call per each ref getting updated. How about executing update_server_info() if no hook was found? That way, it can be turned off by an empty hook, but is enabled in standard settings. Ciao, Dscho - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added hook in git-receive-pack
Linus Torvalds [EMAIL PROTECTED] writes: In the central repo model you have another issue - you have potentially parallell pushes to different branches with no locking what-so-ever (and that's definitely _supposed_ to work), and I have this suspicion that the update for dumb servers code isn't really safe in that setting anyway. I haven't checked. You are absolutely right. It should grab some sort of lock while it does its thing (would fcntl(F_GETLK) be acceptable to networked filesystem folks?). I have one question regarding the hooks. We seem to prefer avoiding system and roll our own. Is there a particular reason, other than bypassing the need to quote parameters for shell? - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
