mirror of
https://github.com/AdaCore/git-hooks.git
synced 2026-02-12 12:43:11 -08:00
This change is mostly a cleanup which simplifies a bit the iteration over the list of commits for which to perform the various checks, by having that information provided via an attribute whose name is very specific about the semantics of what to use that list for. This was motivated by an issue we discovered recently where we forgot to exclude some commits when calling the commit-extra-checker hook. This happened because, no matter how simple the algorithm for computing the list of commits to check was, that simple algorithm was duplicated in many places. This avoids that, in the hopes that this will prevent this same kind of issues again in the future. Another way to view the benefits of this cleanup is that this will help ensure consistency in terms of the list of commits to which the various checks are applied. Note that we could have logically split this patch into two steps, with the first step simply renaming the "added_commits" attribute into "new_commits_for_ref", and then a second step introducing the "commits_to_check" attribute. In the end, both are combined in a single commit because it seems easier to review every location where the "added_commits" attribute was used, and make sure from the context that the correct list of commits was chosen in each instance the "added_commits" list used to be referenced. Otherwise, it's harder to review the second commit adding the "commits_to_check" attribute, because the reviewer then has to audit the entire set of sources himself in order to make sure no spot was missed. No actual behavior change should result from this change, and therefore no test is being added. Change-Id: I93c206968800dc738d3ebe4f5424f9201875383b TN: T929-030
107 lines
4.3 KiB
Python
107 lines
4.3 KiB
Python
"""Handling of branch updates."""
|
|
|
|
from errors import InvalidUpdate
|
|
from fast_forward import check_fast_forward
|
|
from git import is_null_rev, commit_oneline, commit_subject
|
|
from updates import AbstractUpdate, RefKind
|
|
from updates.branches import branch_summary_of_changes_needed
|
|
|
|
BRANCH_UPDATE_EMAIL_BODY_TEMPLATE = """\
|
|
The branch '%(short_ref_name)s' was updated to point to:
|
|
|
|
%(commit_oneline)s
|
|
|
|
It previously pointed to:
|
|
|
|
%(old_commit_oneline)s"""
|
|
|
|
|
|
def reject_retired_branch_update(short_ref_name, all_refs):
|
|
"""Raise InvalidUpdate if trying to update a retired branch.
|
|
|
|
PARAMETERS:
|
|
short_ref_name: The reference's short name (see short_ref_name
|
|
attribute in class AbstractUpdate).
|
|
all_refs: See the all_refs attribute in class AbstractUpdate.
|
|
|
|
REMARKS
|
|
By convention, retiring a branch means "moving" it to the
|
|
"retired/" sub-namespace. Normally, it would make better
|
|
sense to just use a tag instead of a branch (for the reference
|
|
in "retired/"), but we allow both.
|
|
"""
|
|
# If short_ref_name starts with "retired/", then the user is either
|
|
# trying to create the retired branch (which is allowed), or else
|
|
# trying to update it (which seems suspicious). In the latter
|
|
# case, we could disallow it, but it could also be argued that
|
|
# updating the retired branch is sometimes useful. Keep it simple
|
|
# for now, and allow.
|
|
if short_ref_name.startswith('retired/'):
|
|
return
|
|
|
|
retired_short_ref_name = 'retired/%s' % short_ref_name
|
|
if 'refs/heads/%s' % retired_short_ref_name in all_refs:
|
|
raise InvalidUpdate(
|
|
"Updates to the %s branch are no longer allowed, because"
|
|
% short_ref_name,
|
|
"this branch has been retired (and renamed into `%s')."
|
|
% retired_short_ref_name)
|
|
if 'refs/tags/%s' % retired_short_ref_name in all_refs:
|
|
raise InvalidUpdate(
|
|
"Updates to the %s branch are no longer allowed, because"
|
|
% short_ref_name,
|
|
"this branch has been retired (a tag called `%s' has been"
|
|
% retired_short_ref_name,
|
|
"created in its place).")
|
|
|
|
|
|
class BranchUpdate(AbstractUpdate):
|
|
"""Update object for branch creation/update."""
|
|
def self_sanity_check(self):
|
|
"""See AbstractUpdate.self_sanity_check."""
|
|
assert self.ref_kind == RefKind.branch_ref \
|
|
and self.object_type == 'commit'
|
|
|
|
def validate_ref_update(self):
|
|
"""See AbstractUpdate.validate_ref_update."""
|
|
reject_retired_branch_update(self.short_ref_name,
|
|
self.all_refs)
|
|
|
|
# Check that this is either a fast-forward update, or else that
|
|
# forced-updates are allowed for that branch. If old_rev is
|
|
# null, then it's a new branch, and so fast-forward checks are
|
|
# irrelevant.
|
|
if not is_null_rev(self.old_rev):
|
|
check_fast_forward(self.ref_name, self.old_rev, self.new_rev)
|
|
|
|
def get_update_email_contents(self):
|
|
"""See AbstractUpdate.get_update_email_contents.
|
|
"""
|
|
# For branch updates, we only send the update email when
|
|
# the summary of changes is needed.
|
|
if not branch_summary_of_changes_needed(self.new_commits_for_ref,
|
|
self.lost_commits):
|
|
return None
|
|
|
|
# Compute the subject.
|
|
update_info = {'repo': self.email_info.project_name,
|
|
'short_ref_name': self.short_ref_name,
|
|
'branch': '/%s' % self.short_ref_name,
|
|
'n_commits': '',
|
|
'subject': commit_subject(self.new_rev)[:59],
|
|
'old_commit_oneline': commit_oneline(self.old_rev),
|
|
'commit_oneline': commit_oneline(self.new_rev),
|
|
}
|
|
if self.short_ref_name == 'master':
|
|
update_info['branch'] = ''
|
|
if len(self.new_commits_for_ref) > 1:
|
|
update_info['n_commits'] = \
|
|
' (%d commits)' % len(self.new_commits_for_ref)
|
|
|
|
subject = "[%(repo)s%(branch)s]%(n_commits)s %(subject)s" % update_info
|
|
|
|
body = BRANCH_UPDATE_EMAIL_BODY_TEMPLATE % update_info
|
|
body += self.summary_of_changes()
|
|
|
|
return (self.everyone_emails(), subject, body)
|