477 Commits

Author SHA1 Message Date
Joel Brobecker
d1c4b37752 treat all refs/users/.* references as internal and to be ignored
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
2022-03-04 18:08:33 +04:00
Joel Brobecker
f92492c718 replace distutils.version by packaging.version
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
2022-03-04 18:00:05 +04:00
Joel Brobecker
0f8e59b55f testsuite/README.md: Small formatting adjustments
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
2022-01-10 07:37:49 +04:00
Joel Brobecker
3aa6c961e0 testsuite/README.md: Add documentation on how to create testcases
Change-Id: I8cbef3817a63f00dcef84a3ecb0cec4af1720928
TN: V104-006
2022-01-10 07:35:57 +04:00
Joel Brobecker
ef6c560214 exclude ignored refs from git-notes commit emails information
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)
2022-01-06 08:09:05 +04:00
Joel Brobecker
a4f382e8bf Fix "to allow us to allow us" duplication in script documentation
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
2022-01-04 07:54:02 +00:00
Joel Brobecker
2ee1b2c47e Git Notes commit emails: include references containing annotated commit
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
2022-01-04 07:54:02 +00:00
Joel Brobecker
f778d395eb split_ref_name: New function, factorized out of AbstractUpdate class
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
2022-01-04 07:54:02 +00:00
Joel Brobecker
0e7b6ff7d5 Add support for new force-precommit-checks config option
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
2022-01-04 07:54:02 +00:00
Joel Brobecker
c6345d987f new TestcaseFixture.update_git_hooks_config method (code factorization)
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
2022-01-04 07:54:02 +00:00
Joel Brobecker
e5b28c6890 Add support for adding a warning banner when sending emails
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
2022-01-04 07:54:02 +00:00
Joel Brobecker
8821aec0fd Force the use of quoted-printable as the Content-Transfer-Encoding
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
2022-01-04 07:54:02 +00:00
Joel Brobecker
fd3ce75941 conftest.py: Simplify Run.cmd_out (remove support for old Git versions)
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
2021-12-15 10:25:47 +00:00
Joel Brobecker
12f6f66008 post_receive_when_ignore_refs: Avoid failure with older Git
Found while running the testsuite on Ubuntu 20.04, where the version
of Git is 2.25.1.

Change-Id: I1d441c78f12b98605e171d0f9290c85a3801fde0
2021-12-13 02:25:06 +00:00
Joel Brobecker
752787134a fast_forward.py: Fix arguments in call to warn from InvalidUpdate handler
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
2021-11-30 17:58:55 +04:00
Joel Brobecker
8b5d28174e Remove all imports of __future__.print_functions in the testsuite
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
2021-11-30 17:58:55 +04:00
Joel Brobecker
a26ec7a7ea update/post_receive: Do nothing if reference in hooks.ingore-refs
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
2021-11-30 17:41:32 +04:00
Joel Brobecker
7bc1c15ff6 UB27-007__commit_email_formatter_diff_multiple_charsets: New testcase
Note that I verified that this testcase reproduces the issue
reported at https://github.com/AdaCore/git-hooks/issues/19
when using the legacy-python2 branch:

    | $ echo d408c32bba64a79fc60dbda8fa047524f353e7cd \
    |        28fbd651f8161bcba746f5dbcecf5ec757241c1d \
    |        refs/heads/master | \
    |        ./hooks/post-receive
    | [...]
    | UnicodeDecodeError: 'utf8' codec can't decode byte 0xf1 in
    |   position 425: invalid continuation byte

This testcase allows us to confirm that this issue was resolved
during the transition to Python 3.x.

Change-Id: Ia94dc584577e3b4d145fad7b7f89429a1b031849
2021-11-30 17:39:38 +04:00
Joel Brobecker
34707049ad testsuite/README-testsuite.md: Add new README for the testsuite.
Change-Id: I857c1cdfbd7e15be697320b09b3e2aab050e6b43
2021-11-30 17:39:38 +04:00
Joel Brobecker
09381ab4a1 testsuite: Update expected output to Git 2.32 (with 2.25 compatibility)
This commit upgrades the testsuite's expected output to the output
produced by Git version 2.32.0. However, in an effort to make it easier
for users to run the testsuite in other environments, a bit of testsuite
infrastructure is also introduced whose role is to adapt the expected
output to the version actually available during testing. The initial
version of this adaptor allows us to maintain testsuite compatibility
with Git version 2.25.x.

Change-Id: I6fc353c92c6e680c1a6155d4f961c0b1243e4133
2021-11-30 17:39:38 +04:00
Joel Brobecker
4b927b0a45 Remove remnants of old Python-2.x-based testsuite framework
Now that the hooks are Python 3.x-only, and that we have a new framework
to run the testsuite with Python 3.x, delete the old Python-2.x-based
testsuite framework.

Change-Id: If8f72a3a8f747f768f673dcca42c63f592ee57f7
TN: U530-006
2021-10-19 13:18:07 -07:00
Joel Brobecker
6f3ee523bf Port the testsuite to pytest and Python 3.x
This commit introduces an entirely new testsuite framework based on
pytest, and then converts all our existing testcases to this new
framework.

Note: One helpful way to review this patch is to use "git show -w";
      For the most part, this patch just changes the indentation level
      of existing code. By asking "git show" to hide changes which only
      involve changes in spaces, the reviewer is left with the "essence"
      of the change.

Note: There are some testcases where the content transfer encoding changes
      from 8bit to base64. This is an enhancement brought by using a more
      recent version of Python, and in particular of the email support
      classes it provides.

Note: There are commits where the "[Errno 2] No such file or directory"
      error message now includes the path that's missing. E.g:
        - cannot_find_style_checker
        - update_hook_not_found

Still left to do:

  - Remove the old testsuite framework;
  - Remove the py2-only code;
  - Remove references to __future__;

These will be taken care of via separate commits (for ease of review).

Change-Id: Ib44bda6d5e12be44f73c7ea65d6f6e610efb872d
TN: U530-006
2021-10-19 13:17:32 -07:00
Joel Brobecker
9c82498e7b Introduce (the concept of) git command output decoding
This commit is preparation work for the transition to Python 3.x,
where the output obtained by running Git commands will become
bytes as opposed to a string. In the vast majority of cases,
we'll want to decode that output into a string. Ideally, we would
want to do this in a way that is both compatible with Python 2.x
and Python 3.x, but we have found that this requires a lot of
work with many changes spread all over the code. So, instead,
what this commit does is introduce the concept of decoding
the output, but with the decoding only occurring when running
under Python 3.x.

That way, we can make progress towards Python 3.x while preserving
the behavior under Python 2.x intact.

Change-Id: I189577798ee96cba1fa55c7356babf102575642f
TN: U530-006
2021-10-06 11:27:20 -07:00
Joel Brobecker
6d9cc85f1d commit_filer_non_ascii_diff: Remove unused import (Header)
Found while working on U530-006 (transition to Python 3.x).

Change-Id: I1e8acfed478e67b94fafdac990c98398421981cb
2021-10-04 08:16:01 -07:00
Joel Brobecker
40bf19a92e commit_filer_non_ascii_body: Remove unused import (Header)
Found while working on U530-006 (transition to Python 3.x).

Change-Id: I6ff544fa0cf424b34714d3d0622beccc4c4105b8
2021-10-04 08:16:01 -07:00