RFC: using AdaCore's git hooks for binutils-gdb.git ...

Joel Brobecker brobecker@adacore.com
Fri Nov 28 13:58:00 GMT 2014


I would like to propose the use of AdaCore's git hooks scripts
for handling everything that happens when updates are pushed
to sourceware's binutils-gdb.git repository.

While the scripts currently in place are serving us well, I think
there are a number of features I'd like to have available:

  - Send one email per commit, rather than one per push
    (and have the subject of the email be the subject of
    the commit)

  - Allow the use of specific policies on a per-branch basis
    (ie - reject merges in release branches, for instance).

Our scripts also correctly handle new references (new tags, new
branches), as well as git notes. It is also Gerrit-ready,
although my understanding is that there are no plans to use it
within either project.

Of interest, in terms of features:

  - Allow the use of pre-commit checking scripts that can verify
    both files being modified, as well as the commit's revision log;

  - Extensive configurability through a git-config-type file
    which is itself under configuration management, and therefore
    adjustable by all contributors, not just the admins with
    login access to sourceware.

    I've made a copy of our Users' Guide available at:

  - A "diff" of the changes is also included in the commit-email.

  - It disallows non-fast-forward changes (Eg. rebases) by default,
    but gives the option to allow it on designated branched.

The scripts are written in Python, and are pretty fast for typical

There is a testsuite with 100% code coverage (some lines excluded,
but those are very rare and related to sending emails, which we
do not want to do during testing). We have been using them at
AdaCore for a few years now, with a very low number of issues
being reported, thanks to the fairly extensive testsuite.

If people are interested in switching over to those scripts,
there is still a bit of work for me to do:

  1. Make the scripts publicly available - I will probably use
     something like GitHub, or the open-do source forge.

  2. Some adjustements will be needed in order to accomodate
     the peculiarities of the binutils-gdb.git repo - the one
     that comes to mind is the shared nature, with 2 mailing-lists
     instead of one for the entire project. This should be fairly
     easy to add by allowing the "mailinglist" configuration to
     provide a script instead of a list of email addresses.

  3. Some other adjustments to remove the use of an AdaCore Python
     library, which is publicly available, but not necessarly
     widely deployed.

     The testsuite is using gnatpython as the testsuite back-bone,
     so running it will continue requiring it.

     In the same registry, sourceware's version of Python is slightly
     older, so that may force us to make some adjustments as well.

This is why I am feeling the waters before going ahead. If people
prefer staying with the current scripts, then I won't bother!

Lastly, a few example of the emails you would get:

| Date: Fri, 14 Nov 2014 13:29:21 +0100 (CET)
| From: Joel Brobecker <brobecke@adacore.com>
| Subject: [binutils-gdb] Fix tiny GCS violatiton in
|         ada_is_redundant_range_encoding.
| To: [...]
| commit 246f3fa961a779b90bebd27d4c9e426f3c76c003
| Author: Joel Brobecker <brobecker@adacore.com>
| Date:   Fri Nov 14 16:28:26 2014 +0400
|     Fix tiny GCS violatiton in ada_is_redundant_range_encoding.
|     gdb/ChangeLog:
|             * ada-lang.c (ada_is_redundant_range_encoding): Add missing
|             second space at end of sentence in comment.
| Diff:
| ---
|  gdb/ada-lang.c | 2 +-
|  1 file changed, 1 insertion(+), 1 deletion(-)
| diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
| index ab4e0a3..e8f004e 100644
| --- a/gdb/ada-lang.c
| +++ b/gdb/ada-lang.c
| @@ -8672,7 +8672,7 @@ ada_is_redundant_range_encoding (struct type *range_type,
|    if (bounds_str == NULL)
|      return 0;
| -  n = 8; /* Skip "___XDLU_". */
| +  n = 8; /* Skip "___XDLU_".  */
|    if (!ada_scan_number (bounds_str, n, &lo, &n))
|      return 0;
|    if (TYPE_LOW_BOUND (range_type) != lo)

There isn't much to say in addition to the above, except maybe
describe a bit the start of the email's subject. It is meant to
indicate the name of the repository and the branch being updated.
The hooks provide that info using the following syntax....


... where "/name-of-branch" gets ommitted for branch master
(which is just a convention). So, for instance commits to master
would send emails where the subject's prefix would be as in the
example above, whereas subject for commmits pushed to the
gdb-7.8-branch, for instance, would use [binutils-gdb/gdb-7.8-branch]
as the prefix instead.

When new tags get created, an email notifying us of its creating
will look like the following:

| From: Test Suite <testsuite@example.com>
| To: testsuite@example.com
| Bcc: file-ci@gnat.com
| Subject: [repo] Created tag v0.1
| X-Act-Checkin: repo
| X-Git-Author: Test Suite <testsuite@example.com>
| X-Git-Refname: refs/tags/v0.1
| X-Git-Oldrev: 0000000000000000000000000000000000000000
| X-Git-Newrev: c4c1e91cddc3d48a2aab7d454bc6537149f37dd8
| The unsigned tag 'v0.1' was created pointing to:
|  8b9a0d6... New file: a.
| Tagger: Joel Brobecker <brobecker@adacore.com>
| Date: Tue Jun 26 07:51:14 2012 -0700
|     This is a new tag.

This is from the testsuite, and shows something interesting
I haven't mentioned before, which is the use of private email
headers "X-Act-Checkin:" (name of repo), as well as "X-Git-..."
which provide some easily usable info for email processing/sorting.

When creating a branch, a first email notifying us of the branch
creation will be sent, and looks like this:

| From: Test Suite <testsuite@adacore.com>
| To: git-hooks-ci@example.com
| Subject: [repo] Created branch 'release-0.1-branch'
| X-Act-Checkin: repo
| X-Git-Author: Test Suite <testsuite@adacore.com>
| X-Git-Refname: refs/heads/release-0.1-branch
| X-Git-Oldrev: 0000000000000000000000000000000000000000
| X-Git-Newrev: dcc477c258baf8cf59db378fcc344dc962ad9a29
| The branch 'release-0.1-branch' was created pointing to:
|  dcc477c... New file b, add reference to it from file a.

If this branch brings new commits (ie, commits that did not exist
in any pre-existing branch), then individual emails will be sent
for each of these new commits, just as we do when updating a branch.

Merges are also handled. As always, individual commit emails
are only sent for commits that are new in the repository.
If the merge brings in some commits that were already part
of another branch, a "cover email" gets sent listing all
the commits being added, with a "(*)" telling us that no email
will be sent for those commits. Here is an example of such

| From: Test Suite <testsuite@adacore.com>
| To: git-hooks-ci@example.com
| Subject: [repo] (3 commits) Merge topic branch fsf-head.
| X-Act-Checkin: repo
| X-Git-Author: Test Suite <testsuite@adacore.com>
| X-Git-Refname: refs/heads/master
| X-Git-Oldrev: 33e7556e39b638aa07f769bd894e75ed1af490dc
| X-Git-Newrev: ffb05b4a606fdb7b2919b209c725fe3b71880c00
| The branch 'master' was updated to point to:
|  ffb05b4... Merge topic branch fsf-head.
| It previously pointed to:
|  33e7556... Add new file b.
| Diff:
| Summary of changes (added commits):
| -----------------------------------
|   ffb05b4... Merge topic branch fsf-head.
|   b4bfa84... New file `c', update README accordingly. (*)
|   6d62250... New file README. Update a. (*)
| (*) This commit exists in a branch whose name matches
|     the hooks.noemail config option. No separate email
|     sent.
| commit ffb05b4a606fdb7b2919b209c725fe3b71880c00
| Merge: 33e7556 b4bfa84
| Author: Joel Brobecker <brobecker@adacore.com>
| Date:   Thu Dec 20 13:50:25 2012 +0400
|     Merge topic branch fsf-head.
|     ChangeLog:
|             * a: Add stuff.
|             * c: New file.
|             * README: New file.
| commit b4bfa84ef414162de60ff93005c5528f68b4c755
| Author: Joel Brobecker <brobecker@adacore.com>
| Date:   Thu Dec 20 13:41:24 2012 +0400
|     New file `c', update README accordingly.
|     README new refers to file `c'.
|     ChangeLog:
|             * c: New file.
|             * README: Add reference to new file `c'.
| commit 6d62250fdaed631cb170c0fc19c338accdba14ec
| Author: Joel Brobecker <brobecker@adacore.com>
| Date:   Thu Dec 20 13:40:33 2012 +0400
|     New file README. Update a.
|     Some revision history info.

There are a number of other examples I could show, but I believe
the above should illustrate the majority of our use.

Before I ask what you guys think about this, I would like to quickly
add one thing: let's not allow the discussion to drag into details.
While I don't intend to use a "use it or leave it" attitude, I do not
want the discussion to get bogged down because of enhancement
suggestions and requests. If something is blocking the adoption
of those scripts, then OK. Otherwise, please let's focus on getting
those into production, AND THEN look at possible improvements.
By then, access to the repo should be avaialble, and we can then
adopt a free and open approach towards its maintenance [1].


[1]: I only have a few days allocated each year for working on
     those hooks.

More information about the Binutils mailing list