From: Jon Smirl <hidden> Date: 2007-07-22 18:49:00
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:
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
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
From: Jon Smirl <hidden> Date: 2007-07-22 19:12:19
On 7/22/07, Linus Torvalds [off-list ref] wrote:
On Sun, 22 Jul 2007, Jon Smirl wrote:
quoted
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.
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
From: Jan Engelhardt <hidden> Date: 2007-07-22 19:47:20
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
--
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:23
Hi,
[culling lkml list, since this is really a Git problem]
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.
Looks to me slightly a bit like an XY problem.
How about using git-filter-branch to get rid of the expansions? Or even
better, if you have access to the CVS server, use the -k option to
cvsimport?
Hth,
Dscho
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
Hi,
[culling lkml list, since this is really a Git problem]
On Sun, 22 Jul 2007, Jon Smirl wrote:
quoted
It would really be useful if git diff had an option for suppressing
diffs caused by CVS keyword expansion.
Looks to me slightly a bit like an XY problem.
How about using git-filter-branch to get rid of the expansions? Or even
better, if you have access to the CVS server, use the -k option to
cvsimport?
No access to the server the people handing over the diffs and not
making things easy. You get diffs like this when asking companies to
turn over their changes for GPL compliance purposes. A big source is
from companies building embedded systems., Usually the diffs are
pretty worthless but every once in a while they contain very useful
nuggets (like inadvertently documenting secret hardware features). It
is hard to spot the nuggets in all of the noise.
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:23
Hi,
On Sun, 22 Jul 2007, Jon Smirl wrote:
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
quoted
On Sun, 22 Jul 2007, Jon Smirl wrote:
quoted
It would really be useful if git diff had an option for suppressing
diffs caused by CVS keyword expansion.
Looks to me slightly a bit like an XY problem.
How about using git-filter-branch to get rid of the expansions? Or
even better, if you have access to the CVS server, use the -k option
to cvsimport?
No access to the server the people handing over the diffs and not
making things easy.
Ah yes, that's what I feared. Darn.
But still, I think that it would be much better not to put this into Git.
We do have diff gitattributes now, so that you can roll your own diff for
specific files, but I still think that this is more a task for a
standalone perl script. Possibly being called from filter-branch to be
done with the conversion once and for all times.
Ciao,
Dscho
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
But still, I think that it would be much better not to put this into Git.
We do have diff gitattributes now, so that you can roll your own diff for
specific files, but I still think that this is more a task for a
standalone perl script. Possibly being called from filter-branch to be
done with the conversion once and for all times.
I can provide sample diffs if you want something to play with.
--
Jon Smirl
jonsmirl@gmail.com
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:23
Hi,
On Sun, 22 Jul 2007, Jon Smirl wrote:
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
quoted
But still, I think that it would be much better not to put this into Git.
We do have diff gitattributes now, so that you can roll your own diff for
specific files, but I still think that this is more a task for a
standalone perl script. Possibly being called from filter-branch to be
done with the conversion once and for all times.
I can provide sample diffs if you want something to play with.
As you already did, this is my attempt at a perl script... Feel free to
bash my Perl capabilities, or to correct it...
Ciao,
Dscho
-- snipsnap --
#!/usr/bin/perl
sub init_hunk {
$_ = $_[0];
$current_hunk = "";
$current_hunk_header = $_;
($start_minus, $dummy, $start_plus, $dummy) =
/^\@\@ -(\d+)(,\d+|) \+(\d+)(,\d+|) \@\@/;
$plus = $minus = $space = 0;
$skip_logs = 0;
}
sub flush_hunk {
if ($plus > 0 || $minus > 0) {
if ($current_file ne "") {
print $current_file;
$current_file = "";
}
$minus += $space;
$plus += $space;
print "\@\@ -$start_minus,$minus "
. "+$start_plus,$plus \@\@\n";
print $current_hunk;
}
}
sub check_file {
$_ = $_[0];
$current_file = $_;
while (<>) {
if (/^\@\@/) {
last;
}
$current_file .= $_;
}
init_hunk $_;
# check hunks
while (<>) {
if ($skip_logs && /^\+ *\*/) {
# do nothing
} elsif (/^\@\@.*/) {
flush_hunk;
init_hunk $_;
} elsif (/^diff/) {
flush_hunk;
return;
} elsif (/^-.*\$(Id|Revision|Author|Date).*\$/) {
$key = $1;
s/^-/ /;
$current_hunk .= $_;
$space++;
$_ = <>;
if (!/\+.*\$Id.*\$/) {
die "Expected some changed \$$key line: $_";
}
$skip_logs = 0;
} elsif (/^ .*\$Log.*\$/) {
$current_hunk .= $_;
$space++;
$skip_logs++;
} elsif (/^ /) {
$current_hunk .= $_;
$space++;
$skip_logs = 0;
} elsif (/^\+/) {
$current_hunk .= $_;
$plus++;
} elsif (/^-/) {
$current_hunk .= $_;
$minus++;
$skip_logs = 0;
} else {
die "Unexpected line: $_";
}
}
}
while (<>) {
if (/^diff/) {
do {
check_file $_;
} while(/^diff/);
} else {
printf $_;
}
}
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
As you already did, this is my attempt at a perl script... Feel free to
bash my Perl capabilities, or to correct it...
I don't really know Perl but Perl is probably a better language for
this. I was doing it in C.
This doesn't run on the two full kernel samples I sent you. That's the
problem I was having, I can catch 95% of the expanded keywords with my
program but I have to touch up 5% by hand. I'll stare at this and see
if I can increase my understanding of Perl.
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:23
Hi,
On Sun, 22 Jul 2007, Jon Smirl wrote:
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
quoted
As you already did, this is my attempt at a perl script... Feel free to
bash my Perl capabilities, or to correct it...
I don't really know Perl but Perl is probably a better language for
this. I was doing it in C.
Yes, Perl is much much better for an application like this. You do not
have to cater for newbies, but can keep it to the technical level.
This doesn't run on the two full kernel samples I sent you.
You mean my script? Or your C program?
That's the problem I was having, I can catch 95% of the expanded
keywords with my program but I have to touch up 5% by hand. I'll stare
at this and see if I can increase my understanding of Perl.
If it is my script, please tell me what is not working. I'll fix it. In
the process, I could even add some comments...
Ciao,
Dscho
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
quoted
This doesn't run on the two full kernel samples I sent you.
You mean my script? Or your C program?
Your script, I sent two big samples directly to you as attachments
three hours ago.
Script processes them for a few pages of output and then bails out.
I have more diffs, but those two are representative of the sample
phytec patch has support for the NXP 3180 CPU which is not in the
kernel. I have a project interested in using this CPU so we are
looking at extracting the essence of 3180 support from the patch and
trying to apply it to the current kernel.
@@ -222,7 +222,7 @@ this value. NOTE: This does not fix shared mmaps, check out the sparc64 port for-one way to solve this (in particular SPARC_FLAG_MMAPSHARED).+one way to solve this (in particular arch_get_unmapped_area). Next, you have two methods to solve the D-cache aliasing issue for all other cases. Please keep in mind that fact that, for a given page
Expected some changed $Id line: Readme-File
/usr/src/Documentation/cdrom/aztcd
jonsmirl@jonsmirl:/extra/diffs$
--
Jon Smirl
jonsmirl@gmail.com
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
Okay, I did not really test thoroughly, it seems. Sorry. Next try.
It's working a lot better. People deal with $Id two different ways.
One way they delete all of the $Id lines, that is the case of the
Sonos patch. In the Phytec patch they left all of the $Id lines in
place which caused them to get modified. In both cases you just want
the lines with $Id to disappear in the patch.
It doesn't catch the $Id case from the Phytec patch.
diff -uarN linux-2.6.10/arch/cris/arch-v10/boot/rescue/head.S
linux-2.6.10-lpc3180/arch/cris/arch-v10/boot/rescue/head.S
@@ -1,4 +1,5 @@ /* $Id: head.S,v 1.6 2003/04/09 08:12:43 pkj Exp $+/* $Id: head.S,v 1.2 2005/02/18 13:06:31 mike Exp $ * * Rescue code, made to reside at the beginning of the * flash-memory. when it starts, it checks a partition
It's not catching all of the $Revision and $Date deltas.
The output diff shouldn't contain any CVS keywords. It is somewhat
tricky to catch all of the cases and fix up the diffs. This filter
should get written and debugged once and then made part of something
like git so that it doesn't get written over and over again. Perl is
way better for this I had 1000 lines of C in my program and it was
still missing 10% of the cases.
--
Jon Smirl
jonsmirl@gmail.com
From: Simon 'corecode' Schubert <hidden> Date: 2016-06-15 22:43:23
Jon Smirl wrote:
quoted hunk
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
quoted
Okay, I did not really test thoroughly, it seems. Sorry. Next try.
It's working a lot better. People deal with $Id two different ways.
One way they delete all of the $Id lines, that is the case of the
Sonos patch. In the Phytec patch they left all of the $Id lines in
place which caused them to get modified. In both cases you just want
the lines with $Id to disappear in the patch.
It doesn't catch the $Id case from the Phytec patch.
diff -uarN linux-2.6.10/arch/cris/arch-v10/boot/rescue/head.S
linux-2.6.10-lpc3180/arch/cris/arch-v10/boot/rescue/head.S
/* $Id: head.S,v 1.6 2003/04/09 08:12:43 pkj Exp $
+/* $Id: head.S,v 1.2 2005/02/18 13:06:31 mike Exp $
*
* Rescue code, made to reside at the beginning of the
* flash-memory. when it starts, it checks a partition
It's not catching all of the $Revision and $Date deltas.
The output diff shouldn't contain any CVS keywords. It is somewhat
tricky to catch all of the cases and fix up the diffs. This filter
should get written and debugged once and then made part of something
like git so that it doesn't get written over and over again. Perl is
way better for this I had 1000 lines of C in my program and it was
still missing 10% of the cases.
Maybe I am missing something, but apart from $Log$, what's so hard about collapsing the CVS keywords? That's something like
s/\$(Id|Date|Header|CVSHeader|Author|Revision):[^$]*\$/$\1$/
or not?
I see that people want to modify the patch text to remove the churn, but that seems wrong. It would be much easier to just collapse the keywords before applying the patch, and then work on a newly created diff (from the git repo).
cheers
simon
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:23
Hi,
On Mon, 23 Jul 2007, Jon Smirl wrote:
quoted hunk
On 7/22/07, Johannes Schindelin [off-list ref] wrote:
quoted
Okay, I did not really test thoroughly, it seems. Sorry. Next try.
It's working a lot better. People deal with $Id two different ways.
One way they delete all of the $Id lines, that is the case of the
Sonos patch. In the Phytec patch they left all of the $Id lines in
place which caused them to get modified. In both cases you just want
the lines with $Id to disappear in the patch.
It doesn't catch the $Id case from the Phytec patch.
diff -uarN linux-2.6.10/arch/cris/arch-v10/boot/rescue/head.S
linux-2.6.10-lpc3180/arch/cris/arch-v10/boot/rescue/head.S
/* $Id: head.S,v 1.6 2003/04/09 08:12:43 pkj Exp $
+/* $Id: head.S,v 1.2 2005/02/18 13:06:31 mike Exp $
*
* Rescue code, made to reside at the beginning of the
* flash-memory. when it starts, it checks a partition
It's not catching all of the $Revision and $Date deltas.
The output diff shouldn't contain any CVS keywords. It is somewhat
tricky to catch all of the cases and fix up the diffs. This filter
should get written and debugged once and then made part of something
like git so that it doesn't get written over and over again. Perl is
way better for this I had 1000 lines of C in my program and it was
still missing 10% of the cases.
Next try. It includes documentation, skips $Source:$ and $Header:$ as
well, and the output diff does not contain any CVS keywords.
Ah yes, and if a file was actually created, it substitutes the --- file
name with /dev/null. In that case, the keywords are skipped, too!
However, when a file was deleted, they are _not_ skipped.
Strictly speaking, it is wrong to just cut out the lines containing CVS
keywords, since it is possible that there is an important change on that
line, but we can cross that bridge once companies play such cute games
with us.
But at least I tested the resulting patch with your phytec3180 patch this
time...
Ciao,
Dscho
-- snipsnap --
#!/usr/bin/perl
# This is a simple state machine.
#
# There is the state of the current file; its header is stored
# in $current_file to avoid outputting it when all hunks were
# culled. It is only printed before the first hunk, and then
# set to "" to avoid outputting it twice.
#
# There are the states of the current hunk, stored in
# * $current_hunk (possibly modified hunk)
# * $start_minus, $start_plus (from the original)
# * $plus, $minus, $space (the current count of the respective lines)
# a hunk is only printed (in flush_hunk) if any '+' or '-' lines
# are left after filtering.
#
# For $Log..$, there is the state $skip_logs, which is set to 1
# after seeing such a line, and set to 0 when the first line
# was seen which does not begin with '+'.
#
# A particularly nasty special case is when a single "*" was
# misattributed by the diff to be _inserted before_ a $Log, instead
# of _appended after_ a $Log.
# This is the purpose of the $before_log and $after_log variables:
# if not empty, the state machine expects the next line to begin
# with '+' or '-', respectively, and followed by a $Log. If this
# expectation is not met, the variable is output.
#
# The variable $plus_minus_adjust contains the number of lines which
# were skipped from the "+" side, so that the correct offset is shown.
# This function gets a hunk header.
#
# It initializes the state variables described above
sub init_hunk {
$_ = $_[0];
$current_hunk = "";
$current_hunk_header = $_;
($start_minus, $dummy, $start_plus, $dummy) =
/^\@\@ -(\d+)(,\d+|) \+(\d+)(,\d+|) \@\@/;
$plus = $minus = $space = 0;
$skip_logs = 0;
$before_log = '';
$after_log = '';
# we prefer /dev/null as original file name when a file is new
if ($start_minus eq 0) {
$current_file =~ s/\n--- .*\n/\n--- \/dev\/null\n/;
} elsif ($start_plus eq 0) {
$current_file =~ s/\n\+\+\+ .*\n/\n+++ \/dev\/null\n/;
}
}
# This function is called whenever there is possibly a hunk to print.
# Nothing is printed if no '+' or '-' lines are left.
# Otherwise, if the file header was not yet shown, it does so now.
sub flush_hunk {
if ($plus > 0 || $minus > 0) {
if ($current_file ne "") {
print $current_file;
$current_file = "";
}
$minus += $space;
$plus += $space;
print "\@\@ -$start_minus,$minus "
. "+" . ($start_plus - $start_plus_adjust)
. ",$plus \@\@\n";
print $current_hunk;
}
}
# This adds a line to the current hunk and updates $space, $plus or $minus
sub add_line {
my $line = $_[0];
$current_hunk .= $line;
if ($line =~ /^ /) {
$space++;
} elsif ($line =~ /^\+/) {
$plus++;
} elsif ($line =~ /^-/) {
$minus++;
} elsif ($line =~ /^\\/) {
# do nothing
} else {
die "Unexpected line: $line";
}
}
# This function splits the current hunk into the part before the current
# line, and the part after the current line.
sub skip_line {
my $line = $_[0];
if ($start_minus == 0) {
# This patch adds a new file, just ignore that line
return;
} elsif ($start_plus == 0) {
# This patch removes a file, so include the line nevertheless
add_line $_;
return;
}
flush_hunk;
if ($line =~ /^-/) {
$minus++;
} elsif ($line =~ /^\+/) {
$plus++;
$start_plus_adjust++;
}
init_hunk "@@ -" . ($start_minus + $minus + $space)
. " +" . ($start_plus + $plus + $space)
. " @@\n";
}
$simple_keyword = "Id|Revision|Author|Date|Source|Header";
# This is the main loop
sub check_file {
$_ = $_[0];
$current_file = $_;
$start_plus_adjust = 0;
while (<>) {
if (/^\@\@/) {
last;
}
$current_file .= $_;
}
init_hunk $_;
# check hunks
while (<>) {
if ($before_log) {
if (!/\+.*\$Log.*\$/) {
add_line $before_log;
} else {
skip_line $before_log;
}
$before_log = '';
}
if ($after_log) {
if (!/-.*\$Log.*\$/) {
add_line $after_log;
} else {
skip_line $after_log;
}
$after_log = '';
}
if ($skip_logs) {
if (/^\+/) {
skip_line $_;
$skip_logs = 1;
} else {
$skip_logs = 0;
if (/^ *\*$/) {
$after_log = $_;
}
}
} elsif (/^\+.*\$($simple_keyword).*\$/) {
skip_line $_;
} elsif (/^\@\@.*/) {
flush_hunk;
init_hunk $_;
} elsif (/^diff/) {
flush_hunk;
return;
} elsif (/^-.*\$($simple_keyword).*\$/) {
# fake new hunk
skip_line $_;
} elsif (/^\+ *\*$/) {
$before_log = $_;
} elsif (/^([- \+]).*\$Log.*\$/) {
skip_line $_;
$skip_logs = 1;
} else {
add_line $_;
}
}
}
# This loop just shows everything before the first diff, and then hands
# over to check_file whenever it sees a line beginning with "diff".
while (<>) {
if (/^diff/) {
do {
check_file $_;
} while(/^diff/);
flush_hunk;
} else {
printf $_;
}
}
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
Thanks for working on this. I'd like to see it added to git toolkit.
This diff is way easier to read now. I can see that it has SPI support
backported from some future version.
But... it still has some problems.
For the phytec patch it's not getting the $Log changes in the qlogic
files right.
I'm checking the output diff line by line for problems. It's down to
11,528 lines from 88,787. That's a lot of junk removed, I'll have to
make sure it isn't removing too much.
--
Jon Smirl
jonsmirl@gmail.com
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
On 7/23/07, Jon Smirl [off-list ref] wrote:
Thanks for working on this. I'd like to see it added to git toolkit.
This diff is way easier to read now. I can see that it has SPI support
backported from some future version.
But... it still has some problems.
For the phytec patch it's not getting the $Log changes in the qlogic
files right.
I'm checking the output diff line by line for problems. It's down to
11,528 lines from 88,787. That's a lot of junk removed, I'll have to
make sure it isn't removing too much.
I forgot about newly added files, looks like only 20,000 lines of CVS noise.
The phytec patch has several ia64 files added to it, those are obvious
not part of the support and an ARM9 processor. Never noticed those
before.
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:23
Hi,
On Mon, 23 Jul 2007, Jon Smirl wrote:
Thanks for working on this. I'd like to see it added to git toolkit.
I plan to submit it to patchutils instead, since this is not really
dependent on git.
This diff is way easier to read now. I can see that it has SPI support
backported from some future version.
But... it still has some problems. For the phytec patch it's not getting
the $Log changes in the qlogic files right.
You are correct. This slipped by my eyes.
I'm checking the output diff line by line for problems. It's down to
11,528 lines from 88,787. That's a lot of junk removed, I'll have to
make sure it isn't removing too much.
May I ask you to test this version?
Thanks,
Dscho
-- snipsnap --
#!/usr/bin/perl
# This is a simple state machine.
#
# There is the state of the current file; its header is stored
# in $current_file to avoid outputting it when all hunks were
# culled. It is only printed before the first hunk, and then
# set to "" to avoid outputting it twice.
#
# There are the states of the current hunk, stored in
# * $current_hunk (possibly modified hunk)
# * $start_minus, $start_plus (from the original)
# * $plus, $minus, $space (the current count of the respective lines)
# a hunk is only printed (in flush_hunk) if any '+' or '-' lines
# are left after filtering.
#
# For $Log..$, there is the state $skip_logs, which is set to 1
# after seeing such a line, and set to 0 when the first line
# was seen which does not begin with '+'.
#
# A particularly nasty special case is when a single "*" was
# misattributed by the diff to be _inserted before_ a $Log, instead
# of _appended after_ a $Log.
# This is the purpose of the $before_log and $after_log variables:
# if not empty, the state machine expects the next line to begin
# with '+' or '-', respectively, and followed by a $Log. If this
# expectation is not met, the variable is output.
#
# The variable $plus_minus_adjust contains the number of lines which
# were skipped from the "+" side, so that the correct offset is shown.
# This function gets a hunk header.
#
# It initializes the state variables described above
sub init_hunk {
my $line = $_[0];
$current_hunk = "";
($start_minus, $dummy, $start_plus, $dummy) =
($line =~ /^\@\@ -(\d+)(,\d+|) \+(\d+)(,\d+|) \@\@/);
$plus = $minus = $space = 0;
$skip_logs = 0;
$before_log = '';
$after_log = '';
# we prefer /dev/null as original file name when a file is new
if ($start_minus eq 0) {
$current_file =~ s/\n--- .*\n/\n--- \/dev\/null\n/;
} elsif ($start_plus eq 0) {
$current_file =~ s/\n\+\+\+ .*\n/\n+++ \/dev\/null\n/;
}
}
# This function is called whenever there is possibly a hunk to print.
# Nothing is printed if no '+' or '-' lines are left.
# Otherwise, if the file header was not yet shown, it does so now.
sub flush_hunk {
if (($plus > 0 || $minus > 0) && $current_hunk ne '') {
if ($current_file ne "") {
print $current_file;
$current_file = "";
}
$minus += $space;
$plus += $space;
print "\@\@ -$start_minus,$minus "
. "+" . ($start_plus - $start_plus_adjust)
. ",$plus \@\@\n";
print $current_hunk;
$current_hunk = '';
}
}
# This adds a line to the current hunk and updates $space, $plus or $minus
sub add_line {
my $line = $_[0];
$current_hunk .= $line;
if ($line =~ /^ /) {
$space++;
} elsif ($line =~ /^\+/) {
$plus++;
} elsif ($line =~ /^-/) {
$minus++;
} elsif ($line =~ /^\\/) {
# do nothing
} else {
die "Unexpected line: $line";
}
}
# This function splits the current hunk into the part before the current
# line, and the part after the current line.
sub skip_line {
my $line = $_[0];
if ($start_minus == 0) {
# This patch adds a new file, just ignore that line
return;
} elsif ($start_plus == 0) {
# This patch removes a file, so include the line nevertheless
add_line $_;
return;
}
flush_hunk;
if ($line =~ /^-/) {
$minus++;
} elsif ($line =~ /^\+/) {
$plus++;
$start_plus_adjust++;
}
init_hunk "@@ -" . ($start_minus + $minus + $space)
. " +" . ($start_plus + $plus + $space)
. " @@\n";
}
$simple_keyword = "Id|Revision|Author|Date|Source|Header";
# This is the main loop
sub check_file {
$_ = $_[0];
$current_file = $_;
$start_plus_adjust = 0;
while (<>) {
if (/^\@\@/) {
last;
}
$current_file .= $_;
}
init_hunk $_;
# check hunks
while (<>) {
if ($before_log) {
if (!/\+.*\$Log.*\$/) {
add_line $before_log;
} else {
skip_line $before_log;
}
$before_log = '';
}
if ($after_log) {
if (!/-.*\$Log.*\$/) {
add_line $after_log;
} else {
skip_line $after_log;
}
$after_log = '';
}
if ($skip_logs) {
if (/^\+/) {
skip_line $_;
$skip_logs = 1;
} else {
$skip_logs = 0;
if (/^ *\*$/) {
$after_log = $_;
}
}
} elsif (/^\+.*\$($simple_keyword).*\$/) {
skip_line $_;
} elsif (/^\@\@.*/) {
flush_hunk;
init_hunk $_;
} elsif (/^diff/) {
flush_hunk;
return;
} elsif (/^-.*\$($simple_keyword).*\$/) {
# fake new hunk
skip_line $_;
} elsif (/^\+ *\*$/) {
$before_log = $_;
} elsif (/^([- \+]).*\$Log.*\$/) {
skip_line $_;
$skip_logs = 1;
} else {
add_line $_;
}
}
}
# This loop just shows everything before the first diff, and then hands
# over to check_file whenever it sees a line beginning with "diff".
while (<>) {
if (/^diff/) {
do {
check_file $_;
} while(/^diff/);
} else {
printf $_;
}
}
flush_hunk;
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:23
Hi,
On Tue, 24 Jul 2007, Jakub Narebski wrote:
[Cc: Johannes Schindelin [off-list ref], git@vger.kernel.org]
Thanks.
Johannes Schindelin wrote:
quoted
On Mon, 23 Jul 2007, Jon Smirl wrote:
quoted
Thanks for working on this. I'd like to see it added to git toolkit.
I plan to submit it to patchutils instead, since this is not really
dependent on git.
Could you also add it to contrib/ area, please?
Sure. Once it is really all fleshed out. But I really think that
patchutils is a better place. That way, the script also helps the poor
souls stuck with Mercurial or Darcs :-)
The really tedious part is the testing, and the verifying. Fortunately,
Jon made up for my incapability in both areas, with an incredible
patience.
Ciao,
Dscho
From: Jon Smirl <hidden> Date: 2016-06-15 22:43:23
On 7/24/07, Johannes Schindelin [off-list ref] wrote:
The really tedious part is the testing, and the verifying. Fortunately,
Jon made up for my incapability in both areas, with an incredible
patience.
I haven't finished checking every last line yet, if anything is left
it is small. We lost power here all evening last night. That's not
supposed to happen in urban Boston. It was black over a mile from me
in all directions.