We noticed that Gerrit's ref-updated hook was reporting some
errors when Gerrit makes updates to some of its internal references.
For instance:
| Traceback (most recent call last):
| File "./hooks/post_receive.py", line 126, in <module>
[...]
| errors.InvalidUpdate: ('Unable to determine the type of reference
| for: refs/changes/01/104201/meta', '',
[...]
This commit avoids this error by making sure that we properly
ignore all references that match the hooks.ignore-refs config
(note that, by default, hooks.ignore-refs includes matches for
those special Gerrit references).
A new testcase is added to verify that this is the case.
Note that the minor reformatting in maybe_update_hook comes from
black, which adds the extra coma only when the changes made in
this commit are made as well. This is why this reformatting change
is bundled with this commit.
Change-Id: Id94129ad1a9d84ccba6acafef3d8e98f62ec0239
TN: UA21-052
The goal of this commit is to include the updates.sendmail module
in our testing strategy, in order to make sure that the hooks are
passing email data down to the sendmail program without issues.
This will become particularly important when we switch over to
using Python 3.x, because of the strong distinction between bytes
and strings with newer versions of Python which can cause a lot
problems. Hence the need to use this code during our testing.
The main strategy introduced by this commit to achieve this is
fairly simple: The testsuite framework introduces a new minimal
script to be called in place of the standard sendmail. A new
environment variable called GIT_HOOKS_SENDMAIL is introduced
allowing the testsuite to tell the hooks to use its own (fake)
sendmail instead of the system one. With that in place,
the old code bypassing the use of updates.sendmail can be removed,
thus allowing the testsuite to include it as part of the testing.
The testsuite's (fake) sendmail script was written in a way to
mimick the old bypassing code, so there is no change in output.
Parallel to that, the hooks are enhanced to check that we can
indeed find sendmail, and otherwise return immediately with
an error if not. This way, we avoid emails silently being
dropped due to the missing sendmail.
A couple of testcases are also added to double-check some
specific error situations.
Note that I tried to think of ways to split this patch into
smaller individual parts, but couldn't really find a way to
do so in a meaningful way, while at the same time producing
a commit where the coverage report stays clean (0 lines missed).
TN: U530-006 (transition to Python 3.x)
TN: U924-032 (test for sendmail not found)
TN: U924-034 (test for sendmail override when in testsuite mode)
Change-Id: I74b993592ec6d701347bbca5283a42e037411f1c
This commit updates the pre-commit hook to black version 21.5b1.
The hooks where then re-run on all files to update their formatting
to this new version of black.
Change-Id: Ib0866745ef8432cf93380a4d83fa23a479eb4a49
This commit changes some code which currently only works with
Python 2.x, to make it compatible with both Python 2.x and
Python 3.x.
TN: U530-006
Change-Id: I4874452cdbb26410dfe7a2d491ed400e6b038029
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
The idea is to introduce a new set of configuration options allowing
repositories to set alternate naming up for branches and tags.
And, in order to have the default to recognize all standard names
while at the same time give users an option to restrict the name
of the branches/tags that one can have in the standard naming,
we are introducing another option called...
use-standard-{branch,tag}-ref-namespace
... whose default is True.
For repositories that want to add alternate naming schemes without
restrictions in the standard branch/tag namespaces, they just add
the alternate branch/tags names via (with "xxx" being a regular
expression):
{branch,tag}-ref-namespace = xxx
For repositories that want to restrict the branch/tag naming within
the standard namespaces, they start by dropping the standard
ref naming, and then add the ref-naming that they want to allow:
use-standard-{branch,tag}-ref-namespace = false
{branch,tag}-ref-namespace = xxx
{branch,tag}-ref-namespace = yyy
To support this feature, the engine for determining the type of
update we are facing has been made more dynamic. Because things
are more flexible, it can be harder to understand what is happening
when users are faced with an error. In that context, a reasonable
amount of effort is spent trying to provide helpful error messages
for the more common error cases. More far-fetched scenarios are
covered by testing, but no effort is made to provide detailed
diagnostic (at least not yet).
A fair amount of testing has been added, covering a matrix of scenarios.
Some existing tests also had to be adapted as follow:
- check_update_utest: Adapt to the new (more precise) error message;
- post_receive_one_utest: The scenario we were using prior to
this change now triggers an exception, and so we no longer
reach the code we wanted to cover with that scenario. So modify
the repository data, and try to push a commit which is a tag
instead, using a reference name that indicates a branch.
Change-Id: I0b2404c4c4b0d240e9ad29458fb75ddd1cd7dc7d
TN: T209-003
This commit enhances the hooks to read the repository's configuration
from the most recent version. In particular, if an update is updating
the refs/meta/config reference, then read the configuration from that
update, instead of reading from the existing refs/meta/config reference.
For updates of all other references, the behavior remains unchanged.
A testcase (T227-003__meta_config_is_immediate) is added to verify
that refs/meta/config reference creation works as expected.
Following that change, various testcases also needed to be adapted
as follow:
- MC27-006__meta_config_branch_create: Creating the refs/meta/config
reference now works;
- QC13-022__checker_config_on_its_own: This testcase was pushing
a local branch called "meta-config" to refs/meta/config. This
update introduced a style-checker-config-file configuration. Since
the configuration is now taken into account right away, the update
is was rejected due to the file not existing.
The branch "meta-config" is renamed to "meta-config-missing",
and a new branch "meta-config" was created, containg the missing
file in addition to the configuration changes above.
We first try to push the "meta-config-missing" to verify
that this is rejected. We then try to push the (new) "meta-config"
branch, verifying that the presence of the config file allows
the push to be accepted.
- T209-001__update_hook_not_found: A similar situation to the above,
but with the update hook. The added comments in the testcase
should make the changes self-explanatory.
- T209-001__update_hook_reject: Modify the update-hook, which is
called during reference updates, to not reject the refs/meta/config
update the testcase is doing as part of the preliminary setup phase.
Change-Id: I0bd7bc0801ba08cd281a751759bba8c48ba0c454
TN: T227-003
This option allows users to specify a script to be run at update time,
for each reference being updated. This script can then be used to perform
additional checks on top of the standard checks already provided by
our git-hooks.
Change-Id: Iff527f1c9c0ba516ea5181c5f8c066c5175ef0ee
TN: T209-001
Not sure why, but trying to create sockets on some machines triggered
an error, maybe because of insufficient privileges or unportable
socket usage. As a result, all pushes were systematically rejected,
with the hooks thinking that they had detected a concurrent push
request.
This patch re-implement the locking mechanism using a filesystem-based
approach (hard links).
For M903-031.
The primary motivation behind this patch is simplification of
the code, after having noticed that both the update and post-receive
hooks create the object. So we can make it an attribute computed
at instantiation time, and simplify many of the methods implemented
by this class.
The second motivation behind this patch is also a slight performance
improvement, measurable when the number of files changed by a push
is getting more important. This is the change in pre_commit_checks.py,
which now takes an extra argument providing the project/repo name,
instead of recomputing it (involving running git) for each and every
file being checked.
The performance gain is fairly modest, measured to be around 4ms per
file being checked, but worth taking, given the fact that it does not
complicate the code in pre_commit_checks.
... rather than have EmailInfo.__init__ do it but controlled via
a parameter. We only want to do it once at the start of the post-
receive phase. Doing it this way ensures that it happens exactly
as planned, in case we (slightly unintentionally) instantiate
this class more than once - and with print_warnings left as the
default, which is True.
The fact that utils.py imports config.py makes it difficult for
config.py to access the InvalidUpdate exception. So move this
exception to its own package, free of dependencies.
Preparation work for LC27-007.
This attribute is used to store the value of all references
before an update is applied.
This was computed on-demand by expand_new_commit_to_list, but we want
to make this a parameter of this function, in order to allow us to
call this function from any hook. In particular, in the post-receive
hook, all reference values have already been updated, so the only
way to get the value of a reference prior to the update is by reverse-
applying the ref updates passed by argument to the post-receive hook.
The ultimate goal of this patch is to re-use expand_new_commit_to_list
during the phase where we send emails for each new commit.
This patch introduces the same factory-like mechanism used in
the post_receive hook. The intent is to merge the code from
post_receive into this infrastructure.
This is a followup on one of the previous changes. It wasn't made
earlier, in order to avoid mixing code changes and file renamings.
Keeping them separate makes it easier to track what change was made.