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 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
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
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
Note that tag deletion privileges are controlled via a different
and already existing option (hooks.allow-delete-tag). Also, notes
are not covered as notes deletion is currently not allowed at all.
Change-Id: I1ecfd83cf27861885f98c31bdc416f67d2dddb5e
TN: T209-004
Now that the default is changed, we can remove a couple of XFAILs
that were temporarily added.
Change-Id: Id15e0b5ccdf83e82329e02327773687b2c94be42
TN: T512-052
This commit puts in place the necessary infrastructure to replace
replace the hooks.bcc-file-ci configuration in favor of a new
configuration option called hooks.filer-email. The difference
is that the latter is a tuple/list.
For this commit, the default for the new option is such that, for
repositories that don't use the hooks.bcc-file-ci, nothing changes.
This default will be changed in a subsequent commit, and a comment
in the code explain why we're doing it this way.
This change affects two tests we have in the testsuite, which both
were using the hooks.bcc-file-ci config option:
- "no_file_ci_bcc": We could have made this test pass by replacing
the hooks.bcc-file-ci option by an empty hooks.filer-email.
However, this would not be representative of the configuration
of a real repository. So, instead, we just adjust the repository's
configuration to match what it would typically look like, and
accept (XFAIL) that this test is failing until we change
the default for hooks.filer-email.
- "to_file-ci_only": This test is in a similar situation as the test
above, except that it has the additional distinction of now really
becoming pointless. However, pointless or not, it's a test which
requires little maintenance (if at all), so the decision was to
keep that test and apply the same treatment as above.
A new test is also added, to verify that it's possible to provide
more than one email address in hooks.filer-email.
Change-Id: I1733e57b204c49cd189c5a6e1d3f4c47846b40d6
TN: T512-052
This commit is a simple cleanup commit, so as to make sure that,
for all options listed has being a "tuple" option, the default
value for the option is actually a tuple.
In practice, the code is such that it doesn't matter (it works with
None, or the empty string). But it's not the cleanest approach,
and there is no reason for it. It's just as easy as using an empty
tuple.
Doing so actually reduces coverage by one line, inside function
"to_tuple" in type_conversion.py. The uncovered code is actually
useful outside of in the handling of the default values extracted
from the GIT_CONFIG_OPTS dict. So, a test is added specifically
in order to help cover this line of code as well.
Seen while working on T512-052
Change-Id: I7e9a09c5514b29c18cdf7d92f2c5d3eabf0fe82b
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
When initially created, these hooks were reading the repository's
configuration from an untracked file, installed in the repository.
In December 2013, this method of configuring the hooks became obsolete,
in favor of getting the information from a file which is checked in.
However, to ease the transition, support for the untracked config
file was kept as a fallback, with a large warning asking the user
to contact the repository's admin to perform the transition.
At this point, it is believed that all repositories have now
transitioned, and therefore keeping this fallback is no longer
necessary.
This commit therefore removes support for that functionality.
A couple of testcases are adjusted accordingly. As a side-effect,
this caused the testing of branch creations for branches whose name
does not follow the standard naming scheme to no longer be tested.
This commit therefore adds one using a reference name which is not
refs/meta/config, thus ensuring that testing coverage remains at 100%.
Change-Id: I3e9c176f46e831ea82cdc930d32f7f22913a4fbc
TN: T314-004
The main purpose of that pre-receive hook is to ensure that any push
updating the refs/meta/config reference only updates that one reference.
A large comment was added to the code to explain why (in short, this is
a pre-requisite for being able to use the git-hooks config at the same
time the config changes are being pushed).
While at it, to be consistent with the other hooks, support for
a repository-specific pre-receive hook is added. One side-effect
of this is that we end up trying to query the hooks configuration,
which triggers a check for the existence of the refs/meta/config
branch. As a result, an extra copy of the same warning gets generated
when the reference doesn't exist. This is not super elegant, but
isn't a problem in practice, since this is part of handling a deprecated
feature that we'll try to remove support for hopefully soon. In the
meantime, this isn't blocking to adding support for the pre-receive
option.
Since this commit increased the number of times `refs/meta/config`
was spelled out (aka "hardcoded"), this commit introduces a new
constant to avoid duplicating that special reference name.
Change-Id: Ib4fc0a32f639fe693e4d43269baf8c0838fc9214
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
The latter is really all-encompassing, and expected to disable all
precommit checks. And it would be used in the scenario that lead
to the creation of the hooks.no-gitreview-check config. So it should
be good enough for now to simply rely on hooks.no-precommit-check.
For Q802-008.
This commit introduces a new hooks.frozen-ref (tuple) config which
allows a user, via an update of the refs/meta/config::project.config
file, to disallow changes to any reference.
Typically, this would be used when a given branch gets retired,
to prevent people from pushing updates to is, thinking that they
are updating an active branch, when in fact the branch is no longer
being used.
The same effect could be achieved using the "retired/*" namespace,
but this approach was deemed easier to manage for certain users
(and the latter approach was designed before users had access to
the hook's config file).
For Q314-009.
For instance, instead of defining hooks.no-emails as...
no-emails = refs/heads/topic*,refs/heads/wip*
... one can now say:
no-emails = refs/heads/topic*
no-emails = refsheads/wip*
For Q314-009.
Prior to this patch, the git-hooks were assuming that all options
in the project.config file were entirely contained within a single
line. This patch lifts this limitation.
For Q119-043.
This patch adds another entry to the list of references to be ignored
(in GERRIT_INTERNAL_REFS). These are the temporary references which
gerrit creates when editing a commit directly from the gerrit web
interface. These are not user-visible references, and therefore
should not influence the behavior of our hooks.
For P812-021.
