Bug 38454 – svn-apply: Have parseDiffHeader() call parseGitDiffHeader().

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Chris Jerdonek]

In this patch, I think it would make sense to recast the existing parseDiffHeader() as parseSvnDiffHeader() and then create a new parseDiffHeader() that calls the Git or SVN version based on the first line (whether it begins with "Index:" or "diff --git").  The unit tests for the new parseDiffHeader() can have enough tests just to ensure that it supports carrying over each diffHash key value correctly (since the more thorough tests will be in the tests for the Git and SVN versions).

This patch can also delete the gitdiff2svndiff() subroutine.

[Chris Jerdonek]

Created attachment 54958 [details]
Proposed patch

[WebKit Review Bot]

Attachment 54958 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:47:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:48:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:57:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:58:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:73:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:74:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:83:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:84:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:99:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:100:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:109:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:110:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:125:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:126:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:135:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:136:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:151:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:152:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:161:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:162:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 20 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Jerdonek]

Created attachment 54959 [details]
Proposed patch 2

A minor change to the ChangeLog and a code comment.

By the way, expect style errors for tabs in this patch.  This is because diffs contain tab characters, and the unit tests contain sample diffs.

Also marking cq- since this patch will require setting the svn:allow-tabs property.

[WebKit Review Bot]

Attachment 54959 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:47:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:48:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:57:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:58:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:73:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:74:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:83:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:84:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:99:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:100:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:109:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:110:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:125:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:126:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:135:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:136:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:151:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:152:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:161:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:162:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 20 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Daniel Bates]

Comment on attachment 54959 [details]
Proposed patch 2

> +#     * Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> +# its contributors may be used to endorse or promote products derived
> +# from this software without specific prior written permission.

Just wanted to point out you reference Apple in the copyright here.

r=me.

[Chris Jerdonek]

(In reply to comment #5)
> (From update of attachment 54959 [details])
> > +#     * Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> > +# its contributors may be used to endorse or promote products derived
> > +# from this software without specific prior written permission.
> 
> Just wanted to point out you reference Apple in the copyright here.
> 
> r=me.

Thanks, Dan.

I preserved the original copyright and license text since the new file is mostly a copy of the original, though perhaps I can change that text since I'm the sole copyright owner of the file. :)

[Chris Jerdonek]

Comment on attachment 54959 [details]
Proposed patch 2

Clearing flags on attachment: 54959

Committed r58739: <http://trac.webkit.org/changeset/58739>

[Chris Jerdonek]

All reviewed patches have been landed.  Closing bug.