Re: GCC Git hooks

[Joel Brobecker <brobecker@adacore.com>]

Hi Joseph,

Apologies for the slow replies. The end of this week has been
pretty packed, and next week will be as well. But I will make
sure we answer every questions and suggestions you have!

On Thu, Jan 09, 2020 at 02:25:59PM +0000, Joseph Myers wrote:
> On Mon, 16 Sep 2019, Joel Brobecker wrote:
> 
> > Looking at the configuration file, I believe the git-hooks
> > should have most, if not all, of the features that would be needed for
> > the GCC repository. In particular, there is already a way to relay
> > commits to third-party tools via calling of a script; GDB uses that
> > to interface with their Bugzilla database.
> 
> I am now looking at the hook setup for GCC.  As far as I can see, I'll 
> initially need a GCC-specific fork of the hooks for two reasons:
> 
> * Custom ref naming.  The refs/vendors/<vendor>/{heads,tags} and 
> refs/users/<user>/{heads,tags} scheme described at 
> <https://gcc.gnu.org/ml/gcc/2019-11/msg00249.html>, to avoid such branches 
> being fetched by default, will need the hooks to know that such ref names 
> are to be treated as branches / tags, and to allow non-fast-forward pushes 
> to them.

That's actually a neat way of dealing with vendor branches. I have
always felt it was suboptimal that everyone had to fetch updates
in vendor branches in the GDB repositories. I don't know if I could
convince the GDB community to use this scheme, but I think it's
pretty clever.

I'll make sure there is a way to handle that without needing to
create your own version of the git-hooks.

> * I don't see a configuration option to add custom checks before accepting 
> a ref update.  I think we want a custom check that prevents people for 
> accidentally pushing merges between the old and new versions of the GCC 
> history.  It's easy to write something called from a pre-receive / update 
> hook that uses git rev-list to identify problem pushes, but doing that 
> without a fork of the hooks would require a suitable configuration option 
> to call out to such a custom script.

I think an update-hook setting allowing repositories to have a script
called to perform additional ad hoc verifications sounds like it could
be generally useful.

For the specific issue that you are trying to protect against, that's
unlikely to happen, IMO. When someone pushes a commit, what the hooks
do is look at the list of commits which are being added to that branch.
If the number of such commits exceed a limit (100 by default), it
rejects that push. I think a user trying to push a merge that points
to the old history will hit that rejection.

I will add both of these features to my TODO list.

-- 
Joel