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

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

[Joel Brobecker]

Hello,

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:
    https://sourceware.org/gdb/wiki/proposed/git-hooks/UsersGuide

  - 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
pushes.

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.
     https://forge.open-do.org/projects/gnatpython

     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....

    [name-of-repo/name-of-branch]

... 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
email:

| 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].

-- 
Joel

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

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

[H.J. Lu]

On Fri, Nov 28, 2014 at 5:57 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> Hello,
>
> 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)

I like it.

>   - 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,

Does it handle removing a branch

# git push origin :foo

I used to get some extra messages.

> 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:
>     https://sourceware.org/gdb/wiki/proposed/git-hooks/UsersGuide
>
>   - 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
> pushes.
>
> 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.
>      https://forge.open-do.org/projects/gnatpython
>
>      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....

Do we need to show the whole diff in the commit email?  I think
a commit URL should be sufficient.  Binutils-gdb commit
can have a very large commit diff due to generated files.

>     [name-of-repo/name-of-branch]
>
> ... 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
> email:
>
> | 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].
>
> --
> Joel
>
> [1]: I only have a few days allocated each year for working on
>      those hooks.
> --
> Joel



-- 
H.J.

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

[Joel Brobecker]

> >   - 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,
> 
> Does it handle removing a branch
> 
> # git push origin :foo
> 
> I used to get some extra messages.

Yes, you get an email notifying you of the branch being deleted,
and what commit the branch was pointing to before the removal
of that branch. In addition, if any commits are "lost" (no longer
accessible from any remaining branch or other reference), then
a list of the commits that are no longer on any branch is included
as well.

Here is an example of what you'd get in the simple case:

| From: Test Suite <testsuite@adacore.com>
| To: git-hooks-ci@example.com
| Subject: [repo] Deleted branch old-branch
| X-Act-Checkin: repo
| X-Git-Author: Test Suite <testsuite@adacore.com>
| X-Git-Refname: refs/heads/old-branch
| X-Git-Oldrev: cc8d2c2637bda27f0bc2125181dd2f8534d16222
| X-Git-Newrev: 0000000000000000000000000000000000000000
|
| The branch 'old-branch' was deleted.
| It previously pointed to:
|
|  cc8d2c2... Modify `c', delete `b'.

> Do we need to show the whole diff in the commit email?  I think
> a commit URL should be sufficient.  Binutils-gdb commit
> can have a very large commit diff due to generated files.

No such option is available at the moment.

Personally, I wouldn't like having a URL, at least not instead
of a diff.  I think having a diff is much more useful and immediate,
as I use those as a chance to review post commit as well, or verify
that what's committed is what was discussed on the list.

But for large commits, there is a limit to the maximum size of
the email, after which the email is truncated.

-- 
Joel

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

[Joel Brobecker]

> In addition, if any commits are "lost" (no longer
> accessible from any remaining branch or other reference), then
> a list of the commits that are no longer on any branch is included
> as well.

Here is an example of what the email looks like:

| From: Test Suite <testsuite@adacore.com>
| To: git-hooks-ci@example.com
| Subject: [repo] Deleted branch topic/experiment1
| X-Act-Checkin: repo
| X-Git-Author: Test Suite <testsuite@adacore.com>
| X-Git-Refname: refs/heads/topic/experiment1
| X-Git-Oldrev: a60540361d47901d3fe254271779f380d94645f7
| X-Git-Newrev: 0000000000000000000000000000000000000000
|
| The branch 'topic/experiment1' was deleted.
| It previously pointed to:
|
|  a605403... Updated a.
|
| Diff:
|
| !!! WARNING: THE FOLLOWING COMMITS ARE NO LONGER ACCESSIBLE (LOST):
| -------------------------------------------------------------------
|
|   a605403... Updated a.
|
| commit a60540361d47901d3fe254271779f380d94645f7
| Author: Joel Brobecker <brobecker@adacore.com>
| Date:   Fri Apr 27 13:08:29 2012 -0700
|
|     Updated a.
|
|     Just added a little bit of text inside file a.
|     Thought about doing something else, but not really necessary.

-- 
Joel

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

[Joseph Myers]

On Fri, 28 Nov 2014, H.J. Lu wrote:

> Do we need to show the whole diff in the commit email?  I think
> a commit URL should be sufficient.  Binutils-gdb commit
> can have a very large commit diff due to generated files.

For glibc we show the diff in the email to glibc-cvs, but *not* in the 
Bugzilla update (I'm not sure if the Bugzilla update works via sending 
emails to each bug whose number was extracted from the commit message, or 
by some other interface - but in any case, different messages are used for 
the two cases).  Obviously you'd want to make sure that the patterns used 
to extract bug numbers are compatible with all those commonly used for 
binutils and GDB at present.

In the early days of glibc git, sending the diff did result in at least 
one 500MB email to glibc-cvs when the hook decided some branch commit 
required the whole history (or at least the whole source tree) to be 
emailed out, but I haven't seen such problems lately.

I don't know if binutils-gdb uses the whitespace checks that glibc does 
(but again, you'd probably want to be compatible with the existing logic).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Sergio Durigan Junior]

On Friday, November 28 2014, Joel Brobecker wrote:

> Hello,
>
> 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.

Joel,

I definitely like it a lot.  So, from my side, you have the green light
(and also my offer to help deploying the scripts, if you need).

Just a question: AFAIU, those scripts will only affect what we get in
the mailing lists (gdb-cvs and binutils-cvs), right?  Will we see any
change in the messages that are sent to our Bugzilla?

Other than that, thank you very, very much for doing that.

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

[Joel Brobecker]

> I definitely like it a lot.  So, from my side, you have the green light
> (and also my offer to help deploying the scripts, if you need).

Thanks!

> Just a question: AFAIU, those scripts will only affect what we get in
> the mailing lists (gdb-cvs and binutils-cvs), right?  Will we see any
> change in the messages that are sent to our Bugzilla?

Indeed, we're going to have do so some adaptation somewhere
to make sure that the diffs do not make it to bugzilla.
Similarly, we'll need to make it work with the IRC bot.
I don't know how that part works yet (but I think I have access
to the scripts, so it should be easy to figure out).

-- 
Joel

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

[Pedro Alves]

On 11/28/2014 02:57 PM, Joel Brobecker wrote:

> 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:

I've never even looked at the sourceware scripts, so implementation
wise, whatever you prefer works for me.

The new features sound awesome.  In particular, I'm looking
forward to meaningful email subjects and diffs in commit emails.

Thanks a lot for doing this.

Pedro Alves

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

[Joel Brobecker]

Hello everyone,

Just a quick update on this. I've started working on adapting
AdaCore's git hooks. I was planning on making them accessible
somewhere this weekend, but the weekend is almost over and
I am too tired to do it now. So I'll probably do it first thing
next weekend.

In terms of the adaptations I've been making:
  - Added the GPLv3 license, and a small README;
  - I removed the dependencies in gnatpython in the hooks themselves;
  - I started working towards multiple-project&email support.

Unless I get swallowed up by work, I hope to have lots of time
next week to continue on this.

-- 
Joel

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

[Sergio Durigan Junior]

On Sunday, December 07 2014, Joel Brobecker wrote:

> Hello everyone,
>
> Just a quick update on this. I've started working on adapting
> AdaCore's git hooks. I was planning on making them accessible
> somewhere this weekend, but the weekend is almost over and
> I am too tired to do it now. So I'll probably do it first thing
> next weekend.
>
> In terms of the adaptations I've been making:
>   - Added the GPLv3 license, and a small README;
>   - I removed the dependencies in gnatpython in the hooks themselves;
>   - I started working towards multiple-project&email support.
>
> Unless I get swallowed up by work, I hope to have lots of time
> next week to continue on this.

Again, thanks for working on this, Joel.  Do not hesitate to ask for
help if needed!

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

[Joel Brobecker]

> Just a quick update on this. I've started working on adapting
> AdaCore's git hooks. I was planning on making them accessible
> somewhere this weekend, but the weekend is almost over and
> I am too tired to do it now. So I'll probably do it first thing
> next weekend.

FYI: The hooks are now accessible at https://gitorious.org/githooks

I will continue working on supporting dynamic email notification
recipients next (sending to binutils and/or gdb depending on contents
of commit).

-- 
Joel

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

[Joel Brobecker]

> FYI: The hooks are now accessible at https://gitorious.org/githooks
> 
> I will continue working on supporting dynamic email notification
> recipients next (sending to binutils and/or gdb depending on contents
> of commit).

This feature has now been implemented, and pushed to the repository
above.

Next on my list (in random order):

  - Provide a way to call the script which sends updates to bugzilla

  - Add a post-receive hook which we would call at the end
    of our post-receive hook. This hook would then allow us
    to the irker hook, for IRC notification.

  - Add an option to include a URL of the commit in the emails.

  - Merge commit control.

  - Verify compatibility of the new hooks with the git version
    installed on sourceware.org.

Once that's done, I think all features provided by the current hooks
will also be available in the new hooks, so we'll be able to start
working on the integration.

-- 
Joel

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

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 4028 bytes --]

OK, good news, I think I'm near being done!

> Next on my list (in random order):
> 
>   - Provide a way to call the script which sends updates to bugzilla
> 
>   - Add a post-receive hook which we would call at the end
>     of our post-receive hook. This hook would then allow us
>     to the irker hook, for IRC notification.
> 
>   - Add an option to include a URL of the commit in the emails.
> 
>   - Merge commit control.

All the above have been done :)

>   - Verify compatibility of the new hooks with the git version
>     installed on sourceware.org.

This is where thing didn't go as well as I had hoped. I found that
working with the versions of Python and git installed on sourceware
would be too hard. So I sent an email to overseers to discuss solution
about using newer versions. I don't think it'll take very long to
resolve.

> Once that's done, I think all features provided by the current hooks
> will also be available in the new hooks, so we'll be able to start
> working on the integration.

I *think* I have everything covered if we use the config shown below
(full doc: https://sourceware.org/gdb/wiki/proposed/git-hooks/UsersGuide).
It handles:
  - dynamic computation of the email list (by attached email_to.py script).
  - rejection of merge commits on branch master and all GDB release branches.
  - filing of commits in bugzilla.
  - irker notification

|[hooks]
|        from-domain = sourceware.org
|        mailinglist = /git/binutils-gdb.git/hooks-bin/email_to.py
|
|        # We do not want to force a maximum line length in commit
|        # revision logs, as they get in the way of copy-pasting
|        # debugging session, error messages, logs, etc.
|        max-rh-line-length = 0
|
|        # Reject merge commits on a certain number of branches:
|        #  - on master: We request that people rebase their changes
|        #    before pushing instead (merge commits tend to confuse
|        #    git newcommers).
|        #  - on GDB release branches: There is a high risk that a merge
|        #    commit is a merge from master into the branch, which would
|        #    bring a lot more than what the user probably meant to push.
|        #    Request that the user cherry-pick his changes.
|        reject-merge-commits = refs/heads/master, refs/heads/gdb-.*
|
|        # The style checker, applied to the contents of each file being
|        # modified.
|        style-checker = /git/binutils-gdb.git/hooks-bin/style_checker
|
|        # The URL where we can inspect the commit, inserted in the commit
|        # notification email, and also copy sent to the file-commit-cmd.
|        commit-url = "https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=%(rev)s"
|
|        # Do not send emails for the following branches (unofficial
|        # third-party branches).
|        # FIXME: It would be nice to move the "hjl" branches to /user/.
|        no-emails = /refs/heads/hjl/.*, /refs/heads/user/.*
|
|        # Send a copy to bugzilla if a commit has a PR number in it.
|        file-commit-cmd = /sourceware/infra/bin/email-to-bugzilla -G "gdb binutils"
|        # The script that calls the irker (IRC notification of new
|        # commits).
|        post-receive-hook = /git/binutils-gdb.git/hooks-bin/post-receive

Note that this also introduces a "style checker", which is a script
called to check the contents of each and every file being modfied by
a given commit. At the moment, the script does nothing, but we could
imagine checking things like copyright headers, style violations; and
this checking can very well be dependent on the type of file.  But
that's a project on its own for which I won't have time. If someone
implements something, I'll integrate it!

Timeline:
=========

If people are happy with the above, when do we want to transition.
I feel the best time for me would be during the Xmas break.
How do people feel about me doing the transition sometime after
Monday Dec 29th?

Reminder: hooks sources at https://gitorious.org/githooks

-- 
Joel

[-- Attachment #2: email_to.py --]
[-- Type: text/x-python, Size: 1243 bytes --]

#! /usr/bin/env python
import sys

ML_MAP = {'bfd': 'bfd-cvs@example.com',
          'gdb': 'gdb-cvs@example.com',

          }

OWNER_MAP = (
    # BFD file...
    ('bfd/', 'bfd'),
    ('binutils/', 'bfd'),
    ('opcode/', 'bfd'),
    ('cpu/', 'bfd'),
    ('elfcpp/', 'bfd'),
    ('gas/', 'bfd'),
    ('gold/', 'bfd'),
    ('gprof/', 'bfd'),
    ('include/', 'bfd'),
    ('ld/', 'bfd'),
    ('opcodes/', 'bfd'),
    # GDB files...
    ('gdb/', 'gdb'),
    ('readline/', 'gdb'),
    ('sim/', 'gdb'),
    )

EVERYONE = set(ML_MAP[ml_key] for ml_key in ML_MAP)


def ml_from_filename(filename):
    for (path, ml_key) in OWNER_MAP:
        if filename.startswith(path):
            return ML_MAP[ml_key]
    # Not found in map, it is a common file.
    return EVERYONE

result = set()
for filename in sys.stdin:
    ml = ml_from_filename(filename)
    if isinstance(ml, basestring):
        result.add(ml)
    else:
        result.update(ml)
    if len(result) >= len(EVERYONE):
        # We have iterated over enough entries to know already
        # that we have selected all possible recipients. So
        # stop now.
        break

if not result:
    # No files given, return EVERYONE
    result = EVERYONE

print '\n'.join(sorted(result))

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

[Joel Brobecker]

> If people are happy with the above, when do we want to transition?
> I feel the best time for me would be during the Xmas break.
> How do people feel about me doing the transition sometime after
> Monday Dec 29th?

I felt that a lot of people might not have been reading emails
from these mailing lists during the Xmas break, so not quite
fair a schedule. But I'd like to have this transition done before
we branch if possible. It's not a requirement, but it'll allow us
to get an email posted when I push the new branch.

So, if there are no objections, and assuming I manage to get
the necessary privileges by then, I'd like to do the transition
on Saturday, Jan 10th, or Sunday, Jan 11th.

Any objections, please shout!

-- 
Joel

ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[Joel Brobecker]

Hello,

A quick message to confirm that I have switched the binutils-gdb.git
to the new hooks. I've tested the switch by pushing a change to
master, and here is a URL to the corresponding email that got sent:

    https://www.sourceware.org/ml/gdb-cvs/2015-01/msg00027.html

I've tried to test as extensively as I could, and that includes
verifying email sending as well as bugzilla filing and irker
notification (IRC). But let me know if you notice any issues.
In case of major issue that we can't repair right away, I've
kept the old setup and it's a couple of "mv"-s to swap back.

I've created a "Git Hooks" section in GDB's wiki at:
https://sourceware.org/gdb/wiki/GDBAdministrivia
It provides a link to where the hooks are hosted, as well
as a link to the documentation, which is still currently
in GDB's Wiki but will likely be moved to the git repo
at some point.

New URL for the git-hooks doco at:
https://sourceware.org/gdb/wiki/GitHooksUsersGuide
(moved it out of "proposed/git-hooks" since this is no longer
a proposal).

Enjoy!
-- 
Joel

Re: ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[H.J. Lu]

On Fri, Jan 9, 2015 at 10:58 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> Hello,
>
> A quick message to confirm that I have switched the binutils-gdb.git
> to the new hooks. I've tested the switch by pushing a change to
> master, and here is a URL to the corresponding email that got sent:
>
>     https://www.sourceware.org/ml/gdb-cvs/2015-01/msg00027.html
>
> I've tried to test as extensively as I could, and that includes
> verifying email sending as well as bugzilla filing and irker
> notification (IRC). But let me know if you notice any issues.
> In case of major issue that we can't repair right away, I've
> kept the old setup and it's a couple of "mv"-s to swap back.
>

This checkin:

https://sourceware.org/ml/binutils-cvs/2015-01/msg00074.html

didn't update the bugzilla.

-- 
H.J.

Re: ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[Joel Brobecker]

> This checkin:
> 
> https://sourceware.org/ml/binutils-cvs/2015-01/msg00074.html
> 
> didn't update the bugzilla.

I will take a look.

In the meantime, would you mind renaming your user branches to be
"user/[...]" instead of "users/[...]"?  This is the name we agreed
on and the one for which no email is going to get sent.

-- 
Joel

Re: ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[Joel Brobecker]

> In the meantime, would you mind renaming your user branches to be
> "user/[...]" instead of "users/[...]"?  This is the name we agreed
> on and the one for which no email is going to get sent.

Actually - looks like you just followed Cary's lead. I'll just
change the configuration, then.

-- 
Joel

Re: ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[Joel Brobecker]

> This checkin:
> 
> https://sourceware.org/ml/binutils-cvs/2015-01/msg00074.html
> 
> didn't update the bugzilla.

OK - I think I found the source of the problem (configuration issue),
and I suspect that only binutils was affected (ie, GDB PRs would have
been updated).

It should be fixed, now, but we should keep our eyes peeled for
the next commit with a PR number, for both binutils and GDB, just
to be sure.

-- 
Joel

Re: ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[H.J. Lu]

On Sun, Jan 11, 2015 at 8:58 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> This checkin:
>>
>> https://sourceware.org/ml/binutils-cvs/2015-01/msg00074.html
>>
>> didn't update the bugzilla.
>
> OK - I think I found the source of the problem (configuration issue),
> and I suspect that only binutils was affected (ie, GDB PRs would have
> been updated).
>
> It should be fixed, now, but we should keep our eyes peeled for
> the next commit with a PR number, for both binutils and GDB, just
> to be sure.
>

The bugzilla entry doesn't show which branch the commit was
applied into.

I committed the fix into both master and 2.25 branches.  There is
only one bugzilla entry for my commits.


-- 
H.J.

Re: ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[Joel Brobecker]

> The bugzilla entry doesn't show which branch the commit was
> applied into.

That's because I piped the output of "git show" to the script
that files it (which is also why the diff accidently made it
to bugzilla) - in other words, I didn't use the hooks, just
called the script by hand. Future updates will have the name
of the branch.

> I committed the fix into both master and 2.25 branches.  There is
> only one bugzilla entry for my commits.

Just file a comment that says you also pushed to the binutils 2.25
branch as well, explaining why you had to do it this way.

-- 
Joel

Re: ANNOUNCEMENT: we have now switched to new git hooks (was: "Re: RFC: using AdaCore's git hooks for binutils-gdb.git ...")

[Cary Coutant]

>> In the meantime, would you mind renaming your user branches to be
>> "user/[...]" instead of "users/[...]"?  This is the name we agreed
>> on and the one for which no email is going to get sent.
>
> Actually - looks like you just followed Cary's lead. I'll just
> change the configuration, then.

Oops, sorry!

-cary