GCC Git hooks

GCC Git hooks

[Jason Merrill]

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

At Cauldron this weekend Joel offered to adjust his git hooks
(https://github.com/brobecke/git-hooks), which are already used by gdb
and glibc, to meet GCC's needs.  Separately, Joseph volunteered to
deal with converting the gcc-www repository to git and dealing with
those hooks.

I expect that Joel's hooks should work fine for gcc-www with minimal
changes, but that the main GCC repository will need more work for
bugzilla integration.

The GCC SVN hooks initially look like pretty vanilla SVN hooks using
svnmailer (http://opensource.perlig.de/svnmailer/); the customized
part of the svnmailer.conf is just

[libstdcxx]
for_paths = .*(libstdc..-v3)/.*
to_addr = libstdc++-cvs@gcc.gnu.org

[java]
for_paths = .*(boehm-gc|fastjar|gcjx|gcc/java|libffi|libjava|zlib)/.*
to_addr = java-cvs@gcc.gnu.org

[gccdefault]
to_addr = gcc-cvs@gcc.gnu.org
bugzilla_to_addr = gcc-bugzilla@gcc.gnu.org

Pretty short...but the last line relies on Daniel's custom
bugzilla/svnmailer integration (attached below), and it looks like
Joel's hooks don't have anything comparable.  Any thoughts about
adjusting Daniel's bugzilla.py vs. pulling in something like
http://www.theoldmonk.net/gitzilla/ ?

Jason

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

# -*- coding: utf-8 -*-
#
# Copyright 2004-2006 André Malo or his licensors, as applicable
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""
Bugzilla based email notifiers (either piped to a program or via SMTP)
"""
__author__    = "Daniel Berlin"
__docformat__ = "epytext en"
__all__       = ['getNotifier']

handled_bugs = {}

def getNotifier(config, groupset):
    """ Returns an initialized notifier or nothing

        @param config: The svnmailer config
        @type config: C{svnmailer.settings.Settings}

        @param groupset: The groupset to process
        @type groupset: C{list}

        @return: The list of notifiers (containing 0 or 1 member)
        @rtype: C{list}
    """
    from svnmailer import settings
    from svnmailer.notifier import _textmail, _multimail

    cls = None
    if config.general.sendmail_command:
        cls = SendmailSubmitter
    elif config.general.smtp_host:
        cls = SMTPSubmitter

    if cls:
        mtype = (groupset.groups[0].mail_type or u'single').split()[0].lower()
        is_commit = (config.runtime.mode == settings.modes.commit)
        mod = (is_commit and mtype == u'multipart') and \
            _multimail or _textmail
        return mod.getNotifier(cls, config, groupset)

    return []


class SMTPSubmitter(object):
    """ Use SMTP to submit the mail """
    _settings = None

    def sendMail(self, sender, to_addr, mail):
        """ Sends the mail via SMTP """
        import smtplib, cStringIO

        fp = cStringIO.StringIO()
        mail.dump(fp)
        mail = fp.getvalue()
        fp.close()

        general = self._settings.general
        conn = smtplib.SMTP(general.smtp_host)
        if general.smtp_user:
            conn.login(general.smtp_user, general.smtp_pass)

        conn.sendmail(sender, to_addr, mail)
        conn.quit()


class SendmailSubmitter(object):
    """ Pipe all stuff to a mailer """
    _settings = None

    def sendMail(self, sender, to_addr, mail):
        """ Sends the mail via a piped mailer """
        global handled_bugs
        from svnmailer import util
        import sys
        import re
        import cStringIO
        bugre = re.compile('\s+(?:bug|PR|BZ)\s+\#?\s*(?:[a-z\-\+]+\/)?(?:\/)?(\d+)(.*)$',re.I | re.S | re.M)
        start = 0
        fp = cStringIO.StringIO()
        mail.dump(fp)
        mailtxt = fp.getvalue()
        fp.close()
        result = bugre.search(mailtxt, start)
        while result:
          try:
            bugnum = result.group(1)
            if handled_bugs.has_key(bugnum) == False:
              for group in [group for group in self._groupset.groups if group.bugzilla_to_addr]:
                handled_bugs[bugnum] = True
                mail.replace_header('To', group.bugzilla_to_addr)
                mail.replace_header('Subject',"[Bug %s]" % (bugnum, ))
                pipe = util.getPipe2(self._getMailCommand(sender, [group.bugzilla_to_addr]))
                pipe.fromchild.close() # we don't expect something
                mail.dump(pipe.tochild)
                pipe.tochild.close()

                # what do we do with the return code?
                pipe.wait()
            start = result.span(1)[1]
            result = bugre.search(mailtxt, start)
          except:
            result = None
            break

    def _getMailCommand(self, sender, to_addr):
        """ Returns the mailer command

            The command is created using sendmail conventions.
            If you want another commandline, override this method.

            @param sender: The sender address
            @type sender: C{str}

            @param to_addr: The receivers
            @type to_addr: C{list}

            @return: The command
            @rtype: C{list}
        """
        cmd = list(self._settings.general.sendmail_command)
        cmd[1:] = [(isinstance(arg, unicode) and
            [arg.encode("utf-8")] or [arg])[0] for arg in cmd[1:]
        ]
        cmd.extend(['-f', sender])
        cmd.extend(to_addr)

        return cmd

Re: GCC Git hooks

[Joseph Myers]

On Sat, 14 Sep 2019, Jason Merrill wrote:

> At Cauldron this weekend Joel offered to adjust his git hooks
> (https://github.com/brobecke/git-hooks), which are already used by gdb
> and glibc, to meet GCC's needs.  Separately, Joseph volunteered to
> deal with converting the gcc-www repository to git and dealing with
> those hooks.

I also volunteered to look at automated means of identifying local fixes 
for cvs2svn artifacts in the main repository, that could be applied as 
part of any conversion.  (Primarily that's for misplaced branchpoints - 
something that I find actually confusing in practice when trying to work 
out which changes were on an old branch or to find when a branch was 
created in order to locate relevant mailing list discussion.  There are 
however other cvs2svn artifacts, less confusing in practice, that it might 
also be possible to fix in an automated way.)

reposurgeon has some logic to identify and fix some such issues, but some 
preliminary checks on an old partial conversion with an old reposurgeon 
version indicate it's far from complete as regards the misplaced 
branchpoints in the GCC repository, so I think our own logic for 
identifying such fixes (commits with bad parents and appropriate 
replacement parent commits) will be useful for any repository conversion 
(either directly to produce a list of such fixes for the conversion 
process, or indirectly to inform improvements to the logic in reposurgeon, 
and for verification of the results).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Gerald Pfeifer]

On Sat, 14 Sep 2019, Jason Merrill wrote:
> Separately, Joseph volunteered to deal with converting the gcc-www 
> repository to git and dealing with those hooks.

This is great, thank you!

I was absolutely going to join Cauldron this week, but something personal 
is consuming me all of this month (and I am also off work), so I wanted 
to at least write a note with some thoughts, and this would have been one 
of the items -- converting wwwdocs without waiting for anything else --
... and then I got swamped the last days.

Thank you for picking this up, and for volunteering Joseph!

Let me know how I can help, and I will do my best.  (Even if I'll be
offline most of the next three weeks, I can get online most days and
spend some time.)

Yeah. :)

Gerald

Re: GCC Git hooks

[Joseph Myers]

On Sun, 15 Sep 2019, Gerald Pfeifer wrote:

> Let me know how I can help, and I will do my best.  (Even if I'll be
> offline most of the next three weeks, I can get online most days and
> spend some time.)

Apart from general review of the test conversion / conversion and hook 
scripts, which everyone can do, I think the main thing would be to advise 
on what needs to happen to avoid breaking the www.gnu.org copy of the site 
and your HTML validation system for commits.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Gerald Pfeifer]

On Sun, 15 Sep 2019, Joseph Myers wrote:
> Apart from general review of the test conversion / conversion and hook 
> scripts, which everyone can do, I think the main thing would be to advise 
> on what needs to happen to avoid breaking the www.gnu.org copy of the site 
> and your HTML validation system for commits.

I reached out to the GNU webmasters for the former.  

On the latter, let's just flip the switch whenever you're ready, and I'll 
see to adjust my script within a day or two and manually perform checks 
until that's done.  (If you can create a sample e-mail notification for 
me, I'll use that to start looking into it.)

Thanks,
Gerald

Re: GCC Git hooks

[Joel Brobecker]

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

Hello everyone,

On Sat, Sep 14, 2019 at 04:53:17PM -0400, Jason Merrill wrote:
> At Cauldron this weekend Joel offered to adjust his git hooks
> (https://github.com/brobecke/git-hooks), which are already used by gdb
> and glibc, to meet GCC's needs.  Separately, Joseph volunteered to
> deal with converting the gcc-www repository to git and dealing with
> those hooks.
> 
> I expect that Joel's hooks should work fine for gcc-www with minimal
> changes, but that the main GCC repository will need more work for
> bugzilla integration.
> 
> The GCC SVN hooks initially look like pretty vanilla SVN hooks using
> svnmailer (http://opensource.perlig.de/svnmailer/); the customized
> part of the svnmailer.conf is just
> 
> [libstdcxx]
> for_paths = .*(libstdc..-v3)/.*
> to_addr = libstdc++-cvs@gcc.gnu.org
> 
> [java]
> for_paths = .*(boehm-gc|fastjar|gcjx|gcc/java|libffi|libjava|zlib)/.*
> to_addr = java-cvs@gcc.gnu.org
> 
> [gccdefault]
> to_addr = gcc-cvs@gcc.gnu.org
> bugzilla_to_addr = gcc-bugzilla@gcc.gnu.org
> 
> Pretty short...but the last line relies on Daniel's custom
> bugzilla/svnmailer integration (attached below), and it looks like
> Joel's hooks don't have anything comparable.  Any thoughts about
> adjusting Daniel's bugzilla.py vs. pulling in something like
> http://www.theoldmonk.net/gitzilla/ ?

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.

But before I say more I should point to the documentation which,
for historical reasons, is available on the GDB wiki rather than
the git-hooks GitHub repository. I will fix that, but in the meantime:

    https://sourceware.org/gdb/wiki/GitHooksUsersGuide

I'm attaching a file called project.config, which shows the current
configuration for the GDB repository, as it is might be a good
starting point for GCC's configuration.

Of interest:

  * We can see that "hooks.mailinglist" is pointing to a script.
    The purpose of that script is to determine, based on which files
    changed, which mailinglists should the commit email be sent to.

  * There is a hooks.style_checker line. Currently, GDB points to
    a script which does nothing. If GCC has some scripts they want
    to be run to validate the contents of a file, this is where
    this can be done. There is no mechanism, really, to say "don't
    run the style_checker", but it's easy to just pass a null
    style_checker as done by GDB.

  * For bugzilla integration, GDB uses the hooks.file-commit-cmd
    file. I'm attaching the email-to-bugzilla script, although
    I don't know how useful it will be for GCC. It predates the git-hooks
    and I just re-used it as is when we switched over to the git-hooks.

Given that, it seems like the git-hooks might be ready to support
all the needs of the GCC repository? We would need to:

  - write a script that determines the list of recipients based
    on the list of files being changed; that should be a trivial
    adaptation of the script used on the GDB side;

  - Adapt the script filing the commit with bugzilla

  - create a refs/meta/config "branch", and add the project.config
    file with the git-hooks configuration.

I can definitely help with the configuration setup phase in whatever
capacity you'd like. I would recommend people take a look at the list
of options currently available to see what kind of initial configuration
we might want to start with.

-- 
Joel

[-- Attachment #2: project.config --]
[-- Type: text/plain, Size: 1762 bytes --]

[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).
        no-emails = refs/heads/users/.*

        # 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

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

#! /usr/bin/env python
import sys

ML_MAP = {'bfd': 'bfd-cvs@sourceware.org',
          'gdb': 'gdb-cvs@sourceware.org',

          }

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

[-- Attachment #4: email-to-bugzilla --]
[-- Type: text/plain, Size: 3586 bytes --]

#!/usr/bin/perl
# -*-Perl-*-
#
# Perl filter to handle the log messages from the checkin of files in
# a directory.  This script will group the lists of files by log
# message, and mail a single consolidated log message at the end of
# the commit.
#
# This file assumes a pre-commit checking program that leaves the
# names of the first and last commit directories in a temporary file.
#
# Contributed by David Hampton <hampton@cisco.com>
#
# hacked greatly by Greg A. Woods <woods@web.net>
#
# Then chopped down just to send bugzilla email, for git.

use POSIX;
use DBI;
#
#	Configurable options
#

$TMPDIR = "/sourceware/cvs-tmp";

$BMAILER       = "/usr/sbin/sendmail";

#
#	Subroutines
#
sub see_if_bugzilla_bug_exists {    
  local ($dbh, $product, $id) = @_;

  # Split $PRODUCT and SQL-ify.
  my $sql_product = '';
  foreach $i (split (/\s+/, $product)) {
      if ($sql_product ne '') {
	  $sql_product .= ', ';
      }
      $sql_product .= "'" . $i . "'";
  }

  my $sth2 = $dbh->prepare ("SELECT COUNT(*) from bugs where bug_id = $id and product_id = any (select products.id from products where name in ($sql_product))") or return 0;
  $sth2->execute() or return 0;
  my $count = $sth2->fetchrow_array ();
  return $count > 0;
}

sub mail_bug_notification {
    local($name, $subject, @text) = @_;
    open(MAIL, "| $BMAILER -f\"cvs-commit\@gcc.gnu.org\" $name");
    print MAIL "From: cvs-commit\@gcc.gnu.org\n";
    print MAIL "Subject: $subject\n";
    print MAIL "To: $name\n";
    print MAIL "Content-Type: text/plain; charset=UTF-8\n";
    print MAIL "\n";
    print MAIL join("\n", @text), "\n";
    close(MAIL);
}

#
#	Main Body
#

# Initialize basic variables
#
$debug = 0;
chop($hostname = `hostname`);

# Parse command line arguments.

while (@ARGV) {
    $arg = shift @ARGV;

    if ($arg eq '-d') {
	$debug = 1;
	print STDERR "Debug turned on...\n";
    } elsif ($arg eq '-G') {
	($bugzillaproduct) && die("Too many '-G' args\n");
	$bugzillaproduct = shift @ARGV;
    }
}

if ($hostname !~ /\./) {
    chop($domainname = `domainname`);
    $hostdomain = $hostname . "." . $domainname;
} else {
    $hostdomain = $hostname;
}

# Used with sprintf to form name of Gnats notification mailing list.
# %s argument comse from -G option.
$GNATS_MAIL_FORMAT = "%s-bugzilla\@$hostdomain";

# Collect the body of the commit message.
binmode STDIN, ":utf8";
while (<STDIN>) {
    chop;
    push (@text, $_);
}

$log_txt = join ("\n", @text);
%done_ids = {};
while ($log_txt =~ m/[^Aa](?:bug|PR|BZ)\s+\#?\s*(?:[a-z+-]+\/)?(?:\/)?(\d+)(.*)$/si) {
  $bug_id = $1;
  $log_txt = $2;
  if (!defined $done_ids{$bug_id})
    {
      $done_ids{$bug_id} = 1;
      # Send mail to Bugzilla, if required.
      if ($bugzillaproduct ne '') {
	my $dbh = undef;
	if ($bugzillaproduct eq 'gcc')
	  {
	    $dbh = DBI->connect ("dbi:mysql:bugs", "swbugz", "everythingroses");
	  }
	else  # elsif ($bugzillaproduct eq 'glibc')
	  {
	    $dbh = DBI->connect ("dbi:mysql:sourcesbugs", "swbugz", "everythingroses");
	  }
	if ($debug) 
	  {
	    print STDERR "Attempting to see if bug $bug_id exists\n";
	  }
	if (defined $dbh 
	    && &see_if_bugzilla_bug_exists ($dbh, $bugzillaproduct, $bug_id)) 
	  {
	    if ($debug) { print STDERR "It does\n"; }
	    if ($bugzillaproduct ne 'gcc') {
		&mail_bug_notification( sprintf ($GNATS_MAIL_FORMAT, "sourceware"), "[Bug $bug_id]", @text);
	    }
	    else { 
		&mail_bug_notification( sprintf ($GNATS_MAIL_FORMAT, $bugzillaproduct),
					"[Bug $bug_id]", @text);
	    }
	}
	if (defined $dbh)
	  {
	    $dbh->disconnect;
	  }
      }
    }
}

exit 0;

Re: GCC Git hooks

[Joel Brobecker]

Hi Gerald,

On Mon, Sep 16, 2019 at 12:15:57AM +0800, Gerald Pfeifer wrote:
> On Sun, 15 Sep 2019, Joseph Myers wrote:
> > Apart from general review of the test conversion / conversion and hook 
> > scripts, which everyone can do, I think the main thing would be to advise 
> > on what needs to happen to avoid breaking the www.gnu.org copy of the site 
> > and your HTML validation system for commits.
> 
> I reached out to the GNU webmasters for the former.  
> 
> On the latter, let's just flip the switch whenever you're ready, and I'll 
> see to adjust my script within a day or two and manually perform checks 
> until that's done.  (If you can create a sample e-mail notification for 
> me, I'll use that to start looking into it.)

You mean the email notification sent by the hooks when a commit
gets pushed? If yes, here is an example:

https://www.sourceware.org/ml/gdb-cvs/2019-09/msg00041.html

-- 
Joel

Re: GCC Git hooks

[Joseph Myers]

On Mon, 16 Sep 2019, Gerald Pfeifer wrote:

> On the latter, let's just flip the switch whenever you're ready, and I'll 
> see to adjust my script within a day or two and manually perform checks 
> until that's done.  (If you can create a sample e-mail notification for 
> me, I'll use that to start looking into it.)

The email notifications are the part most likely to change substantially 
at various points after the initial git conversion.  The post-receive hook 
I sent uses post-receive-email as something easy to set up, but I fully 
expect we might end up using the AdaCore hooks for the wwwdocs repository 
as well as for the GCC sources, with some suitable configuration that is 
mostly the same between the repositories.

I don't recommend using the emails to extract any metadata about the 
commits because the formatting of that information is likely to change.  
Rather, I suggest using the emails only as a signal that something has 
been pushed to the repository.  You can then e.g. use "git rev-parse HEAD" 
before and after updating the local checkout to see what the old and new 
HEAD commits were, and "git diff --name-only" to list the modified, new or 
removed files (as in the posted hook).  If you want to extract information 
about individual new commits (e.g. authors) then "git rev-list 
$old_HEAD..$new_HEAD" can be used to list the new commits present in 
$new_HEAD but not in $old_HEAD.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Gerald Pfeifer]

On Mon, 16 Sep 2019, Joel Brobecker wrote:
> You mean the email notification sent by the hooks when a commit
> gets pushed? If yes, here is an example:
> 
> https://www.sourceware.org/ml/gdb-cvs/2019-09/msg00041.html

Thank you, Joel!  I got a little worried how to best parse that ;-),
but then Joseph recommended against it and...

On Mon, 16 Sep 2019, Joseph Myers wrote:
> Rather, I suggest using the emails only as a signal that something has 
> been pushed to the repository.  You can then e.g. use "git rev-parse HEAD" 
> before and after updating the local checkout to see what the old and new 
> HEAD commits were, and "git diff --name-only" to list the modified, new or 
> removed files (as in the posted hook).

...luckily provided an alternative.  Thanks for that, Joseph!  I had
a look and have started to adjust my script following your recommendation.

From my perspective this should not hold off anything.  I can keep 
adjusting even once the switch has taken place and validate changes
manually during that period.

When, roughly, do you expect the switch can be ready?  I assume we'll
have some sort of flag day?  (I got in contact with webmaster@gnu.org 
who are asking the same questions.)

Gerald

Re: GCC Git hooks

[Joel Brobecker]

> > You mean the email notification sent by the hooks when a commit
> > gets pushed? If yes, here is an example:
> > 
> > https://www.sourceware.org/ml/gdb-cvs/2019-09/msg00041.html
> 
> Thank you, Joel!  I got a little worried how to best parse that ;-),
> but then Joseph recommended against it and...
> 
> On Mon, 16 Sep 2019, Joseph Myers wrote:
> > Rather, I suggest using the emails only as a signal that something has 
> > been pushed to the repository.  You can then e.g. use "git rev-parse HEAD" 
> > before and after updating the local checkout to see what the old and new 
> > HEAD commits were, and "git diff --name-only" to list the modified, new or 
> > removed files (as in the posted hook).
> 
> ...luckily provided an alternative.  Thanks for that, Joseph!  I had
> a look and have started to adjust my script following your recommendation.

Not sure if the following could be of use, but the commit emails have
some information in the email header itself.

For instance, if you look at
https://sourceware.org/cgi-bin/get-raw-msg?listname=gdb-cvs&date=2019-09&msgid=20190917115728.124469.qmail%40sourceware.org

You can see:

    X-Act-Checkin: binutils-gdb
    X-Git-Author: Tom de Vries <tdevries@suse.de>
    X-Git-Refname: refs/heads/gdb-8.3-branch
    X-Git-Oldrev: 6f4f8f476a4e41cc7117a8e85087963c0ac3e95b
    X-Git-Newrev: fafa92ec3ca92e06fdea8f0f6a0fb08f5f906f77

(the X-Act-Checkin field gives the name of the repository)

> >From my perspective this should not hold off anything.  I can keep 
> adjusting even once the switch has taken place and validate changes
> manually during that period.
> 
> When, roughly, do you expect the switch can be ready?  I assume we'll
> have some sort of flag day?  (I got in contact with webmaster@gnu.org 
> who are asking the same questions.)

Just for the avoidance of doubt, the only item on my list is
to move the git-hooks' doc directly into the git-hooks repository
so they can be easily found when going to the hooks' github page
(in progress, pull request sent yesterday).

-- 
Joel

Re: GCC Git hooks

[Joseph Myers]

On Tue, 17 Sep 2019, Gerald Pfeifer wrote:

> When, roughly, do you expect the switch can be ready?

That's a matter of how much time we want to allow for people to try out 
and comment on the conversion.

> I assume we'll have some sort of flag day?

Yes, at some point we'd do a final conversion and switch the live website 
over to using git.  Then, in git, commit the post-receive hook, updates to 
the preprocess script so that when run in whole-site mode it knows to 
ignore .git/ the way it now knows to ignore CVS/, and updates to the 
website to document how to edit it using git instead of CVS.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Joel Brobecker]

> But before I say more I should point to the documentation which,
> for historical reasons, is available on the GDB wiki rather than
> the git-hooks GitHub repository. I will fix that, but in the meantime:
> 
>     https://sourceware.org/gdb/wiki/GitHooksUsersGuide

Just a quick note to confirm that the documentation has now been
moved to the git-hooks github page: https://github.com/adacore/git-hooks

Don't hesitate to reach out to me, if you have questions, or would
like some help configuring the hooks.

-- 
Joel

Re: GCC Git hooks

[Joseph Myers]

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.

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

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Joseph Myers]

On Thu, 9 Jan 2020, 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:

Concretely, these are the changes I'm currently using to configure the 
hooks in a way I think appropriate for GCC, and it would be useful if the 
hooks could support such configuration in a more generic way in future so 
that we can stop using a GCC-specific patched installation of the hooks.  
The following features are hardcoded that didn't seem to have a way to 
configure them:

* Additional branch namespaces refs/users/<user>/heads and 
refs/vendors/<vendor>/heads, and similar tag namespaces 
refs/users/<user>/tags and refs/vendors/<vendor>/tags.

* Only allowing branch deletion for certain ref patterns (refs/users and 
refs/vendors in this case).

* Controlling when non-fast-forward pushes are allowed for ref patterns 
outside refs/heads.

* Preventing pushes that add branches based on a given commit or introduce 
it to the history of branches not previously based on that commit (to 
prevent getting the history of the git-svn GCC mirror accidentally added 
to the refs fetched by default).  This precise form of check may be fairly 
GCC-specific, so providing a way to call out to a custom script to check 
ref updates would also be a possibility for how to support such checks.

diff --git a/hooks/fast_forward.py b/hooks/fast_forward.py
index 1e90cc5..f958dce 100755
--- a/hooks/fast_forward.py
+++ b/hooks/fast_forward.py
@@ -48,8 +48,11 @@ def check_fast_forward(ref_name, old_rev, new_rev):
     # such an update is allowed.
     ok_branches = git_config('hooks.allow-non-fast-forward')
 
-    for branch in ["refs/heads/" + branch.strip()
-                   for branch in ok_branches + FORCED_UPDATE_OK_BRANCHES]:
+    ok_refs = ["refs/heads/" + branch.strip()
+               for branch in ok_branches + FORCED_UPDATE_OK_BRANCHES]
+    ok_refs.append('refs/users/')
+    ok_refs.append('refs/vendors/')
+    for branch in ok_refs:
         if re.match(branch, ref_name) is not None:
             # This is one of the branches where a non-fast-forward update
             # is allowed.  Allow the update, but print a warning for
diff --git a/hooks/updates/branches/deletion.py b/hooks/updates/branches/deletion.py
index 118945f..edaab64 100644
--- a/hooks/updates/branches/deletion.py
+++ b/hooks/updates/branches/deletion.py
@@ -1,5 +1,6 @@
 """Handling of branch deletion."""
 
+from errors import InvalidUpdate
 from git import commit_oneline
 from updates import AbstractUpdate
 from updates.branches import branch_summary_of_changes_needed
@@ -15,12 +16,22 @@ class BranchDeletion(AbstractUpdate):
     """Update object for branch creation/update."""
     def self_sanity_check(self):
         """See AbstractUpdate.self_sanity_check."""
-        assert self.ref_name.startswith('refs/heads/')
+        assert (self.ref_name.startswith('refs/heads/')
+                or self.ref_name.startswith('refs/users/')
+                or self.ref_name.startswith('refs/vendors/'))
 
     def validate_ref_update(self):
         """See AbstractUpdate.validate_ref_update."""
-        # Deleting a branch is always allowed.
-        pass
+        # Deleting a user or vendor branch is always allowed.
+        if not (self.ref_name.startswith('refs/users')
+                or self.ref_name.startswith('refs/vendors')):
+            raise InvalidUpdate(
+                'Branch deletion is only allowed for user and vendor '
+                'branches. If another branch was created by mistake, '
+                'contact an administrator to delete it on the server '
+                'with git update-ref. If a development branch is dead, '
+                'also contact an administrator to move it under '
+                'refs/dead/heads/ rather than deleting it.')
 
     def get_update_email_contents(self):
         """See AbstractUpdate.get_update_email_contents.
diff --git a/hooks/updates/branches/update.py b/hooks/updates/branches/update.py
index eb51f9a..96cc777 100644
--- a/hooks/updates/branches/update.py
+++ b/hooks/updates/branches/update.py
@@ -2,7 +2,7 @@
 
 from errors import InvalidUpdate
 from fast_forward import check_fast_forward
-from git import is_null_rev, commit_oneline, commit_subject
+from git import git, is_null_rev, commit_oneline, commit_subject
 from updates import AbstractUpdate
 from updates.branches import branch_summary_of_changes_needed
 
@@ -63,6 +63,8 @@ class BranchUpdate(AbstractUpdate):
         # the update unless we have had a chance to verify that these hooks
         # work well with those branches.
         assert (self.ref_name.startswith('refs/heads/')
+                or self.ref_name.startswith('refs/users/')
+                or self.ref_name.startswith('refs/vendors/')
                 # Namespaces used by Gerrit.
                 or self.ref_name.startswith('refs/meta/')
                 or self.ref_name.startswith('refs/publish/')
@@ -80,6 +82,20 @@ class BranchUpdate(AbstractUpdate):
         # irrelevant.
         if not is_null_rev(self.old_rev):
             check_fast_forward(self.ref_name, self.old_rev, self.new_rev)
+            # GCC-specific: do not allow updates introducing ancestry
+            # based on the old git-svn repository, to ensure people
+            # rebase onto the new history rather than merging branches
+            # based on git-svn history into those based on the new history.
+            rev_list = git.rev_list(
+                self.new_rev, '^%s' % self.old_rev)
+        else:
+            rev_list = git.rev_list(
+                self.new_rev)
+        if '3cf0d8938a953ef13e57239613d42686f152b4fe' in rev_list:
+            raise InvalidUpdate(
+                'Refs not based on the git-svn history must not be '
+                'updated to be based on it, and new branches may not be '
+                'based on the old history.')
 
     def get_update_email_contents(self):
         """See AbstractUpdate.get_update_email_contents.
diff --git a/hooks/updates/factory.py b/hooks/updates/factory.py
index b24a8ff..49b349c 100644
--- a/hooks/updates/factory.py
+++ b/hooks/updates/factory.py
@@ -49,6 +49,10 @@ REF_CHANGE_MAP = {
     ('refs/tags/',         UPDATE, 'commit'): LightweightTagUpdate,
 }
 
+# GCC-specific namespaces.  refs/vendors/<vendor>/ and
+# refs/users/<user>/ contain heads/ and tags/.
+GCC_NAMESPACES = ('refs/users/', 'refs/vendors/')
+
 
 def new_update(ref_name, old_rev, new_rev, all_refs, submitter_email):
     """Return the correct object for the given parameters.
@@ -81,6 +85,22 @@ def new_update(ref_name, old_rev, new_rev, all_refs, submitter_email):
             new_cls = REF_CHANGE_MAP[key]
             break
     if new_cls is None:
+        for namespace in GCC_NAMESPACES:
+            if ref_name.startswith(namespace):
+                sub_name = ref_name[len(namespace):]
+                sub_split = sub_name.split('/')
+                if len(sub_split) >= 3:
+                    mod_name = 'refs/' + ('/'.join(sub_split[1:]))
+                    for key in REF_CHANGE_MAP:
+                        (map_ref_prefix, map_change_type, map_object_type) = key
+                        if ((change_type == map_change_type
+                             and object_type == map_object_type
+                             and mod_name.startswith(map_ref_prefix))):
+                            new_cls = REF_CHANGE_MAP[key]
+                            break
+                if new_cls is not None:
+                    break
+    if new_cls is None:
         return None
 
     return new_cls(ref_name, old_rev, new_rev, all_refs,
diff --git a/hooks/updates/tags/atag_update.py b/hooks/updates/tags/atag_update.py
index 4366f30..ed4a9d5 100644
--- a/hooks/updates/tags/atag_update.py
+++ b/hooks/updates/tags/atag_update.py
@@ -32,7 +32,9 @@ class AnnotatedTagUpdate(AbstractUpdate):
     """
     def self_sanity_check(self):
         """See AbstractUpdate.self_sanity_check."""
-        assert self.ref_name.startswith('refs/tags/')
+        assert (self.ref_name.startswith('refs/tags/')
+                or self.ref_name.startswith('refs/users/')
+                or self.ref_name.startswith('refs/vendors/'))
         assert self.new_rev_type == 'tag'
 
     def validate_ref_update(self):
diff --git a/hooks/updates/tags/ltag_deletion.py b/hooks/updates/tags/ltag_deletion.py
index 50ce1a3..90c04d8 100644
--- a/hooks/updates/tags/ltag_deletion.py
+++ b/hooks/updates/tags/ltag_deletion.py
@@ -32,7 +32,9 @@ class LightweightTagDeletion(AbstractUpdate):
         REMARKS
             This method handles both lightweight and annotated tags.
         """
-        assert self.ref_name.startswith('refs/tags/')
+        assert (self.ref_name.startswith('refs/tags/')
+                or self.ref_name.startswith('refs/users/')
+                or self.ref_name.startswith('refs/vendors/'))
         assert self.new_rev_type == 'delete'
 
     def validate_ref_update(self):
diff --git a/hooks/updates/tags/ltag_update.py b/hooks/updates/tags/ltag_update.py
index 5bfa2b2..dca6a9f 100644
--- a/hooks/updates/tags/ltag_update.py
+++ b/hooks/updates/tags/ltag_update.py
@@ -29,7 +29,9 @@ class LightweightTagUpdate(AbstractUpdate):
     """
     def self_sanity_check(self):
         """See AbstractUpdate.self_sanity_check."""
-        assert self.ref_name.startswith('refs/tags/')
+        assert (self.ref_name.startswith('refs/tags/')
+                or self.ref_name.startswith('refs/users/')
+                or self.ref_name.startswith('refs/vendors/'))
         assert self.new_rev_type == 'commit'
 
     def validate_ref_update(self):

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Jonathan Wakely]

On Thu, 9 Jan 2020 at 22:07, Joseph Myers wrote:
>
> @@ -63,6 +63,8 @@ class BranchUpdate(AbstractUpdate):
>          # the update unless we have had a chance to verify that these hooks
>          # work well with those branches.
>          assert (self.ref_name.startswith('refs/heads/')
> +                or self.ref_name.startswith('refs/users/')
> +                or self.ref_name.startswith('refs/vendors/')
>                  # Namespaces used by Gerrit.
>                  or self.ref_name.startswith('refs/meta/')
>                  or self.ref_name.startswith('refs/publish/')
> @@ -80,6 +82,20 @@ class BranchUpdate(AbstractUpdate):
>          # irrelevant.
>          if not is_null_rev(self.old_rev):
>              check_fast_forward(self.ref_name, self.old_rev, self.new_rev)
> +            # GCC-specific: do not allow updates introducing ancestry
> +            # based on the old git-svn repository, to ensure people
> +            # rebase onto the new history rather than merging branches
> +            # based on git-svn history into those based on the new history.
> +            rev_list = git.rev_list(
> +                self.new_rev, '^%s' % self.old_rev)
> +        else:
> +            rev_list = git.rev_list(
> +                self.new_rev)
> +        if '3cf0d8938a953ef13e57239613d42686f152b4fe' in rev_list:
> +            raise InvalidUpdate(
> +                'Refs not based on the git-svn history must not be '
> +                'updated to be based on it, and new branches may not be '
> +                'based on the old history.')

Could you avoid the double negative here? And the error message could
be more specific to the actual error by testing the two cases
separately, e.g.

        if not is_null_rev(self.old_rev):
            check_fast_forward(self.ref_name, self.old_rev, self.new_rev)
            # GCC-specific: do not allow updates introducing ancestry
            # based on the old git-svn repository, to ensure people
            # rebase onto the new history rather than merging branches
            # based on git-svn history into those based on the new history.
            rev_list = git.rev_list(
                self.new_rev, '^%s' % self.old_rev)
            if '3cf0d8938a953ef13e57239613d42686f152b4fe' in rev_list:
                raise InvalidUpdate(
                'Refs must not be updated to introduce history from '
                'the old git-svn repo.')
        else:
            rev_list = git.rev_list(
                self.new_rev)
            if '3cf0d8938a953ef13e57239613d42686f152b4fe' in rev_list:
                raise InvalidUpdate(
                'New branches must not be based on the old git-svn repo.')

Re: GCC Git hooks

[Joseph Myers]

On Fri, 10 Jan 2020, Jonathan Wakely wrote:

> Could you avoid the double negative here? And the error message could
> be more specific to the actual error by testing the two cases
> separately, e.g.

I'm sort of hoping we don't end up using the hooks in this form for very 
long - the patch was posted to demonstrate the features that seem to need 
changes to the hook code, but hopefully that code can get extended 
upstream to support such features in a cleaner way and then we'll only 
need some custom configuration, not custom code.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Jonathan Wakely]

On Fri, 10 Jan 2020 at 13:06, Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Fri, 10 Jan 2020, Jonathan Wakely wrote:
>
> > Could you avoid the double negative here? And the error message could
> > be more specific to the actual error by testing the two cases
> > separately, e.g.
>
> I'm sort of hoping we don't end up using the hooks in this form for very
> long - the patch was posted to demonstrate the features that seem to need
> changes to the hook code, but hopefully that code can get extended
> upstream to support such features in a cleaner way and then we'll only
> need some custom configuration, not custom code.

Gotcha. Forget what I said then.

Re: GCC Git hooks

[Joseph Myers]

On Thu, 9 Jan 2020, Joseph Myers wrote:

> Concretely, these are the changes I'm currently using to configure the 
> hooks in a way I think appropriate for GCC, and it would be useful if the 
> hooks could support such configuration in a more generic way in future so 
> that we can stop using a GCC-specific patched installation of the hooks.  
> The following features are hardcoded that didn't seem to have a way to 
> configure them:

Plus one further change now: if a newly created branch is in refs/heads/, 
require it to be in refs/heads/devel/ or refs/heads/releases/ (i.e. 
enforce a particular branch naming convention, in particular to prevent 
mistakes where people accidentally push a branch into refs/heads/ because 
their push configuration for user or vendor branches was wrong).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Joel Brobecker]

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

Re: GCC Git hooks

[Joel Brobecker]

> * Additional branch namespaces refs/users/<user>/heads and 
> refs/vendors/<vendor>/heads, and similar tag namespaces 
> refs/users/<user>/tags and refs/vendors/<vendor>/tags.

Hmmm. Note to self: I missed the fact that this namespace was
also used for tags.

> * Only allowing branch deletion for certain ref patterns (refs/users and 
> refs/vendors in this case).

Once we add support for the update-hook, I think you'll be able to
implement that feature there.

This might be of general interest however, so I'll think about
something. Up until now, the configuration options taking patterns
have always taken pattern that activate the option of they match.
Here, we want "turn this option on, except in those cases".
Maybe the way to go is to have two options, both related:

  - allow-branch-deletion (default: True)
  - branch-deletion-exception (default: empty list)

The second option is ignored if allow-branch-deletion is True.

Project that want to reject them all, just sets the option to False.

Projects like GCC then use both:

  allow-branch-deletion: True
  branch-deletion-exception = <vendors_branch_ref_regexp>
  branch-deletion-exception = <users_branch_ref_regexp>

> * Controlling when non-fast-forward pushes are allowed for ref patterns 
> outside refs/heads.

OK.


-- 
Joel

Re: GCC Git hooks

[Joel Brobecker]

> > Concretely, these are the changes I'm currently using to configure the 
> > hooks in a way I think appropriate for GCC, and it would be useful if the 
> > hooks could support such configuration in a more generic way in future so 
> > that we can stop using a GCC-specific patched installation of the hooks.  
> > The following features are hardcoded that didn't seem to have a way to 
> > configure them:
> 
> Plus one further change now: if a newly created branch is in refs/heads/, 
> require it to be in refs/heads/devel/ or refs/heads/releases/ (i.e. 
> enforce a particular branch naming convention, in particular to prevent 
> mistakes where people accidentally push a branch into refs/heads/ because 
> their push configuration for user or vendor branches was wrong).

I'm having a hard time understanding this requirement.

You want to say that, before branch "<xxx>" gets created, you want
to verify that a branch named either "devel/<xxx>" or "releases/<xxx>"
does exist? And probably also that the commit in branch "<xxx>"
is already present in the branch that already exists?

IIUC, I think this one is highly specialized, and shoud be done
in the update-hook script. Would that be OK?

-- 
Joel

Re: GCC Git hooks

[Joseph Myers]

On Fri, 10 Jan 2020, Joel Brobecker wrote:

> > Plus one further change now: if a newly created branch is in refs/heads/, 
> > require it to be in refs/heads/devel/ or refs/heads/releases/ (i.e. 
> > enforce a particular branch naming convention, in particular to prevent 
> > mistakes where people accidentally push a branch into refs/heads/ because 
> > their push configuration for user or vendor branches was wrong).
> 
> I'm having a hard time understanding this requirement.
> 
> You want to say that, before branch "<xxx>" gets created, you want
> to verify that a branch named either "devel/<xxx>" or "releases/<xxx>"
> does exist?

No.  What we want to ensure is that people don't accidentally create a 
branch called refs/heads/foo when they should (by our naming conventions) 
have created one called refs/heads/devel/foo or 
refs/users/someone/heads/foo.  Our naming conventions mean that all 
branches in refs/heads/ should be called master, devel/something or 
releases/something.  But it's easy for someone to get a "git push" command 
wrong so that it would create a badly named branch.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Joel Brobecker]

> > > Plus one further change now: if a newly created branch is in refs/heads/, 
> > > require it to be in refs/heads/devel/ or refs/heads/releases/ (i.e. 
> > > enforce a particular branch naming convention, in particular to prevent 
> > > mistakes where people accidentally push a branch into refs/heads/ because 
> > > their push configuration for user or vendor branches was wrong).
> > 
> > I'm having a hard time understanding this requirement.
> > 
> > You want to say that, before branch "<xxx>" gets created, you want
> > to verify that a branch named either "devel/<xxx>" or "releases/<xxx>"
> > does exist?
> 
> No.  What we want to ensure is that people don't accidentally create a 
> branch called refs/heads/foo when they should (by our naming conventions) 
> have created one called refs/heads/devel/foo or 
> refs/users/someone/heads/foo.  Our naming conventions mean that all 
> branches in refs/heads/ should be called master, devel/something or 
> releases/something.  But it's easy for someone to get a "git push" command 
> wrong so that it would create a badly named branch.

Could you rely on the update-hook script for that?

-- 
Joel

Re: GCC Git hooks

[Joseph Myers]

On Fri, 10 Jan 2020, Joel Brobecker wrote:

> > No.  What we want to ensure is that people don't accidentally create a 
> > branch called refs/heads/foo when they should (by our naming conventions) 
> > have created one called refs/heads/devel/foo or 
> > refs/users/someone/heads/foo.  Our naming conventions mean that all 
> > branches in refs/heads/ should be called master, devel/something or 
> > releases/something.  But it's easy for someone to get a "git push" command 
> > wrong so that it would create a badly named branch.
> 
> Could you rely on the update-hook script for that?

We could, it just feels like "branch names [in refs/heads/] must match one 
of these naming conventions" is something fairly generic rather than 
extremely GCC-specific.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Joel Brobecker]

> We could, it just feels like "branch names [in refs/heads/] must match one 
> of these naming conventions" is something fairly generic rather than 
> extremely GCC-specific.

I understand.

My fear is that we're discussing a lot of new configurations knobs.
And from experience, they can start interacting between them, and
getting every one of them to behave the way people expect them
in all possible situations can quickly become a challenge.

I was concerned specifically with the interaction with the naming
scheme chosen for vendor and user branches, but I think we can make
it work:

   One config to list the naming scheme for branches
   One config to list the naming scheme for tags

I just want to be careful to also consider how all the options
are interacting with each other. In this case, we were able to
combine two requirements into one, so that addresses my concern.

-- 
Joel

Re: GCC Git hooks

[Joseph Myers]

One further change I've made for the GCC hooks: rejecting commits whose 
commit message's first line looks like a ChangeLog header (matches the 
regular expression '[0-9]{4}-[0-9]{2}-[0-9]{2} .*<.*@.*>').  That should 
help avoid people's habits writing such commit messages carrying over into 
commits made to git.  That feels like a reasonably generic check that 
might be reasonable to do in general without needing a configuration 
option, though I suppose it's only generic to repositories that have 
contributors who are used to GNU ChangeLog format.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC Git hooks

[Joseph Myers]

One more addition I've made to the hooks: rejecting new commits with a 
line in the commit message starting 'From-SVN: ', since such commits would 
confuse the scripts people have to find the git commit corresponding to a 
given SVN commit, and without such a rejection it would be easy for 
someone using "git cherry-pick" (to cherry-pick an SVN-era commit onto 
another branch) to push a commit with From-SVN: by mistake (as git 
cherry-pick copies the original commit message, with a statement about 
being cherry-picked added if you use -x).

-- 
Joseph S. Myers
joseph@codesourcery.com