Re: GCC Git hooks

[Joseph Myers <joseph at codesourcery dot com>]

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