Git help for kernel archeology, suppress diffs caused by CVS keyword expansion

Git help for kernel archeology, suppress diffs caused by CVS keyword expansion

[Jon Smirl]

It would really be useful if git diff had an option for suppressing
diffs caused by CVS keyword expansion. I run into this problem over
and over when trying to recover stuff out of old kernel sources that
people checked into CVS and then posted CVS diffs to fulfill their GPL
obligations. I sometimes wonder if vendors are doing this on purpose
to make it more difficult to recover the changes they made to the
code.

To prevent this in the future, I'd love to see a patch removing all
CVS keywords from the kernel sources. Quick grep of kernel shows 1,535
$Id, 197 $Log, 441 $Revision, 144 $Date. I guestimate that this would
remove about 5,000 lines form the kernel source and touch 1,700 files.
Would this be accept or do those expansions contain useful info?

Example diff caused by $Log expansion:

diff --git a/arch/cris/kernel/ptrace.c b/arch/cris/kernel/ptrace.c
index b3d10fb..32e47ca 100644
--- a/arch/cris/kernel/ptrace.c
+++ b/arch/cris/kernel/ptrace.c
@@ -8,6 +8,9 @@
  * Authors:   Bjorn Wesen
  *
  * $Log: ptrace.c,v $
+ * Revision 1.2  2003/06/26 21:08:16  vangool
+ * Commit kernel changes.
+ *
  * Revision 1.8  2001/11/12 18:26:21  pkj
  * Fixed compiler warnings.
  *

Or in this case the person edited out all of the lines in the kernel
containing $Id

diff --git a/arch/i386/boot/tools/build.c b/arch/i386/boot/tools/build.c
index 2edd0a4..792aeb1 100644
--- a/arch/i386/boot/tools/build.c
+++ b/arch/i386/boot/tools/build.c
@@ -1,5 +1,4 @@
 /*
- *  $Id: build.c,v 1.5 1997/05/19 12:29:58 mj Exp $
  *
  *  Copyright (C) 1991, 1992  Linus Torvalds
  *  Copyright (C) 1997 Martin Mares

A variation on this where the Id was updated..
- *  $Id: build.c,v 1.5 1997/05/19 12:29:58 mj Exp $
+ *  $Id: build.c,v 1.5 2002/09/29 12:29:58 mj Exp $

There are a more CVS keywords that can cause expansion, probably around twenty.
When removing these with a simple grep it can get fooled and mess up.
CVS keyword expansion adds about 60,000 lines of noise to a typical
diff.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Git help for kernel archeology, suppress diffs caused by CVS keyword expansion

[Linus Torvalds]

On Sun, 22 Jul 2007, Jon Smirl wrote:
>
> It would really be useful if git diff had an option for suppressing
> diffs caused by CVS keyword expansion.

I really think it's not a "git diff" issue, but it might be a "import" 
issue.

IOW, I think you'd be a *lot* better off just not importing those things 
in the first place (which is what CVS does internally), or possibly 
importing them as two trees (ie you'd have the "non-log" version and the 
"log expansion" version, so that you can track and compare both).

Doing the thing at "diff" time is certainly possible, but this is simply 
much better done as a totally independent preprocessing phase. The diff 
handling is already some of the more complex parts (and very central), it 
would be much simpler and efficient to not try to make that thing fancier, 
and instead solve the problem at the front-end.

			Linus

Re: Git help for kernel archeology, suppress diffs caused by CVS keyword expansion

[Jon Smirl]

On 7/22/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Sun, 22 Jul 2007, Jon Smirl wrote:
> >
> > It would really be useful if git diff had an option for suppressing
> > diffs caused by CVS keyword expansion.
>
> I really think it's not a "git diff" issue, but it might be a "import"
> issue.
>
> IOW, I think you'd be a *lot* better off just not importing those things
> in the first place (which is what CVS does internally), or possibly
> importing them as two trees (ie you'd have the "non-log" version and the
> "log expansion" version, so that you can track and compare both).
>
> Doing the thing at "diff" time is certainly possible, but this is simply
> much better done as a totally independent preprocessing phase. The diff
> handling is already some of the more complex parts (and very central), it
> would be much simpler and efficient to not try to make that thing fancier,
> and instead solve the problem at the front-end.

These diffs are coming from companies doing GPL compliance without
really wanting to comply. CVS servers are not made available.


>
>                         Linus
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: Git help for kernel archeology, suppress diffs caused by CVS keyword expansion

[David Kastrup]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Sun, 22 Jul 2007, Jon Smirl wrote:
>>
>> It would really be useful if git diff had an option for suppressing
>> diffs caused by CVS keyword expansion.
>
> I really think it's not a "git diff" issue, but it might be a "import" 
> issue.
>
> IOW, I think you'd be a *lot* better off just not importing those
> things in the first place (which is what CVS does internally), or
> possibly importing them as two trees (ie you'd have the "non-log"
> version and the "log expansion" version, so that you can track and
> compare both).

One problem is that those strings more often than not are involved in
some magic computation, like placing version and date information into
extracted output.  While an import of the unexpanded version is really
the sanest option, it might render the resulting code inoperable.

CVS diff itself reports those differences, too (and it makes for a
good quota of merge problems even in CVS), so it is not like we are in
bad company here.

Uh, strike that.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Git help for kernel archeology, suppress diffs caused by CVS keyword expansion

[Linus Torvalds]

On Sun, 22 Jul 2007, Jon Smirl wrote:

> On 7/22/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > 
> > 
> > On Sun, 22 Jul 2007, Jon Smirl wrote:
> > >
> > > It would really be useful if git diff had an option for suppressing
> > > diffs caused by CVS keyword expansion.
> > 
> > I really think it's not a "git diff" issue, but it might be a "import"
> > issue.
> > 
> > IOW, I think you'd be a *lot* better off just not importing those things
> > in the first place (which is what CVS does internally), or possibly
> > importing them as two trees (ie you'd have the "non-log" version and the
> > "log expansion" version, so that you can track and compare both).
> > 
> > Doing the thing at "diff" time is certainly possible, but this is simply
> > much better done as a totally independent preprocessing phase. The diff
> > handling is already some of the more complex parts (and very central), it
> > would be much simpler and efficient to not try to make that thing fancier,
> > and instead solve the problem at the front-end.
> 
> These diffs are coming from companies doing GPL compliance without
> really wanting to comply. CVS servers are not made available.

That wasn't what I said. 

You want to supporess the CVS keyword expansion.

I'm telling you that you should just do so.

But I'm *also* telling you that this has nothing to do with "git diff".

The way to get "git diff" to not show the CVS expansion is to *remove* the 
CVS expansion. By simply running some pre-processing on the patches 
*before* you put them into git in the first place (or, like Dscho 
suggested: you could do it later too, by using git filter-branch).

Once you have the version that doesn't have the CVS log, "git diff" will 
automatically do the rigth thing.

In other words, I'm just saying that you're trying to solve the wrong 
problem. Once you solve the *right* problem, the wrong problem just goes 
away.

			Linus

Re: Git help for kernel archeology, suppress diffs caused by CVS keyword expansion

[Jan Engelhardt]

On Jul 22 2007 14:48, Jon Smirl wrote:
>
> It would really be useful if git diff had an option for suppressing
> diffs caused by CVS keyword expansion. I run into this problem over
> and over when trying to recover stuff out of old kernel sources that
> people checked into CVS and then posted CVS diffs to fulfill their GPL
> obligations. I sometimes wonder if vendors are doing this on purpose
> to make it more difficult to recover the changes they made to the
> code.
>
> To prevent this in the future, I'd love to see a patch removing all
> CVS keywords from the kernel sources.

I say: yes please. Even svn (which Linus dislikes very much) has cvs
keywords disabled by default. And at the same time, a patch to
CodingStyle not to include CVS tags, and a patch to checkpatch.pl
to enforce it :)

> Quick grep of kernel shows 1,535 $Id, 197 $Log, 441 $Revision, 144 $Date. I
> guestimate that this would remove about 5,000 lines form the kernel source
> and touch 1,700 files. Would this be accept or do those expansions contain
> useful info?

Common argument for $Id$ tags has been, that upon a problem, a user
can look into dmesg (or whatever log is appropraite - it's not
limited to the kernel) for the version string instead of figuring it
out himself (by means or rpm -q, or whatever hides it).

For the Linux kernel however, I do not think it is important, since:

- the code of a module changes faster than you can think (e.g.
  a change in the network code touched a network filesystem). Result:
  same $Id$ tag in the vendor's version AND the mainline version.
  So the tag is useless.

- you know what version you are running, mingetty usually prints it
  for you; uname -a also shows it.

- if you have a problem, people kindly ask you to bisect anyway, in
  which case $Id$ numbers become irrelevant anyway, because you got
  the git SHA1 ids.

Yep, that's pretty much it from my side. Conclusion: Remove CVS
tags from kernel.



	Jan
--