threads / discuss / 23199

CIA hook for contrib/

Subject: CIA hook for contrib/

## tl;dr

8 messages between Mar 27, 2010 and Apr 21, 2010.

replies: 7people: 3as markdown or json

Eric Raymond· Mar 27, 2010, 10:26 UTC · lore
Summary:
* The CIA hooks for git are presently dusty and somewhat broken.
* I am requesting that they be given a home in git contrib/.
* I am supplying working, updated versions.
* I will accept maintainance responsibility if required.

Many listmembers will be aware of CIA.vc, a useful service that echoes commits from open-source projects to IRC channels and maintains various interesting statistics on commit activity. Under git, it is supported by one of two hook scripts: ciabot.pl and ciabot.pl.

In the course of migrating the GPSD project from Subversion to git, I discovered that the 2006 version of ciabot.sh available from CIA.vc is broken. It requires, but does not have, a PATH adjustment to cope with the fact that various git tools it uses now go in a commands directory private to git. The 2008 version of ciabot.sh made available on the git wiki also has no PATH adjustment.

Upon investigating further, I find that ciabot.sh seems to be a derivative of ciabot.pl, which is a dangling bit of the moribund Cogito project. I have been unable to get responses from the authors of either ciabot.sh or ciabot.pl.

A fixed and working version in shell is enclosed. It has some advantages over the 2006 and 2008 versions:

* It uses only POSIX features.  Thus, it will work with Debian/Ubuntu
  dash and BSD's non-bash shells.
* It's faster, because it uses a lighter-weight shell.
* It's much closer to being autoconfiguring.  In the normal case, all
  the person installing an instance need supply is the project name.
* It is considerably better documented.
* It can support cgit as well as gitweb installations.

I am also enclosing a rewrite in Python. This has a serious functional advantage over either Perl or sh; the batteries-included effect of the Python libraries means it's not dependent on things like the path location of sendmail or whether the hosting site will allow it to run wget.

I think the broad usage of CIA.vc justifies adding one of these as an official ciabot hook to git contrib/, and am so proposing. Should it be required, I will accept maintainance responsibility; I expect the amount of effort involved to be minimal.

Both versions have been live-tested on the GPSD project repo. The Python version is now in production use there.

Code is enclosed.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>


#!/usr/bin/env python
# Distributed under the terms of the GNU General Public License v2
# Copyright (c) 2010 Eric S. Raymond <esr@thyrsus.com>
#
# This script contains porcelain and porcelain byproducts.
# It's Python because the Python standard libraries avoid portability/security
# issues raised by callouts in the ancestral Perl and sh scripts.  It should
# be compatible back to Python 2.1.5.
#
# It is meant to be run either on a post-commit hook or in an update
# hook:
#
# post-commit: It queries for current HEAD and latest commit ID to get the
# information it needs.
#
# update: You have to call it once per merged commit:
#
#       refname=$1
#       oldhead=$2
#       newhead=$3
#       for merged in $(git rev-list ${oldhead}..${newhead} | tac) ; do
#               /path/to/ciabot.py ${refname} ${merged}
#       done
#
# Note: this script uses mail, not XML-RPC, in order to avoid stalling
# until timeout when the XML-RPC server is down. 
#
# Call with -n to see the notification mail dumped to stdout rather
# than shipped to CIA.
#
import os, sys, commands, socket, urllib
#
# The project as known to CIA. You will want to change this:
#
project="GPSD"

#
# You may not need to change these:
#

# Name of the repository.
# You can hardwire this to make the script faster.
repo = os.path.basename(os.getcwd())

# Fully-qualified domain name of this host.
# You can hardwire this to make the script faster.
host = socket.getfqdn()

# Changeset URL prefix for your repo: when the commit ID is appended
# to this, it should point at a CGI that will display the commit
# through gitweb or something similar. The default will probably
# work if you have a typical gitweb/cgit setup.
#
#urlprefix="http://%(host)s/cgi-bin/gitweb.cgi?p=%(repo)s;a=commit;h="%locals()
urlprefix="http://%(host)s/cgi-bin/cgit.cgi/%(repo)s/commit/?id="%locals()

#
# No user-serviceable parts below this line:
#

def do(command):
    return commands.getstatusoutput(command)[1]

# Addresses for the e-mail. The from address is a dummy, since CIA
# will never reply to this mail.
fromaddr = "CIABOT-NOREPLY@" + host
toaddr = "cia@cia.vc"

# Identify the generator script.
# Should only change when the script itself has a new home
generator="http://www.catb.org/~esr/ciabot.sh"

# Should add to the command path both places sendmail is likely to lurk, 
# and the git private command directory.
os.environ["PATH"] += ":/usr/sbin/:" + do("git --exec-path")

# Call this script with -n tp dump the notification mail to stdout
mailit = True 
if sys.argv[1] == '-n':
    mailit = False
    sys.argv.pop(1)

# Script wants a reference to head followed by the commit ID to notify about. 
if len(sys.argv) == 1:
    refname = do("git symbolic-ref HEAD 2>/dev/null")
    merged = do("git rev-parse HEAD")
else:
    refname = sys.argv[1]
    merged = sys.argv[2]

# This tries to turn your gitwebbish URL into a tinyurl so it will take up
# less space on the IRC notification line. You can tweak the tinyfier
# service if you need to.
tinyifier = "http://tinyurl.com/api-create.php?url="
try:
    url = open(urllib.urlretrieve(tinyifier + urlprefix + merged)[0]).read()
except:
    url = urlprefix + merged

refname = os.path.basename(refname)
gitver = do("git --version").split()[0]	# Git version number

rev = do("git describe ${merged} 2>/dev/null") or merged[:12]
rawcommit = do("git cat-file commit " + merged)
files=do("git diff-tree -r --name-only '"+ merged +"' | sed -e '1d' -e 's-.*-<file>&</file>-'")

inheader = True
headers = {}
logmessage = ""
for line in rawcommit.split("\n"):
    if inheader:
        if line:
            fields = line.split()
            headers[fields[0]] = " ".join(fields[1:])
        else:
            inheader = False
    else:
        logmessage = line
        break
(author, ts) = headers["author"].split(">")
author = author.replace("<", "").split("@")[0].split()[-1]
ts = ts.strip()

out = '''\
<message>
  <generator>
    <name>CIA Shell client for Git</name>
    <version>%(gitver)s</version>
    <url>%(generator)s</url>
  </generator>
  <source>
    <project>%(project)s</project>
    <branch>%(repo)s:%(refname)s</branch>
  </source>
  <timestamp>%(ts)s</timestamp>
  <body>
    <commit>
      <author>%(author)s</author>
      <revision>%(rev)s</revision>
      <files>
        %(files)s
      </files>
      <log>%(logmessage)s %(url)s</log>
      <url>%(url)s</url>
    </commit>
  </body>
</message>
''' % locals()

message = '''\
Message-ID: <%(merged)s.%(author)s@%(project)s>
From: %(fromaddr)s
To: %(toaddr)s
Content-type: text/xml
Subject: DeliverXML

%(out)s''' % locals()

if mailit:
    import smtplib
    server = smtplib.SMTP('localhost')
    server.sendmail(fromaddr, [toaddr], message)
    server.quit()
else:
    print message

#End
Junio C Hamano· Mar 29, 2010, 06:14 UTC · re: Eric Raymond · lore

Re: CIA hook for contrib/

Eric Raymond <esr@thyrsus.com> writes:
> Summary:
> * The CIA hooks for git are presently dusty and somewhat broken.

Thanks; but I don't ship any ancient hooks, so this probably wants to be sent to whoever still have them displayed on their sites.

> * I am requesting that they be given a home in git contrib/.

I gave a quick read to the shell one, and I found it reasonable. Thanks for sharing.

One tiny nit is the "git --exec-path" bit, though. You don't seem to use any ancient "git-frotz" form, so it is not just unnecessary but is misleading. I'd recommend to just drop it.

Another tiny nit is that you _might_ want to make it fail loudly if somebody copies this out of contrib/ without changing $project properly, instead of spamming cia with bogus messages claiming that they pertain to the GPSD project.

Show 7 quoted lines
> I am also enclosing a rewrite in Python. This has a serious functional
> advantage over either Perl or sh; the batteries-included effect of the
> Python libraries means it's not dependent on things like the path
> location of sendmail or whether the hosting site will allow it to
> run wget.
> ...
> Code is enclosed.

Thanks. Could you send a signed-off patch that creates contrib/ciabot/ directory, and places these two files in there, with a README describing that you are the maintainer of these tools?

Eric Raymond· Mar 29, 2010, 09:04 UTC · re: Junio C Hamano · lore

Re: CIA hook for contrib/

Hm...this showed up about two minutes after I sent you private mail wondering why nobody had responded to my post. Sorry about that.

Junio C Hamano <gitster@pobox.com>:
Show 7 quoted lines
> Eric Raymond <esr@thyrsus.com> writes:
> 
> > Summary:
> > * The CIA hooks for git are presently dusty and somewhat broken.
> 
> Thanks; but I don't ship any ancient hooks, so this probably wants to be
> sent to whoever still have them displayed on their sites.
The ancient hooks are in scattered locations - one in the Cogito repo, another
on some random project site called alphine, and teo stale copies on CIA.vc.
I'm trying to getthese updated.
 
> One tiny nit is the "git --exec-path" bit, though.  You don't seem to use
> any ancient "git-frotz" form, so it is not just unnecessary but is
> misleading.  I'd recommend to just drop it.
I will test and drop that if possible.
 
> Another tiny nit is that you _might_ want to make it fail loudly if
> somebody copies this out of contrib/ without changing $project properly,
> instead of spamming cia with bogus messages claiming that they pertain to
> the GPSD project.
Good idea.  Will do.
Show 11 quoted lines
> > I am also enclosing a rewrite in Python. This has a serious functional
> > advantage over either Perl or sh; the batteries-included effect of the
> > Python libraries means it's not dependent on things like the path
> > location of sendmail or whether the hosting site will allow it to
> > run wget.
> > ...
> > Code is enclosed.
> 
> Thanks.  Could you send a signed-off patch that creates contrib/ciabot/
> directory, and places these two files in there, with a README describing
> that you are the maintainer of these tools?
I will do so.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
Petr Baudis· Apr 21, 2010, 10:10 UTC · re: Eric Raymond · lore

Re: CIA hook for contrib/

  Hi!
On Sat, Mar 27, 2010 at 06:26:32AM -0400, Eric Raymond wrote:
> Upon investigating further, I find that ciabot.sh seems to be a
> derivative of ciabot.pl, which is a dangling bit of the moribund
> Cogito project.  I have been unable to get responses from the authors
> of either ciabot.sh or ciabot.pl.
  Strange, I received no mail from you. When did you send it?
> A fixed and working version in shell is enclosed.
  It would have been easier to review if it was quotable. ;-)
(I have also looked at the current checked in version.)
  I'm happy that someone resurrected the hook script, thanks for that!
By the way, you seem to drop support for XML::RPC altogether while the
original ciabot.pl could use both. While XML::RPC requires another
dependency (not sure if plain Python installation can do it) and it can
time out in case of CIA server trouble (not sure how common these are
nowadays), the distinct advantage is that the commits will always end up
in correct order, while it seemed to be common that by mail, push of
multiple commits would reorder them randomly.
Show 7 quoted lines
> # update: You have to call it once per merged commit:
> #
> #       refname=$1
> #       oldhead=$2
> #       newhead=$3
> #       for merged in $(git rev-list ${oldhead}..${newhead} | tac) ; do
> #               /path/to/ciabot.bash ${refname} ${merged}
                  ^^^^^^^^^^^^^^^^^^^^
Obsolete .bash reference.
Show 6 quoted lines
> rev=$(git describe ${merged} 2>/dev/null)
> # ${merged:0:12} was the only bashism left in the 2008 version of this
> # script, according to checkbashisms.  Replace it with ${merged} here
> # because it was just a fallback anyway, and it's worth taking accepting
> # a longer fallback for faster execution and removing the bash deoendency.
> [ -z ${rev} ] && rev=${merged}

I personally find git-describe'd revspecs extremely ugly, unreadable and less practical than plain hex ids (maybe I'm weird?), it would be really nice to get a configuration choice between git describe and just:

	rev=$(echo "$merged" | cut -c 1-12)

(Not sure about the bashism rant since you can trivially just replace it with the cut.)

> # This discards the part of the author's address after @.
> # Might be nice to ship the full email address, if not
> # for spammers' address harvesters - getting this wrong
> # would make the freenode #commits channel into harvester heaven.

The comment is not accurate since that code seems to be doing a lot of other things as well.

I would personally prefer to have this configurable; I consider trying to protect your e-mail address against harvesters is a lost fight anyway and you'd be much better off just getting a good spam filter, rather than making yours and others' life harder by trying to fight in vain. But it's not too important for me since I can just disable this easily.

Show 5 quoted lines
> rawcommit=$(git cat-file commit ${merged})
> author=$(echo "$rawcommit" | sed -n -e '/^author .*<\([^@]*\).*$/s--\1-p')
> logmessage=$(echo "$rawcommit" | sed -e '1,/^$/d' | head -n 1)
> logmessage=$(echo "$logmessage" | sed 's/\&/&amp\;/g; s/</&lt\;/g; s/>/&gt\;/g')
> ts=$(echo "$rawcommit" | sed -n -e '/^author .*> \([0-9]\+\).*$/s--\1-p')

Your life would be much easier in both the shell and python script if you used something like:

	git log -1 '--pretty=format:%an <%ae>%n%at%n%s'
You would also get <ts> in the correct format, incl. timezone correction.
  Kind regards,
-- 
				Petr "Pasky" Baudis
http://pasky.or.cz/ | "Ars longa, vita brevis." -- Hippocrates
Eric Raymond· Apr 21, 2010, 15:51 UTC · re: Petr Baudis · lore

Re: CIA hook for contrib/

Petr Baudis <pasky@suse.cz>:
Show 7 quoted lines
> On Sat, Mar 27, 2010 at 06:26:32AM -0400, Eric Raymond wrote:
> > Upon investigating further, I find that ciabot.sh seems to be a
> > derivative of ciabot.pl, which is a dangling bit of the moribund
> > Cogito project.  I have been unable to get responses from the authors
> > of either ciabot.sh or ciabot.pl.
> 
>   Strange, I received no mail from you. When did you send it?
About three weeks ago now.  Sorry, it appears I fat-fingered your
address.  I did eventually hear from the author of the 2008 sh
version; he passed me the maintainer's baton on that one.
 
Show 8 quoted lines
>   I'm happy that someone resurrected the hook script, thanks for that!
> By the way, you seem to drop support for XML::RPC altogether while the
> original ciabot.pl could use both. While XML::RPC requires another
> dependency (not sure if plain Python installation can do it) and it can
> time out in case of CIA server trouble (not sure how common these are
> nowadays), the distinct advantage is that the commits will always end up
> in correct order, while it seemed to be common that by mail, push of
> multiple commits would reorder them randomly.

Ah, I did not know of that advantage, it wasn't documented anywhere. It's a sufficient reason to bring back XML-RPC support...and I have just done so in the Python version.

Show 10 quoted lines
> > # update: You have to call it once per merged commit:
> > #
> > #       refname=$1
> > #       oldhead=$2
> > #       newhead=$3
> > #       for merged in $(git rev-list ${oldhead}..${newhead} | tac) ; do
> > #               /path/to/ciabot.bash ${refname} ${merged}
>                   ^^^^^^^^^^^^^^^^^^^^
> 
> Obsolete .bash reference.
Fixed, thanks.
Show 8 quoted lines
> I personally find git-describe'd revspecs extremely ugly, unreadable and
> less practical than plain hex ids (maybe I'm weird?), it would be really
> nice to get a configuration choice between git describe and just:
> 
> 	rev=$(echo "$merged" | cut -c 1-12)
> 
> (Not sure about the bashism rant since you can trivially just replace it
> with the cut.)
Good point about use of cut; I'll do that.  
I've implemented a revformat variable, but defaulted it to 'describe'.
 
Show 5 quoted lines
> I would personally prefer to have this configurable; I consider trying
> to protect your e-mail address against harvesters is a lost fight anyway
> and you'd be much better off just getting a good spam filter, rather
> than making yours and others' life harder by trying to fight in vain.
> But it's not too important for me since I can just disable this easily.
But email name collisions within projects are vanishingly rare, so I
don't see a lot of benefit in publishing the FQDN.
 
Show 6 quoted lines
> Your life would be much easier in both the shell and python script if
> you used something like:
> 
> 	git log -1 '--pretty=format:%an <%ae>%n%at%n%s'
> 
> You would also get <ts> in the correct format, incl. timezone correction.

Good point. I inherited that nasty code; perhaps this facility did not exist when it was written.

-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
Petr Baudis· Apr 21, 2010, 21:04 UTC · re: Eric Raymond · lore

Re: CIA hook for contrib/

Thanks for doing all the changes!
On Wed, Apr 21, 2010 at 11:51:49AM -0400, Eric Raymond wrote:
Show 9 quoted lines
> Petr Baudis <pasky@suse.cz>:
> > I would personally prefer to have this configurable; I consider trying
> > to protect your e-mail address against harvesters is a lost fight anyway
> > and you'd be much better off just getting a good spam filter, rather
> > than making yours and others' life harder by trying to fight in vain.
> > But it's not too important for me since I can just disable this easily.
> 
> But email name collisions within projects are vanishingly rare, so I
> don't see a lot of benefit in publishing the FQDN.
That's a good point, I guess I was just ranting in general. ;-)
Show 9 quoted lines
> > Your life would be much easier in both the shell and python script if
> > you used something like:
> > 
> > 	git log -1 '--pretty=format:%an <%ae>%n%at%n%s'
> > 
> > You would also get <ts> in the correct format, incl. timezone correction.
> 
> Good point.  I inherited that nasty code; perhaps this facility did not
> exist when it was written.

It most probably wasn't, the original perl script in particular was written at the very dawn of time from git history perspective.

-- 
				Petr "Pasky" Baudis
http://pasky.or.cz/ | "Ars longa, vita brevis." -- Hippocrates
Eric Raymond· Apr 21, 2010, 21:55 UTC · re: Petr Baudis · lore

Re: CIA hook for contrib/

Petr Baudis <pasky@suse.cz>:
> Thanks for doing all the changes!

Thanks for the review. You pushed me into doing two things I was naggingly aware I ought to do but hadn't gotten around to: (1) XML-RPC support from the Python version (alas, it's too hard from shell), and (2) fixing the scripts so that they can be entirely informed by config variables, no modifications or command-line switches required.

You can read about the current state of play at 
    <http://www.catb.org/esr/ciabot/>.
I've spent the last couple of hours testing
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
Eric Raymond· Apr 21, 2010, 22:00 UTC · re: Eric Raymond · lore

Re: CIA hook for contrib/

Eric Raymond <esr@thyrsus.com>:
> I've spent the last couple of hours testing
Sentence truncated...

I've spent the last couple of hours testing and epect to mail a patch to the list shortly. Well, after I get dinner, now.

-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

← back to recent threads