[PATCH 1/2] checkpatch: Don't check for 75 chars per line for create/delete mode lines

[PATCH 1/2] checkpatch: Don't check for 75 chars per line for create/delete mode lines

[Saravana Kannan]

Cover letters have a "create/delete mode <mode> <filename>" line for files
added/deleted in the patch series. Ignore these lines when checking for the
maximum 75 chars per line limit.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 scripts/checkpatch.pl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c4c4a61bc83..f306634a938c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3271,6 +3271,8 @@ sub process {
 					# filename then :
 		      $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i ||
 					# A Fixes:, link or signature tag line
+		      $line =~ /^\s*(?:delete|create) mode\s+[0-8]+\s+\S+\s*$/i ||
+					# A "create/delete mode <mode> <filename>" line found in cover letters
 		      $commit_log_possible_stack_dump)) {
 			WARN("COMMIT_LOG_LONG_LINE",
 			     "Prefer a maximum 75 chars per line (possible unwrapped commit description?)\n" . $herecurr);
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 2/2] checkpatch: Don't check for unified diff format in git sendemail headers

[Saravana Kannan]

When checkpatch is used as a git sendemail-validate hook, it's also passed
in the email header for sanity check. These headers are, as expected, not
in unified diff format. So, don't complain about unified diff format for
these header files.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 scripts/checkpatch.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f306634a938c..4312166ca828 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7689,7 +7689,8 @@ sub process {
 		exit(0);
 	}
 
-	if (!$is_patch && $filename !~ /cover-letter\.patch$/) {
+	if (!$is_patch && $filename !~ /cover-letter\.patch$/ &&
+	    $filename !~ /\.git\/\.gitsendemail\.header\.\w+$/) {
 		ERROR("NOT_UNIFIED_DIFF",
 		      "Does not appear to be a unified-diff format patch\n");
 	}
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [PATCH 2/2] checkpatch: Don't check for unified diff format in git sendemail headers

[Joe Perches]

On Wed, 2024-02-21 at 21:15 -0800, Saravana Kannan wrote:
> When checkpatch is used as a git sendemail-validate hook, it's also passed
> in the email header for sanity check.

Why?

If so, why not use a front-end script to stop/remove
the file from being scanned by checkpatch?

> These headers are, as expected, not
> in unified diff format. So, don't complain about unified diff format for
> these header files.

Re: [PATCH 2/2] checkpatch: Don't check for unified diff format in git sendemail headers

[Saravana Kannan]

On Thu, Feb 22, 2024 at 12:54 AM Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2024-02-21 at 21:15 -0800, Saravana Kannan wrote:
> > When checkpatch is used as a git sendemail-validate hook, it's also passed
> > in the email header for sanity check.
>
> Why?
>
> If so, why not use a front-end script to stop/remove
> the file from being scanned by checkpatch?

Sure, I could do that. But this also makes it easier for people to
start using checkpatch. Or I can put up a git hook wrapper script in
here for people to symlink into their .git/hooks that does this.

I'd prefer the lazy route of not creating a 1 line wrapper script :)

-Saravana

>
> > These headers are, as expected, not
> > in unified diff format. So, don't complain about unified diff format for
> > these header files.
>

Re: [PATCH 2/2] checkpatch: Don't check for unified diff format in git sendemail headers

[Joe Perches]

On Thu, 2024-02-22 at 16:45 -0800, Saravana Kannan wrote:
> On Thu, Feb 22, 2024 at 12:54 AM Joe Perches <joe@perches.com> wrote:
> > 
> > On Wed, 2024-02-21 at 21:15 -0800, Saravana Kannan wrote:
> > > When checkpatch is used as a git sendemail-validate hook, it's also passed
> > > in the email header for sanity check.
> > 
> > Why?
> > 
> > If so, why not use a front-end script to stop/remove
> > the file from being scanned by checkpatch?
> 
> Sure, I could do that. But this also makes it easier for people to
> start using checkpatch. Or I can put up a git hook wrapper script in
> here for people to symlink into their .git/hooks that does this.
> 
> I'd prefer the lazy route of not creating a 1 line wrapper script :)

I'd not.  checkpatch is for _patches_.
Don't feed stuff to it that isn't patches and expect good results.

Re: [PATCH 2/2] checkpatch: Don't check for unified diff format in git sendemail headers

[Saravana Kannan]

On Thu, Feb 22, 2024 at 5:01 PM Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2024-02-22 at 16:45 -0800, Saravana Kannan wrote:
> > On Thu, Feb 22, 2024 at 12:54 AM Joe Perches <joe@perches.com> wrote:
> > >
> > > On Wed, 2024-02-21 at 21:15 -0800, Saravana Kannan wrote:
> > > > When checkpatch is used as a git sendemail-validate hook, it's also passed
> > > > in the email header for sanity check.
> > >
> > > Why?
> > >
> > > If so, why not use a front-end script to stop/remove
> > > the file from being scanned by checkpatch?
> >
> > Sure, I could do that. But this also makes it easier for people to
> > start using checkpatch. Or I can put up a git hook wrapper script in
> > here for people to symlink into their .git/hooks that does this.
> >
> > I'd prefer the lazy route of not creating a 1 line wrapper script :)
>
> I'd not.  checkpatch is for _patches_.
> Don't feed stuff to it that isn't patches and expect good results.

Would you be open to being a maintainer if I add a git hook
sendemail-validate wrapper? It feels silly to add myself as a
maintainer for a 1-line script. I'd rather give it to you :)

-Saravana

Re: [PATCH 2/2] checkpatch: Don't check for unified diff format in git sendemail headers

[Joe Perches]

On Thu, 2024-02-22 at 17:10 -0800, Saravana Kannan wrote:
> On Thu, Feb 22, 2024 at 5:01 PM Joe Perches <joe@perches.com> wrote:
> > 
> > On Thu, 2024-02-22 at 16:45 -0800, Saravana Kannan wrote:
> > > On Thu, Feb 22, 2024 at 12:54 AM Joe Perches <joe@perches.com> wrote:
> > > > 
> > > > On Wed, 2024-02-21 at 21:15 -0800, Saravana Kannan wrote:
> > > > > When checkpatch is used as a git sendemail-validate hook, it's also passed
> > > > > in the email header for sanity check.
> > > > 
> > > > Why?
> > > > 
> > > > If so, why not use a front-end script to stop/remove
> > > > the file from being scanned by checkpatch?
> > > 
> > > Sure, I could do that. But this also makes it easier for people to
> > > start using checkpatch. Or I can put up a git hook wrapper script in
> > > here for people to symlink into their .git/hooks that does this.
> > > 
> > > I'd prefer the lazy route of not creating a 1 line wrapper script :)
> > 
> > I'd not.  checkpatch is for _patches_.
> > Don't feed stuff to it that isn't patches and expect good results.
> 
> Would you be open to being a maintainer if I add a git hook
> sendemail-validate wrapper? It feels silly to add myself as a
> maintainer for a 1-line script. I'd rather give it to you :)

<shrug>

I think you need a local script and not one in the tree.
Or maybe some different git invocation command.  Dunno.

Re: [PATCH 2/2] checkpatch: Don't check for unified diff format in git sendemail headers

[Lukas Bulwahn]

On Fri, Feb 23, 2024 at 2:10 AM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Feb 22, 2024 at 5:01 PM Joe Perches <joe@perches.com> wrote:
> >
> > On Thu, 2024-02-22 at 16:45 -0800, Saravana Kannan wrote:
> > > On Thu, Feb 22, 2024 at 12:54 AM Joe Perches <joe@perches.com> wrote:
> > > >
> > > > On Wed, 2024-02-21 at 21:15 -0800, Saravana Kannan wrote:
> > > > > When checkpatch is used as a git sendemail-validate hook, it's also passed
> > > > > in the email header for sanity check.
> > > >
> > > > Why?
> > > >
> > > > If so, why not use a front-end script to stop/remove
> > > > the file from being scanned by checkpatch?
> > >
> > > Sure, I could do that. But this also makes it easier for people to
> > > start using checkpatch. Or I can put up a git hook wrapper script in
> > > here for people to symlink into their .git/hooks that does this.
> > >
> > > I'd prefer the lazy route of not creating a 1 line wrapper script :)
> >
> > I'd not.  checkpatch is for _patches_.
> > Don't feed stuff to it that isn't patches and expect good results.
>
> Would you be open to being a maintainer if I add a git hook
> sendemail-validate wrapper? It feels silly to add myself as a
> maintainer for a 1-line script. I'd rather give it to you :)
>

I agree with Joe's shrug. We got enough scripts, where very few
(actually: probably nobody) know what they are good for.

However, Saravana, if it helps you, feel free to add a section in the
checkpatch documentation where you describe which workflow you have
and in which files you need to set up what.
So, in case you forget, you will find it in the documentation and
possibly it is also helpful to others---if they read the
documentation, or some AI bot reads the documentation in the future
and then suggests it to someone asking that AI bot---well, that is our
brave new world nowadays...

I will maintain that section in the checkpatch documentation for you,
if you submit the documentation change as a proper patch.

Lukas

Re: [PATCH 1/2] checkpatch: Don't check for 75 chars per line for create/delete mode lines

[Saravana Kannan]

On Wed, Feb 21, 2024 at 9:15 PM Saravana Kannan <saravanak@google.com> wrote:
>
> Cover letters have a "create/delete mode <mode> <filename>" line for files
> added/deleted in the patch series. Ignore these lines when checking for the
> maximum 75 chars per line limit.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

I know patch 2/2 is not going to be picked up. But can we pick up this
one please?

-Saravana

> ---
>  scripts/checkpatch.pl | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9c4c4a61bc83..f306634a938c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3271,6 +3271,8 @@ sub process {
>                                         # filename then :
>                       $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i ||
>                                         # A Fixes:, link or signature tag line
> +                     $line =~ /^\s*(?:delete|create) mode\s+[0-8]+\s+\S+\s*$/i ||
> +                                       # A "create/delete mode <mode> <filename>" line found in cover letters
>                       $commit_log_possible_stack_dump)) {
>                         WARN("COMMIT_LOG_LONG_LINE",
>                              "Prefer a maximum 75 chars per line (possible unwrapped commit description?)\n" . $herecurr);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>