Files
git-hooks/hooks/updates/branches/update.py
Joel Brobecker e536bf865a centralize the computation of the list of commits to apply checks on
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
2020-10-02 11:09:37 -07:00

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)