We noticed that Gerrit can create some references using the following
naming scheme:
refs/users/<last-2-digits-of-userid>/<userid>
When it does, this causes the hooks to fail with a message indicating
that it is unable to determine the type of reference it is, because
it doesn't recognize it.
This commit changes the hooks.ignore-refs default configuration
to ignore all references whose name start with "refs/users/".
We could be stricter in our matching, but I think this would be
slightly overkill. And meanwhile, we can see that this is not
the first time we notice that references in the "refs/users/"
namespace where created by Gerrit (e.g. we were already ignoring
references matching "refs/users/.*/edit-.*" before). So, being
being more relaxed here helps us avoid having to change the config
again in the future should Gerrit decide to expand on its use of
this namespace.
Change-Id: I3196cfc833ac94473d4b5fcdca773ded4ced24cf
TN: V218-013
distutils.version is now deprecated, so transition to packaging.version
instead. Unfortunately, the Version object from packaging.version
is not as convenient to use as distutils.version, as the comparison
operators do not support the use of strings as the righ-hand-side.
So users of those objects are updated accordingly.
Change-Id: I72aeb40050e5da25ead616b3d9475218ba88e403
TN: V304-012
This commit adjusts the formatting used in the blocks providing
the command to run the testsuite, by using the "console" identifier,
as is already done in other parts of the git-hooks READMEs.
In addition, it removes the identation that was inserted at
the start of each command, as this indentation isn't consistently
followed and doesn't really serve any real purpose.
Change-Id: I3c2a0b84ff3b1c0d0a30d0ec5371681cb64d3d17
TN: V104-006
A recent change enhanced the emails being sent for git notes
to also provide the list of branches (technically, the references)
which contain the annotated commit (S731-057). For instance:
| Subject: [notes][repo/branch] Annotated commit subject
^^^^^^^^^^^
And also this section in the "Diff:"
| For the record, the references containing the annotated
| commit above are:
|
| refs/heads/branch
However, there is a small hole in the implementation. I forgot
to take into account the "hooks.ignore-refs" config. As a result,
for repositories hosted on Gerrit, the emails mention references
which are internal to Gerrit.
| Subject: [notes][repo/master,(refs/changes/67/108467/1)] subject
^^^^^^^^^^^^^^^^^^^^^^^^^^
... and ... in the email:
| For the record, the references containing the annotated commit
| above are:
|
| refs/changes/67/108467/1
| refs/heads/master
This commit fixes this oversight.
Change-Id: I4e21d5c906e94c01b650615282258cc7b4fb81d9
TN: S731-057 (ticket introducing this feature)
TN: V105-012 (ticket opened to fix the oversight)
This commit updates the documentation of the hooks.no-emails
configuration option with some information which I had to fish
out of the implementation in order to confirm my understanding.
Change-Id: I1468265a715503735bb479fbe4e75ca63f3f5189
Pierre noticed during review of an earlier patch that
a script used by the associated testcase had some documentation
saying "to allow us" twice. I fixed the issue for the new testcase
introduced in the previous commit, but I knew that this script
was a copy of a script which is quite standard across the testsuite.
This commit fixes the issue for all the other instances of that
same script (all 111 of them ;-)).
For the record, the change was performed entirely manually
using the following command:
$ perl -pi -e 's/to allow us to allow us/to allow us/g' \
`git grep 'to allow us to allow us' | cut -d: -f1 | sort -u`
Change-Id: Ia532067b1bcdac22f72b874d67f0f210fdf83862
TN: V103-002
This commit enhances the commit emails we sent for Git Notes updates.
The goal is to include the names of the branches that contain the
annotated commit, similar to how we include the name of the branch
for regular commit emails, implemented in a way that this feature
covers all references, rather than just all branches (some projects
have branches under a non-standard namespace).
In summary, notes commit emails have their subject changed from...
[notes][repo] annotated commit subject
... to...
[notes][repo/branch1,branch2] annotated commit subject
Note that, if only one branch contains the annotated commit,
and that branch name is "master", then the subject remains
unchanged. This follows a practice we already have for regular
commit emails.
This commit also introduces a new config option called
max-ref-names-in-subject-prefix, which controls how many
such branch/reference names we include in the subject,
to avoid making that subject too long.
The full list of branches/references containing the annotated
commit is also included, unabridged, at the start of the "Diff:"
section.
Change-Id: I61ef0c497862f1243d3435a429120d63a27e4b3b
TN: S731-057
This commit factorizes some code out of the AbstractUpdate class
into its own function, so that future code can use it.
This is preparation work for a later change where we want to enhance
commit emails sent for notes updates, where we want to include
in the information provided in the email the list of references
that contain the commit being annotated by each notes commit.
We want to provide that information in a way that's as readable
as possible, and this starts by spliting the reference name.
Hence this factorization.
One thing this refactorization made us realize is that Git currently
does not allow a push on references whose name does not have what
we call a namespace! This became apparent with this commit because
it changes slightly the way the code is written, exposing the fact
that this scenario wasn't covered by our testsuite. It was while
trying to add coverage for that scenario that I realized that Git
rejects such pushes. In the spirit of being flexible in what we accept,
the code is left as is, rather than replaced by an error or an assertion.
It is then validated via unit testing.
Change-Id: I3108d540ac82c7353fd5e73a12cd958f0877245a
TN: S731-057
This commit add support for a new force-precommit-checks config
option, which allows projects to request that, on branches of
their choice, the precommit checks always be performed, even
when the commits were already present in the repository via
other references (pre-existing).
Change-Id: Ie24099ee7804d3eeec7d8d333a09811f48e5bf32
TN: V103-002
This commit introduces a new update_git_hooks_config method
in our TestcaseFixture class which implements an operation
that many testcase need to do: Update the bare repository's
git-hooks config (in the project.config file provided by
the special reference called refs/meta/config).
Several testcases are simplified by using the new method.
Change-Id: I419c0957bbd5c62c8603308797dedf319b89e409
This warning banner is triggered by the presence of an environment
variable to be set by the user prior to manually calling the
post_receive hook. This banner is then inserted at the beginning
of the email body in order to warn all readers of that email
that the email was not automatically sent at the time of push,
but rather at a later time.
TN: UC02-038
Change-Id: I42dece88dd0619df33adcc44d7adfc3ccd63a162
This commit changes the hooks to always use the quoted-printable
encoding when sending emails. This replaces the use of 7bit, 8bit
and base64 encodings. A comment is added explaining the reasons
for choosing this encoding.
For the record, the expected output in our testsuite has been
adjusted automatically using the following Python script...
| #! /usr/bin/env python3
|
| import argparse
|
| def fixup(filename):
| with open(filename) as f:
| contents = f.read()
|
| new_contents = contents.replace(
| """\
| remote: DEBUG: MIME-Version: 1.0
| remote: Content-Transfer-Encoding: 7bit
| remote: Content-Type: text/plain; charset="utf-8"
| """,
| """\
| remote: DEBUG: Content-Type: text/plain; charset="utf-8"
| remote: MIME-Version: 1.0
| remote: Content-Transfer-Encoding: quoted-printable
| """,
| )
|
| new_contents = new_contents.replace(
| "remote: Content-Transfer-Encoding: base64",
| "remote: Content-Transfer-Encoding: quoted-printable",
| )
|
| if new_contents == contents:
| return
|
| with open(filename, "w") as f:
| f.write(new_contents)
|
|
| parser = argparse.ArgumentParser()
| parser.add_argument("files_list", nargs="+")
| args = parser.parse_args()
|
| for filename in args.files_list:
| fixup(filename)
... calling it as follow:
$ cd testsuite/tests
$ /path/to/myscript.py */run_test.py
I think a perl oneliner was also possible, but I couldn't get it
to work (multiline issue, I think).
This still left one testcase that needed adjustement
(post-receive_from_email); I just manually adjusted it.
Change-Id: I8455d24f8fe0b9a732c68090a21091db060e03f2
TN: TB22-001
This commit simplifies the implementation of the cmd_out property
in the testsuite's Run class by removing some output substitution
code designed to run the testsuite against versions of Git which
are now very old (the last 1.6 and 1.7 releases are from 2010 and
2012). We no longer want to support such old versions, so this code
is no longer necessary.
Note that one testcase had to be updated as a result of this change.
This is expected, as the old code was masking (stripping) some output
printed by current versions of Git.
Change-Id: I9b6c294478e4ef65ab346b7e9a48d0b333c3c6ba
We have notice by auditing the call Gerrit's ref-updated
hook that Gerrit uses a number of other references for
internal purposes. Since those are internal, the git-hooks
should simply ignore them.
Change-Id: I46841da7926be7a69fc0f8e6c1def4834eb32a5b
TN: UC02-022
This is a change that comes from the transition to Python 3.x.
The only way it was detected by the current testsuite is that
the line of code located just after the call to warn appeared
as not being covered, which was odd, since the function "warn"
is supposed to always return. To investigate this further,
I simply added an invalid line of code right after the call
to "warn"...
except InvalidUpdate as E:
warn(*E)
invalid()
... and re-ran the testsuite: clean results (and invalid is
not covered).
Looking at the code more closely allowed me to realize that,
with Python 3.x, we need to pass *E.args, not *E. As a result,
we get an exception when calling warn as follow:
TypeError: __main__.warn() argument after * must be an iterable,
not InvalidUpdate
The reason we're not seeing this error in the testsuite is
because the issue is in a part of the file which only gets
executed when that file is used as a Python script, something
that the git-hooks don't do (the git-hooks imports the module
and calls the check_fast_forward function). Understanding this
then allowed us to track the testcase that tests the script,
and see that it was checking the script's status code, but
not its output. This commit enhances the testcase accordingly.
Change-Id: Icb92d82be6e108c6f05d84f0df5b787b97783082
TN: U530-006
This commit is the result of running the following command in
the testsuite/ directory:
$ sed -i '/^from\s\+__future__\s\+import\s\+print_function\s*$/d' \
`git grep print_function | cut -d: -f1 | sort -u`
Change-Id: Idb18a24f5368434a505af42b347b8f2a1d0481b5
TN: U530-006
Now that Python 3.x is required, this import is no longer useful.
Note that this commit deliberately excludes the imports done
in the testsuite, so as to allow these changes to be reviewed
independently of the changes to be made in the testsuite.
Change-Id: I28e1857df2cf0b2f9e7ddeab00b456d6ef513755
TN: U530-006
This marker was necessary during the transition phase to tell
the coverage analyzer to ingnore some lines of code which were
only run when using Python 2.x. All those lines of code have
now been removed, so we can now remove this exclusion.
Change-Id: Ieb2256e9c2a9b8b275781640e03d90eba479bdf9
TN: U530-006