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
132 lines
5.5 KiB
Python
132 lines
5.5 KiB
Python
"""Handling of branch creation."""
|
|
|
|
from errors import InvalidUpdate
|
|
from git import commit_oneline, git, file_exists
|
|
from updates.branches import branch_summary_of_changes_needed
|
|
from updates.branches.update import BranchUpdate
|
|
|
|
BRANCH_CREATION_EMAIL_BODY_TEMPLATE = """\
|
|
The branch %(human_readable_ref_name)s pointing to:
|
|
|
|
%(commit_oneline)s"""
|
|
|
|
|
|
class BranchCreation(BranchUpdate):
|
|
"""Update class for branch creation.
|
|
|
|
REMARKS
|
|
Creation is a special case of update, and the implementation of
|
|
some of the abstract methods would be identical. So inherit
|
|
from BranchUpdate.
|
|
"""
|
|
def validate_ref_update(self):
|
|
"""See AbstractUpdate.validate_ref_update."""
|
|
# First, perform the same validation as our parent class
|
|
# (which means perform the same validation checks as for
|
|
# branch updates).
|
|
super(BranchCreation, self).validate_ref_update()
|
|
|
|
self.__check_gitreview_defaultbranch()
|
|
|
|
def get_update_email_contents(self):
|
|
"""See AbstractUpdate.get_update_email_contents.
|
|
"""
|
|
subject = "[%s] Created branch %s" % (
|
|
self.email_info.project_name, self.human_readable_ref_name())
|
|
|
|
update_info = {
|
|
'human_readable_ref_name': self.human_readable_ref_name(
|
|
action='was created'),
|
|
'commit_oneline': commit_oneline(self.new_rev),
|
|
}
|
|
body = BRANCH_CREATION_EMAIL_BODY_TEMPLATE % update_info
|
|
if branch_summary_of_changes_needed(self.new_commits_for_ref,
|
|
self.lost_commits):
|
|
body += self.summary_of_changes()
|
|
|
|
return (self.everyone_emails(), subject, body)
|
|
|
|
def __check_gitreview_defaultbranch(self):
|
|
"""If .gitreview exists, validate the defaultbranch value.
|
|
|
|
This is meant to catch the situation where a user creates
|
|
a new branch for a repository hosted on gerrit. Those
|
|
repositories typically have a .gitreview file at their root,
|
|
providing various information, one of them being the default
|
|
branch name when sending patches for review. When creating
|
|
a new branch, it is very easy (and frequent) for a user to
|
|
forget to also update the .gitreview file, causing patch
|
|
reviews to be sent with the wrong branch, which later then
|
|
causes the patch to be merged (submitted) on the wrong
|
|
branch once it is approved.
|
|
|
|
We try to avoid that situation by reading the contents of
|
|
those files at branch creation time, and reporting an error
|
|
if it exists and points to a branch name different from ours.
|
|
|
|
Note that we only do that for the traditional git branches.
|
|
We don't worry about the branches in the gerrit-specific
|
|
special namespaces, for which a user checking out the branch
|
|
and sending a review is unlikely.
|
|
"""
|
|
GITREVIEW_FILENAME = '.gitreview'
|
|
DEFAULTBRANCH_CONFIG_NAME = 'gerrit.defaultbranch'
|
|
|
|
# Only perform this check for traditional git branches.
|
|
# See method description above for the reason why.
|
|
if not self.ref_name.startswith('refs/heads/'):
|
|
return
|
|
|
|
if self.search_config_option_list('hooks.no-precommit-check')\
|
|
is not None:
|
|
# The user explicitly disabled the .gitreview check
|
|
# on this branch.
|
|
return
|
|
|
|
# If the file doesn't exist for that branch, then there is
|
|
# no problem.
|
|
if not file_exists(self.new_rev, GITREVIEW_FILENAME):
|
|
return
|
|
|
|
# Get the contents of the gitreview file, and then get git
|
|
# to parse its contents. We process it all into a dictionary
|
|
# rather than just query the value of the one config we are
|
|
# looking for, for a couple of reasons:
|
|
# 1. This allows us to avoid having to git returning with
|
|
# and error status when the file does not have the config
|
|
# entry we are looking for (git returns error code 1
|
|
# in that case);
|
|
# 2. If we even want to look at other configurations in
|
|
# that file, the code is already in place to do so.
|
|
|
|
gitreview_contents = git.show(
|
|
'%s:%s' % (self.new_rev, GITREVIEW_FILENAME))
|
|
gitreview_configs = git.config(
|
|
'-z', '-l', '--file', '-',
|
|
_input=gitreview_contents).split('\x00')
|
|
|
|
config_map = {}
|
|
for config in gitreview_configs:
|
|
if not config:
|
|
# "git config -z" adds a nul character at the end of
|
|
# its output, which cause gitreview_configs to end with
|
|
# an empty entry. Just ignore those.
|
|
continue
|
|
config_name, config_val = config.split('\n', 1)
|
|
config_map[config_name] = config_val
|
|
|
|
if DEFAULTBRANCH_CONFIG_NAME in config_map and \
|
|
config_map[DEFAULTBRANCH_CONFIG_NAME] != self.short_ref_name:
|
|
raise InvalidUpdate(
|
|
"Incorrect gerrit default branch name in file `%s'."
|
|
% GITREVIEW_FILENAME,
|
|
"You probably forgot to update your %s file following"
|
|
% GITREVIEW_FILENAME,
|
|
"the creation of this branch.",
|
|
'',
|
|
"Please create a commit which updates the value",
|
|
"of %s in the file `%s'"
|
|
% (DEFAULTBRANCH_CONFIG_NAME, GITREVIEW_FILENAME),
|
|
"and set it to `%s' (instead of `%s')."
|
|
% (self.short_ref_name, config_map[DEFAULTBRANCH_CONFIG_NAME]))
|