Example post-receive git hook is dangerous

[strk@…]

The example post-receive hook for explicit sync of git repository found on https://trac.edgewall.org/wiki/TracRepositoryAdmin#Git is dangerous because it invokes trac-admin for ALL the commits upon pushing a new branch, even if the new branch has exactly the same history of an existing one, and is broken in that *dropping* a branch results in an error.

The current example code is as follows:

#!/bin/sh
tracenv=/path/to/env     # change with your Trac environment's path
repos=                   # change with your repository's name
while read oldrev newrev refname; do
    if [ "$oldrev" = 0000000000000000000000000000000000000000 ]; then
        git rev-list --reverse "$newrev" --
    else
        git rev-list --reverse "$newrev" "^$oldrev" --
    fi | xargs trac-admin "$tracenv" changeset added "$repos"
done

A saner version would be (tested):

#!/bin/sh
tracenv=/path/to/env     # change with your Trac environment's path
repos=                   # change with your repository's name
while read oldrev newrev refname; do
    if expr "$oldrev" : '0*$' >/dev/null; then
        branches=`git for-each-ref --format="%(refname)" | grep -vw "$refname"`
        git rev-list --reverse "$newrev" --not $branches --
    elif expr "$newrev" : '0*$' >/dev/null; then
        : # do nothing upon removing a branch
    else
        git rev-list --reverse "$newrev" "^$oldrev" --
    fi | xargs --no-run-if-empty trac-admin "$tracenv" changeset added "$repos"

done

Using the current example in production resulted in adding ~4000 commits at once, and for each commit containing a ticket reference sending out email notifications to all ticket subscribers. Pretty huge flood…

[Ryan J Ollos]

See also #10730. If the script in comment:11:ticket:10730 also fixes the behavior we should just add that script to /contrib and update the documentation.

[strk@…]

In our case we also want to send an email for every changeset, so using an external script like the one you reference won't work as it would consume the standard input.

[Jun Omae]

The proposed script has another issue, which is ignoring new commits in the new branch.

[anonymous]

It does not, was tested

[Jun Omae]

Ah, sorry. I've misunderstood.

Another issue in the example and the proposed script: It wouldn't work as expected when multiple branches are pushed.

[strk@…]

I did not test pushing multiple branches but I believe in that case the hook would get multiple lines on stdin (one per pushed branch). For each line, only the commits that are exclusively in the pushed branch are added to trac, which I guess would mean that pushing 2 branches with the same new commits would get no trac-admin call at all. If that's confirmed the only way out of it would be reading all of stdin before starting the work.

[Ryan J Ollos]

Closing as a duplicate of #10730.