2021-10-27 12:44:32

by Kari Argillander

[permalink] [raw]
Subject: [PATCH] checkpatch: Remove cvs keyword check

Time has pass and we do not need these anymore as almost all people are
using git now days. Those who use cvs for kernel development probably
will handle cvs pretty well already so this check is not needed anymore.

Signed-off-by: Kari Argillander <[email protected]>
---

If someona nack about this can you also help howto fix false alarms in
fs/ntfs3/ by this cvs check. We need to use $Log in comments as it is
real name. I'm also triccered to see these false alarms. I'm not
familier with perl regex syntax and do not really understand what we
are trying to do here. Maybe also because I do not know CVS very well.

I can do patch or someone else can do it. I just want this "problem"
away as it has been sitting my todo list too long.

---
Documentation/dev-tools/checkpatch.rst | 5 -----
scripts/checkpatch.pl | 6 ------
2 files changed, 11 deletions(-)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index f0956e9ea2d8..08e53ec02f48 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -1105,11 +1105,6 @@ Others
The patch seems to be corrupted or lines are wrapped.
Please regenerate the patch file before sending it to the maintainer.

- **CVS_KEYWORD**
- Since linux moved to git, the CVS markers are no longer used.
- So, CVS style keywords ($Id$, $Revision$, $Log$) should not be
- added.
-
**DEFAULT_NO_BREAK**
switch default case is sometimes written as "default:;". This can
cause new cases added below default to be defective.
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 461d4221e4a4..6d65b748ac20 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4017,12 +4017,6 @@ sub process {
}
}

-# check for RCS/CVS revision markers
- if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
- WARN("CVS_KEYWORD",
- "CVS style keyword markers, these will _not_ be updated\n". $herecurr);
- }
-
# check for old HOTPLUG __dev<foo> section markings
if ($line =~ /\b(__dev(init|exit)(data|const|))\b/) {
WARN("HOTPLUG_SECTION",
--
2.25.1


2021-10-27 13:54:57

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Remove cvs keyword check

On Wed, 2021-10-27 at 02:16 +0300, Kari Argillander wrote:
> Time has pass and we do not need these anymore as almost all people are
> using git now days. Those who use cvs for kernel development probably
> will handle cvs pretty well already so this check is not needed anymore.

I think it's a relatively harmless thing to keep.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -4017,12 +4017,6 @@ sub process {
> }
> }
>
> -# check for RCS/CVS revision markers
> - if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {

Looks like this would be better using

if ($rawline =~ /^\+.*\b\$(?:Revision|Log|Id)\$?\b/) {

> - WARN("CVS_KEYWORD",
> - "CVS style keyword markers, these will _not_ be updated\n". $herecurr);


2021-10-27 20:44:50

by Kari Argillander

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Remove cvs keyword check

On Tue, Oct 26, 2021 at 05:26:21PM -0700, Joe Perches wrote:
> On Wed, 2021-10-27 at 02:16 +0300, Kari Argillander wrote:
> > Time has pass and we do not need these anymore as almost all people are
> > using git now days. Those who use cvs for kernel development probably
> > will handle cvs pretty well already so this check is not needed anymore.
>
> I think it's a relatively harmless thing to keep.
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -4017,12 +4017,6 @@ sub process {
> > }
> > }
> >
> > -# check for RCS/CVS revision markers
> > - if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
>
> Looks like this would be better using
>
> if ($rawline =~ /^\+.*\b\$(?:Revision|Log|Id)\$?\b/) {

As I say before I do not know much about cvs or perl regex, but I do not
get any match with your suggestion. Test strings which I have tested:

/* $Log: frob.c,v $ */
/* $Log: frob.c,v$ */
/* $Log$ */

But these can be wrong as I do not fully understand how cvs keywords
works.

>
> > - WARN("CVS_KEYWORD",
> > - "CVS style keyword markers, these will _not_ be updated\n". $herecurr);
>
>

2021-10-27 21:21:25

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Remove cvs keyword check

On Wed, 2021-10-27 at 10:35 +0300, Kari Argillander wrote:
> On Tue, Oct 26, 2021 at 05:26:21PM -0700, Joe Perches wrote:
> > On Wed, 2021-10-27 at 02:16 +0300, Kari Argillander wrote:
> > > Time has pass and we do not need these anymore as almost all people are
> > > using git now days. Those who use cvs for kernel development probably
> > > will handle cvs pretty well already so this check is not needed anymore.
> >
> > I think it's a relatively harmless thing to keep.
> >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -4017,12 +4017,6 @@ sub process {
> > > }
> > > }
> > >
> > > -# check for RCS/CVS revision markers
> > > - if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
> >
> > Looks like this would be better using
> >
> > if ($rawline =~ /^\+.*\b\$(?:Revision|Log|Id)\$?\b/) {
> As I say before I do not know much about cvs or perl regex, but I do not
> get any match with your suggestion

Meh. The regex doesn't like the \b before and after the $

This seems better:
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1784921c645da..40552da610019 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4063,7 +4063,7 @@ sub process {
}

# check for RCS/CVS revision markers
- if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
+ if ($rawline =~ /^\+.*\$(?:Revision|Log|Id)(?::.*)?\$/) {
WARN("CVS_KEYWORD",
"CVS style keyword markers, these will _not_ be updated\n". $herecurr);
}