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 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 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
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 change is mostly a cleanup which simplifies a bit the iteration
over the list of commits for which to perform the various checks,
by having that information provided via an attribute whose name
is very specific about the semantics of what to use that list for.
This was motivated by an issue we discovered recently where we forgot
to exclude some commits when calling the commit-extra-checker hook.
This happened because, no matter how simple the algorithm for
computing the list of commits to check was, that simple algorithm
was duplicated in many places. This avoids that, in the hopes that
this will prevent this same kind of issues again in the future.
Another way to view the benefits of this cleanup is that this will
help ensure consistency in terms of the list of commits to which
the various checks are applied.
Note that we could have logically split this patch into two steps,
with the first step simply renaming the "added_commits" attribute
into "new_commits_for_ref", and then a second step introducing
the "commits_to_check" attribute. In the end, both are combined
in a single commit because it seems easier to review every location
where the "added_commits" attribute was used, and make sure from
the context that the correct list of commits was chosen in each
instance the "added_commits" list used to be referenced.
Otherwise, it's harder to review the second commit adding
the "commits_to_check" attribute, because the reviewer then has to
audit the entire set of sources himself in order to make sure
no spot was missed.
No actual behavior change should result from this change, and
therefore no test is being added.
Change-Id: I93c206968800dc738d3ebe4f5424f9201875383b
TN: T929-030
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
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
Initially, this property was born an attribute. It was then converted
to a property when the attribute's value got split into two attributes.
The original name of the attribute was used for this property so as
to facilitate the transition, as well as review of that change.
Now that the transition is done, this commit renames the property
into something which is more meaningful and obvious to the reader,
and then updates all the users.
Change-Id: I29f759dfae2a71a1c9b1a65fd93fe7604b6d952a
TN: T209-005
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 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
Include original commit name and note ref name (if not default)
in subject.
Minor wording tweaks.
for S724-027
Change-Id: I9660c19876627c1259e9b7a42ea7b2f9f9669f43
The CommitInfo currently has a base_rev attribute which currently
was only half-handled. Part of it was set up by commit_info_list,
except that it wasn't really done properly, so the updates class
was re-doing it afterwards. And then we had this slightly confusing
situation I wasn't sure whether a None base_rev meant no base
(root commit), or un-calculated yet.
This patch clarifies this by getting rid of the "base_rev" attribute,
replacing it by a "parent_revs" attribute, which is either None if
un-calculated yet, or else a list, which would be empty if the commit
has no parent (root commit).
This allows us to simplify a couple of areas, but otherwise, this
is just fallout from those changes.
This should also help us prepare for having easy access to the list
of changes a commit introduces, which is going to be necessary to
handle the FSF's binutils-gdb.git repository.
For NC07-001.
... mostly into the CommitInfo object instead. This is preparation work
for allowing email-lists that depend on the changes introduced by each
commit, as is the case in the binutils-gdb.git repository.
For NC07-001.
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.
The hooks now enqueue all emails in that queue, and post_receive_email
now triggers the flushing of that queue at the end of the script
execution. The emails are still sent within the same process, but
the goal is to daemonize the part that sends it.
Part of LC28-010.
