In preparation for the transition to Python 3, convert to using the
print function. The special import
from __future__ import print_function
in Python 2.7 makes "print" become the function and not the statement.
In Python 3, it has no effect. Once the transition is done and we
exclusively use Python 3, we can simply remove the imports.
The testsuite shows no regressions.
This testcase was an attempt at trying to reproduce the problem
reported in https://github.com/AdaCore/git-hooks/issues/13.
As it happens, this testcase is not sufficient to reproduce
the problem above; but it's sufficiently interesting on its own
that it's worth adding to our testsuite nonetheless, while we continue
trying to find another reproducer for the GitHub issue above.
Change-Id: I1c63d64797ebb9ccd79eca52e45e179e741c9132
TN: TB15-003
This commit fixes a crash which occurs during the validation phase
(when the "update" hook is called). This only happens when the repository
is configured with the combined-style-checking config option set.
So far, we found a couple of scenarios where this could happen:
- The user pushes a single commit, which happens to be a "revert"
commit (i.e. a commit which is created via the "git revert"
command); those commits are purposefully excluded from the list
of commits we have to check;
- The user pushes a tag, where the tag points to a commit which
already exists in the repository (something very much typical
of people pushing tags).
In both cases, we land in the AbstractUpdate.__do_style_checks method,
doing the following:
| added = self.commits_to_check
| if git_config('hooks.combined-style-checking'):
| # This project prefers to perform the style check on
| # the cumulated diff, rather than commit-per-commit.
| # Behave as if the update only added one commit (new_rev),
| # with a single parent being old_rev. If old_rev is nul
| # (branch creation), then use the first parent of the oldest
| # added commit.
| debug('(combined style checking)')
!!! -> | if not added[-1].pre_existing_p:
| [...]
The problem in the scenarios mentioned above is that, because there are
no commits to check, added is the empty list, and so trying to get
the last element of that list does not work.
This commit fixes the problem by checking the list of added commits
early, and returning right away if there are no commits to check.
As a result of this early return, a couple of tests saw their output
change, because a debug log is no longer printed. I verified that
these were for two tests where the push did not send any commits
that are new to the repository. So the change of behavior is expected.
Change-Id: I28e5ffe61ec19b8acf46b6da5c0bfe5ed0139d2c
TN: TA21-016
See the "a_tag_before_commit" testcase where we have a very similar
situation, and where the comment explicitly confirms that we should
not be checking any commit during this push.
Found while working on TA21-016.
Change-Id: I758ac7d7559cc9c76e981e47700674b163ffbcc6
The git-hooks currently incorrectly disable the check against
merge commits for commits that looks like (from the revision log)
merge commits. This commit fixes this problem and adds a new
testcase to verify this is correctly handled.
Change-Id: Ie2d12de818c43cc11971734ea12ae3dc3fc79039
TN: TA02-034.
A new testcase for this change is not necessary, because an existing
testcase was actually already exercising this scenario (and is therefore
adjusted to expect the correct behavior). An extra test within the same
testcase is also added to exercise the scenario where all commits being
pushed are new to the repository. To make the branch names clearer
in terms of which has all-new commits, and which has some preexisting
ones, the multiple-commits-accept and future-multiple-commits-accept
have been renamed to add a "-some-prexisting" suffix to them. And
in the new test, we use the "-all-new" suffix.
Change-Id: I784a5f2fb3a4619c94d667f301d5a948cf843ac0
TN: T929-030
Note that, as part of the implementation, this commit introduces
a couple of calls to Git for each commit meant to retrieve the entire
list of files in a given commit. Luckily, one of these two calls
was already part of another check (filename collisions), and
the result of that call is cached in a CommitInfo object, so
the delta is only one extra Git call. Some performance measurements
seem to indicate that this is a fast-enough operation that users
should not notice the difference in performance, even for repositories
with a very large number of files [1]. If real-life usage proves
this assessment wrong, however, we'll revisit the documented behavior
so as to allow the use of a cheaper approach (e.g perform the check
on all modified files, rather than just the new ones).
[1]: Tested for instance on the GCC repository, which has more than
87,000 files in total; the first call took roughly 100ms, and
the next ones took about 60-70ms.
Change-Id: If0eea2c4990a945a3006cdd74d7a9aca2fc770d9
TN: T811-015
This commit just changes this testcase's bare repository to add
a new branch, called bad-after-subject, pointing to the same commit
as the master branch. This new branch name was chosen to match
the name of one of the two branches used by this testcase.
The testcase is then changed to try to push its bad-after-subject
branch to bad-after-subject, rather than re-using master as
the destination again.
This change doesn't make a difference at the moment, because neither
of the two push operations performed by the test are expected to
succeed. But they are preparation work for a followup change which
provides the ability to skip the verifications causing these pushes
to fail. In that followup commit, we'll have the testcase change
the test repository's configuration, and then re-attempt the pushes,
expecting them to succeed this time. When we do that, we'll want
to push each of the two branches to different branches on the remote.
That's why this commit introduces the second branch on the remote.
Change-Id: I86e0c18ae094818821202e545e646cbcad595c25
TN: T829-004
The issue with having two methods is that the order in which those
methods are executed is not visual and can create situations
where the test is mislead because the order chosen by unittest to
execute the testing methods is different from the order they appear
in the test script. For this test, as of now, this does not matter,
because nothing gets pushed to the bare repository. However, in
an upcoming change where we add a new feature allowing users to
push commits with revision logs containing characters outside
the range allowed by default, the order will matter. Prevent
this issue by merging the two methods into a single one. The downside
is that a failure in the first test will prevent the second one
from running. But since we never allow changes that fail a test
to get pushed to production, this shouldn't be an issue in practice.
Change-Id: I4f8f17b8ae0a73055c1207de62e9615500dd9b57
TN: T829-004
Currently, all users of AbstractUpdate.__set_send_email_p_attr first
verify that the reference being updated isn't on the hooks.no-emails
list, so __set_send_email_p_attr could arguably assume that this is
the case, and add an assertion. However, from a functional perspective,
this method is perfectly capable of doing its jobs in the case of
hooks.no-emails references as well, and doing so avoid a self-induced
potential crash, which means better resilience.
Therefore, this commit moves the check in __check_max_commit_emails
down to __set_send_email_p_attr.
This change makes the hooks slightly less effective in terms of
performance in the case of references with the hooks.no-emails and
the hooks.no-precommit-checks config options set (as demonstrated
by the slight change of behavior in one of our testcases showing that
we end up making one additional to Git): However, the impact is very
minimal, and only concerns updates for which we know the hooks have
very little to do, and therefore where performance is not key.
In the case of references that only have the hooks.no-emails config
option set, the overhead consists in setting one attribute for all
commits in a list and then counting the number of commits in that list.
This is offset to some degree by the fact that we save ourselves the
need to add an assertion check which would check the hooks.no-email
config option (which consists of regular expressions). Either way,
it should be negligible.
The upside is that future users of the __set_send_email_p_attr method
can save themselves a check of the hooks.no-emails config option.
Change-Id: Ib2216c639967fe6abf2dde57db7b6f0c7e5f554e
TN: T815-009
This commit introduces a new config option hooks.commit-email-formatter
which allows projects to customize the emails being sent for new commits.
Change-Id: Ibd6b4991f15e204e0a076bb705fc8106f08ff8b4
TN: T209-006
Prior to this commit, emails are always sent with the default charset.
This is a problem when the email body contains some characters which
are not ASCII (e.g. accented letters).
This commit fixes the issue by checking the contents of the email body,
makes a best-guess as to the encoding being used for that body, and
then sets the email's charset accordingly.
A new testcase is not added, because an existing testcase already
demonstrates the issue and thus needed to be adjusted to verify
that this change has the desired effect: email_header_encoding.
This testcase verifies the behavior when sending emails for two
commits, and those two commits do indeed use different encodings
for the commit author (the first one is UTF-8, and the second is
indeed iso8859-1).
Change-Id: I20e4659ac416845f5970448d5cf97d96e061fc5d
TN: S507-007
This option allows projects to ask the git-hooks to perform custom
checks of any kind to validate every new commit being pushed.
This commit also contains a change in hooks/updates/emails.py which
enhances the email-sending debug tracing mechanism used by the testsuite,
adding support for a tracing mode where the traces are just one line
per email. Technically, this change is independent from the rest,
but it is merged with this commit nonetheless because these changes
would show up as uncovered by the testsuite without the testcases
we're adding here. That way, we maintain the principle that we always
have 100% testing coverage for any commit.
Change-Id: I0e446a95d05f5578f977af3eaa547144187eb86b
TN: T209-005
This commit introduces a new class (ThirdPartyHook), which centralizes
the handling of third-party hooks (scripts that project set up via
the configuration file). Most of the code is simply a move of the code
from maybe_call_thirdparty_hook, with a couple of enhancements:
- Ability to specify the directory from which to call the hook;
- Handling of the situation when calling the hook itself fails.
We can already see the benefits of this in function style_check_files,
where the code now necessary to call the style_checker is both clearer
and more compact.
In the future, the intent is to push this class one step further to
allow users to specify hooks to be fetched from the repository, rather
than requiring that the hooks be installed (usually by an admin) on
the machine hosting the repository. The current class' API should
allow us to implement this in a way that's transparent to all users.
Change-Id: Ie8bf8accbbfd75a91b914628fad27d780532dac4
TN: T209-005
As part of determining which Update class it needs to instantiate
for the reference udpate being evaluated, the udpate factory
computes the following information:
- The kind of reference being updated (branch? notes? tag?)
- What operation on the branch is being performed (create? delete?
update?)
- The type of object the commit targeted by the reference
(commit? tag?)
In the context of enhancing the git-hooks to allow projects to provide
custom-checks on commits, the first and third items seem like these
could be useful information to pass to those hooks. In order to allow
this, without recomputing that information, this commit enhances
the AbstractUpdate class __init__ method to add those as additional
required paramaters, and then stores them as two new attributes.
As a bonus side-effect of this change, the new_rev_type attribute
is no longer necessary, which allows us to save one external call
to Git.
In the meantime, having these new attributes means that we can use
those to cross-check, within each AbstractUpdate child class'
self_sanity_check method, that the ref_kind and object_type values
correspond to each class' expectation (in other words, we cross-check
that the factory instantiated the correct class).
One other side-effect of that change is that we are no longer calling
the get_object_type function with a null SHA1 anymore. We could
modify the function's implementation to only accept non-null SHA1
revisions, but this would denature the function, in my opinion.
There is a genuine chance that perhaps, one day, we'll need that
again. So, instead, we cover that function by adding a new unit test
instead.
Change-Id: I8fd1ce180a6e17c0401b4dee07b9bc07d2abfdda
TN: T209-005
This change modifies the git_run function, which is a generic
function at the center of all calls to Git, to stop stripping
the start of the output of whitespaces and newlines.
While running the testsuite without any stripping at all clearly
reveals why stripping of the end of the output is necessary (see
added comment for that), stripping of the start of the output
introduces some limitations. For instance, we might want to verify
that commit subjects don't start with spaces.
Since there is no reason that we know of that we would want to strip
the start of the output of Git commands, this change adjusts git_run
accordingly.
TN: T209-005
Change-Id: Ic48a10b4f6115eecc7dbe1c7bb2300ba6070d867
All the changes were performed automatically by a script, which
did the following actions:
- fixup the indentation in the refs/meta/config:project.config file
- for all test repositories that had a local branch called
'meta/config', rebase that branch on top of the change we just
made.
This affects the following testcases, which are trying to push changes
to the refs/meta/config reference, since the changes we made cause
various SHA1s to change as a result:
- meta_config_update
- meta_config_update_with_other_ref/
Change-Id: I035732c6ebb4996c644e49fc604c02c5f67eeaeb
TN: T704-001
This one is treated on its own, because the hooks_config had
two deviations from what we would typically do (those deviations
are not problems per se, but we like to keep things consistent):
- Most config options were indented with 4 spaces instead of 8;
- the filer-email config was indented using one tab.
Everything is changed to use 8 spaces instead.
Change-Id: I934a16449b6fd5db5cfa15a4e765764deca49119
TN: T704-001
This testcase uses a repository which has some branches which are based
on the bare repository's refs/meta/config "branch". Thus, after having
updating the hooks configuration in that reference, we had to rebase
those branches, which means changes in metadata (in particular SHA1s),
affecting the output obtained when using those branches during
the course of the testcase.
Change-Id: If581d8739813b6446e2bacd1dbec83ba1731da41
TN: T704-001
This is to keep the indentation method for this configuration file
consistent across all entries.
Change-Id: Ie0e5414d8167aabb6f5c7e2fa6f419765215889c
TN: T704-001
This commit is to make the indentation in the hooks' config consitent,
using only spaces. As a result of this, the commit this testcase
is trying to push (on the refs/meta/config "branch") had to be
rebased on top of that change we're making here, thus causing
some minor SHA1 changes, and there some changes in output.
Change-Id: Id89ee4f8d479b7679091d987a2b1f016cc699b58
TN: T704-001
When a project's hooks.mailinglist configuration is the name of
a script, the call to that script now includes the name of the reference
being updated as the first (and so far only) command-line argument.
This allows these projects to have different mailing list addresses
depending on the name of the reference being updated.
Change-Id: Idc920357023c2b5ec7b3b4d755e298a0f0b83bf0
TN: T209-008
This is a followup on the previous commit, which introduced a feature
allowing projects to control which branches can be deleted.
This feature is added separately, as it is only a minor enhancement,
and this allows to keep the previous commit more focused.
Change-Id: I90d630e0a5b37d092db408379193e7a863201f8b
TN: T209-004
