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
jos...@codesourcery.com
