Bug 38454

Summary:

svn-apply: Have parseDiffHeader() call parseGitDiffHeader().

Product:

WebKit

Reporter:

Chris Jerdonek <cjerdonek>

Component:

Tools / Tests

Assignee:

Chris Jerdonek <cjerdonek>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cjerdonek, dbates, eric, webkit.review.bot, zoltan

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

38425

Bug Blocks:

Attachments:

Description	Flags
Proposed patch	none
Proposed patch 2	none

Chris Jerdonek

Reported 2010-05-03 04:02:24 PDT

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.

Attachments
Proposed patch (31.95 KB, patch) 2010-05-03 13:57 PDT, Chris Jerdonek	no flags	Details Formatted Diff Diff
Proposed patch 2 (31.94 KB, patch) 2010-05-03 14:07 PDT, Chris Jerdonek	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Chris Jerdonek

Comment 1 2010-05-03 13:57:59 PDT

Created attachment 54958 [details] Proposed patch

WebKit Review Bot

Comment 2 2010-05-03 14:03:57 PDT

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

Comment 3 2010-05-03 14:07:09 PDT

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

Comment 4 2010-05-03 14:09:59 PDT

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 5 2010-05-03 23:56:54 PDT

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

Comment 6 2010-05-04 00:37:06 PDT

(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 7 2010-05-04 00:56:43 PDT

Comment on attachment 54959 [details] Proposed patch 2 Clearing flags on attachment: 54959 Committed r58739: <http://trac.webkit.org/changeset/58739>

Chris Jerdonek

Comment 8 2010-05-04 00:56:58 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.