On 6/21/26 1:51 PM, Dumitru Ceara wrote:
> Hi Ilya,
>
> On 6/19/26 10:19 PM, Ilya Maximets wrote:
>> On 6/19/26 9:44 AM, Dumitru Ceara via dev wrote:
>>> The pre-push example hook was suggesting the usage of:
>>>
>>> checkpatch.py -1 $sha
>>>
>>> That is incorrect, checkpatch.py will then only check the last commit on
>>> the current branch, i.e., only the "-1" argument is enforced. Moreover,
>>> `checkpatch.py $sha` is also wrong, it would fail with:
>>>
>>> ERROR: Unable to parse file '<SHA>'. Is it a patch?
>>>
>>> A better way seems to be:
>>>
>>> git format-patch -1 --stdout $sha | checkpatch.py
>>
>> This one is also not bulletproof. The format-patch with the
>> default pretty format wraps the subject lines messing up the
>> subject line checks. We'll need to add the format string to
>> make that work properly or enhance the checkpatch itself to
>> accept sha.
>>
>
> Ah, my bad, I missed that.
>
>> We also need to call ./utilities/checkpatch.py instead of plain
>> checkpatch.py, AFAIU.
>>
>
> That's maybe true. However, I never run the checkpatch.py script that's
> part of the tree but instead I run a private copy of it. Just in case
> stuff creeped into the in-tree script with me missing it. That's why
> I messed up the relative path here.
>
> I can update it though.
>
> I was thinking of something like this as alternative but then you
> lose the interleaved git log + checkpatch:
>
> diff --git a/Documentation/internals/committer-responsibilities.rst
> b/Documentation/internals/committer-responsibilities.rst
> index 63acec42f8ae..c7f091452104 100644
> --- a/Documentation/internals/committer-responsibilities.rst
> +++ b/Documentation/internals/committer-responsibilities.rst
> @@ -119,16 +119,9 @@ in your ``.git`` directory and make sure to mark it as
> executable with
> while read local_ref local_sha1 remote_ref remote_sha1; do
> case $remote_ref in
> refs/heads/main)
> - n=0
> - while read sha
> - do
> - n=$(expr $n + 1)
> - git log -1 $sha
> - echo
> - git format-patch -1 --stdout $sha | checkpatch.py
> - done <<EOF
> - $(git --no-pager log --pretty=%H $local_sha1...$remote_sha1)
> - EOF
> + n=$(git --no-pager log --pretty=%H $local_sha1...$remote_sha1
> | wc -l)
> + git log -$n
> + ./utilities/checkpatch.py -$n
>
> b=${remote_ref#refs/heads/}
> echo "You're about to push $n commits to protected branch $b
> on $remote."
> ---
>
> Adding the custom format string works too but looks quite bad. :)
>
> diff --git a/Documentation/internals/committer-responsibilities.rst
> b/Documentation/internals/committer-responsibilities.rst
> index 63acec42f8ae..96aaba4d79f5 100644
> --- a/Documentation/internals/committer-responsibilities.rst
> +++ b/Documentation/internals/committer-responsibilities.rst
> @@ -119,13 +119,17 @@ in your ``.git`` directory and make sure to mark it as
> executable with
> while read local_ref local_sha1 remote_ref remote_sha1; do
> case $remote_ref in
> refs/heads/main)
> - n=0
> while read sha
> do
> n=$(expr $n + 1)
> git log -1 $sha
> echo
> - git format-patch -1 --stdout $sha | checkpatch.py
> + git format-patch -1 --stdout --pretty=format:"\
> +Author: %an <%ae>
> +Commit: %cn <%ce>
> +Subject: %s
> +
> +%b" $sha | ./utilities/checkpatch.py
> done <<EOF
> $(git --no-pager log --pretty=%H $local_sha1...$remote_sha1)
> EOF
> ---
>
> I could also look at enhancing checkpatch but OTOH this is a one time
> configuration in the maintainer's pre-push hook so I don't know if
> that's really worth it. Maybe I should just accept the "ugly"
> pretty format args.
Sometimes I find myself wanting to run checkpatch on a specific commit
that is not at the top of the current branch and end up using -N for
this and getting a lot of output that I didn't really need. So, I think,
the general -g/--git option that just passes the rest of the arguments
directly into 'git log' would be nice to have. So we could pass a SHA or
a range or any other head. AFAICT, the 'n_patches' path can be re-used
for that without a lot of trouble.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
