Re: fedora-git-commit-mail-hook: Fix command for finding previous tag

[Todd Zullinger]

I wrote:
>   configs/system/fedora-git-commit-mail-hook |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
>  New commits:
>  commit 226e06441f2fab7c7c52609834e86c236910e963
>  Author: Todd Zullinger <tmz(a)pobox.com&gt;
>  Date:   Sun Jun 21 03:34:20 2009 +0000
>
>      fedora-git-commit-mail-hook: Fix command for finding previous tag
>
>      The spacewalk maintainers reported problems when pushing tags[1].  The
>      root of the problem appears to be in determining the previous tag,
>      which used 'git describe' piped to sed.  The sed call is better
>      replaced with the --abbrev=0 option to 'git describe' and should
>      correct the problems noted by the spacewalk maintainers.
>
>      This fix was made to the upstream update hook in c47e6a4[2], and was
>      required because the format of git describe changed.
>
>      It is worth noting that the upstream update hook has been completely
>      re-worked.  Email notification has been removed from the update
>      hook[3] and implemented in the post-receive hook[4]. 
I think we may want to look at converting from our use of the update
hook to the post-receive hook for sending commit notifications.
Upstream moved the mail notification bits from the update hook to a
new post-receive hook only a few weeks after Jeremy added the initial
fedora-git-commit-mail-hook.  So what we're running now isn't
something which will receive any attention or bug fixes from upstream,
unfortunately.  The reasoning for upstream moving mail notification is
stated in the the commit message for 46d409d:

     The update hook's only job is to decide is a particular update
     is allowed or not.  It was not the right place to send out
     update notification e-mails from to begin with, as the final
     stage of updating refs can fail after this hook runs.

>      [1] https://fedorahosted.org/fedora-infrastructure/ticket/1362
>      [2] http://git.kernel.org/?p=git/git.git;a=commitdiff;h=c47e6a4
>      [3] http://git.kernel.org/?p=git/git.git;a=commitdiff;h=46d409d
>      [4] http://git.kernel.org/?p=git/git.git;a=commitdiff;h=4557e0d
>
>  diff --git a/configs/system/fedora-git-commit-mail-hook
> b/configs/system/fedora-git-commit-mail-hook
>  index 85359ca..e80a461 100644
>  --- a/configs/system/fedora-git-commit-mail-hook
>  +++ b/configs/system/fedora-git-commit-mail-hook
>  @@ -82,7 +82,7 @@ then
>   			git cat-file tag $3 | sed -n '5,$p'
>   			echo
>   		fi
>  -		prev=$(git describe "$3^" | sed 's/-g.*//')
>  +		prev=$(git describe --abbrev=0 "$3^")
>   		# the first tag in a repo will yield no $prev
>   		if [ -z "$prev" ]; then
>   			echo "Changes since the dawn of time:" 
-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The man who is a pessimist before forty-eight knows too much; the man
who is an optimist after forty-eight knows too little.
    -- Mark Twain

fedora-git-commit-mail-hook: Fix command for finding previous tag

[Jeremy Katz]

On Monday, June 22 2009, Todd Zullinger said:
[snip]
>  I think we may want to look at converting from our use of the update
>  hook to the post-receive hook for sending commit notifications.
>  Upstream moved the mail notification bits from the update hook to a
>  new post-receive hook only a few weeks after Jeremy added the initial
>  fedora-git-commit-mail-hook.  So what we're running now isn't
>  something which will receive any attention or bug fixes from upstream,
>  unfortunately.  The reasoning for upstream moving mail notification is
>  stated in the the commit message for 46d409d: 
That sounds sensible.  I have no problem with us changing, although I
won't have the time to sit down and make it happen any time in the next
couple of weeks.  If someone can get to it before, I'm more than happy
to look over it.

Alternately, I can stick on my queue for July :)

Jeremy

fedora-git-commit-mail-hook: Fix command for finding previous tag

[Todd Zullinger]

Jeremy Katz wrote:
>  That sounds sensible.  I have no problem with us changing, although
>  I won't have the time to sit down and make it happen any time in the
>  next couple of weeks.  If someone can get to it before, I'm more
>  than happy to look over it. 
I'm happy to work on this.  I mostly wanted to check that the idea was
agreeable first.  I'll try not to come up with too many suggestions
that cause other people work -- other than the work of reading my long
emails... :)

I an ideal world, we would use the upstream post-receive-email
unmodified.  I'm not sure if that will be possible or not, as we'd
want to pipe mail to the send-unicode-email.py script, rather than
just through sendmail.

A patch to the upstream script to allow setting the path to 'sendmail'
might be in order.  And at the least, a simple puppet exec could fix
that one part and leave us otherwise using the upstream hook.

My biggest concern is that the format of the notifications has changed
a good bit since the current fedora-git-commit-mail-hook was added.

An example of what our installed hook produces:

    https://fedorahosted.org/pipermail/spacewalk-commits/2009-June/001536.html

An example of what the upstream post-receive-email hook produces:

    http://sourceware.org/ml/glibc-cvs/2009-q2/msg00403.html

The upstream hook should produce more useful results for various
operations, adding tags, branches, and such.  But as with any change,
some people are bound to not like it.  (I admit that I'm not all that
fond of the output for the simple case of pushing a new commit, as the
summary is added after the diff.)

Unfortunately, while the change I made yesterday to the current hook
fixed some of the problems the spacewalk.git repo has, it didn't fix
them all.  So I'll be spending a little more time with that to see if
I can locate the remaining issues.  It might be an opportunity to
enable the new post-receive hook for the reporter and see if that
output is better.

BTW, Jeremy, do you happen to recall just what the origin of the
current script is?  I was trying to find a common ancestor that I
could use to cherry-pick patches from git.git into.

-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Procrastination is the art of keeping up with yesterday.
    -- Don Marquis

fedora-git-commit-mail-hook: Fix command for finding previous tag

[Jeremy Katz]

On Tuesday, June 23 2009, Todd Zullinger said:
>  Jeremy Katz wrote:
>  > That sounds sensible.  I have no problem with us changing, although
>  > I won't have the time to sit down and make it happen any time in the
>  > next couple of weeks.  If someone can get to it before, I'm more
>  > than happy to look over it.
>  
>  I'm happy to work on this.  I mostly wanted to check that the idea was
>  agreeable first.  I'll try not to come up with too many suggestions
>  that cause other people work -- other than the work of reading my long
>  emails... :) 
Awesome.  Reading and replying to emails is what I do best at this point
;)
 
>  I an ideal world, we would use the upstream post-receive-email
>  unmodified.  I'm not sure if that will be possible or not, as we'd
>  want to pipe mail to the send-unicode-email.py script, rather than
>  just through sendmail.
>  
>  A patch to the upstream script to allow setting the path to 'sendmail'
>  might be in order.  And at the least, a simple puppet exec could fix
>  that one part and leave us otherwise using the upstream hook. 
That sounds sensible

>  My biggest concern is that the format of the notifications has
> changed
>  a good bit since the current fedora-git-commit-mail-hook was added. 
[snip]
>  The upstream hook should produce more useful results for various
>  operations, adding tags, branches, and such.  But as with any change,
>  some people are bound to not like it.  (I admit that I'm not all that
>  fond of the output for the simple case of pushing a new commit, as the
>  summary is added after the diff.) 
Hmmm, most of the changes I think are harmless aside from people getting
thrown off the first time.  I do agree that the summary being at the
bottom is a little bit more bothersome than most of the changes.  Also,
it looks like we'd lose the X-Git-Module header which is sort of
important for at least some people.

>  BTW, Jeremy, do you happen to recall just what the origin of the
>  current script is?  I was trying to find a common ancestor that I
>  could use to cherry-pick patches from git.git into. 
The current script was originally running on git.freedesktop.org and
written by Carl Worth, then hijacked and tweaked more than I would have
liked by me to fit our needs

Jeremy