From: Joseph Myers <joseph@codesourcery.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: Jason Merrill <jason@redhat.com>,
Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>,
gcc Mailing List <gcc@gcc.gnu.org>,
Gerald Pfeifer <gerald@pfeifer.com>,
Daniel Berlin <dberlin@dberlin.org>
Subject: Re: GCC Git hooks
Date: Thu, 09 Jan 2020 22:07:00 -0000 [thread overview]
Message-ID: <alpine.DEB.2.21.2001092158480.27516@digraph.polyomino.org.uk> (raw)
In-Reply-To: <alpine.DEB.2.21.2001091417030.4040@digraph.polyomino.org.uk>
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
next prev parent reply other threads:[~2020-01-09 22:07 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-14 20:53 Jason Merrill
2019-09-15 1:54 ` Joseph Myers
2019-09-15 3:50 ` Gerald Pfeifer
2019-09-15 12:07 ` Joseph Myers
2019-09-15 16:16 ` Gerald Pfeifer
2019-09-16 15:11 ` Joel Brobecker
2019-09-16 20:02 ` Joseph Myers
2019-09-17 11:13 ` Gerald Pfeifer
2019-09-17 12:55 ` Joel Brobecker
2019-09-17 15:56 ` Joseph Myers
2019-09-16 15:06 ` Joel Brobecker
2019-09-26 12:41 ` Joel Brobecker
2020-01-09 14:26 ` Joseph Myers
2020-01-09 22:07 ` Joseph Myers [this message]
2020-01-10 11:03 ` Jonathan Wakely
2020-01-10 13:06 ` Joseph Myers
2020-01-10 13:38 ` Jonathan Wakely
2020-01-10 15:53 ` Joseph Myers
2020-01-10 18:00 ` Joel Brobecker
2020-01-10 18:15 ` Joseph Myers
2020-01-10 18:22 ` Joel Brobecker
2020-01-10 18:24 ` Joseph Myers
2020-01-10 18:40 ` Joel Brobecker
2020-01-10 20:44 ` Joseph Myers
2020-01-13 20:47 ` Joseph Myers
2020-01-10 17:57 ` Joel Brobecker
2020-01-10 17:38 ` Joel Brobecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.2001092158480.27516@digraph.polyomino.org.uk \
--to=joseph@codesourcery.com \
--cc=brobecker@adacore.com \
--cc=dberlin@dberlin.org \
--cc=gcc@gcc.gnu.org \
--cc=gerald@pfeifer.com \
--cc=jason@redhat.com \
--cc=maxim.kuvyrkov@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).