Bug 43981 - svn-apply doesn't detect empty line with Windows line endings after property value
Summary: svn-apply doesn't detect empty line with Windows line endings after property ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
: 42868 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-08-13 11:02 PDT by Patrick R. Gansterer
Modified: 2010-08-26 09:58 PDT (History)
9 users (show)

See Also:


Attachments
Patch 1 of 2: Detect empty line with Windows line ending (4.69 KB, patch)
2010-08-15 14:27 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch 2 of 2: Unit tests (20.13 KB, patch)
2010-08-15 14:28 PDT, Daniel Bates
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2010-08-13 11:02:34 PDT
In the attachment 63151 [details] from bug 39164 are changes to WebCore/CMakeLists.txt.
In the commit at http://trac.webkit.org/changeset/65319 this changes are missing.

maybe related to bug 42868
Comment 1 Daniel Bates 2010-08-15 14:26:25 PDT
*** Bug 42868 has been marked as a duplicate of this bug. ***
Comment 2 Daniel Bates 2010-08-15 14:27:40 PDT
Created attachment 64455 [details]
Patch 1 of 2: Detect empty line with Windows line ending
Comment 3 Daniel Bates 2010-08-15 14:28:09 PDT
Created attachment 64456 [details]
Patch 2 of 2: Unit tests
Comment 4 Daniel Bates 2010-08-15 14:29:07 PDT
I thought to break up the patch into two patches:

"Patch 1 of 2: Detect empty line with Windows line ending" - the code change and change log
"Patch 2 of 2: Unit tests" - the unit tests
Comment 5 WebKit Review Bot 2010-08-15 14:31:58 PDT
Attachment 64456 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:394:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:395:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:721:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:722:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:809:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:810:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:834:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:835:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:911:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:912:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:926:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:927:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:939:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:940:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 14 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Daniel Bates 2010-08-15 14:56:02 PDT
Notice, both attachment 63151 [details] <https://bugs.webkit.org/attachment.cgi?id=63151> and attachment 62341 [details] <https://bugs.webkit.org/attachment.cgi?id=62341> (from bug #42071) have Windows line endings and property change diffs.

Without loss of generality, we define the property value(*) as being all the text up to the start of the first empty line. Currently we use the regular expression /^$/ to describe the empty line, which only works for lines that end in LF (Unix line endings). Instead, we should use /^[\r\n]+$/ so that the empty line is detected even if it ends in CRLF (Windows line endings).

(*) The anatomy of a property change diff:

1. Property changes on: WebCore\platform\win\BitmapInfo.cpp
2. ___________________________________________________________________
3. Added: svn:eol-style
4.    + native

Line 1 describes the file to apply/modify/remove properties from, called the property path.
Line 3 describes the property type and name, "Added", and "svn:eol-style", respectively.
Line 4 describes whether the property is to be added or removed ("+" or "-", respectively) and the value of the property. This value can span multiple lines.
Comment 7 Daniel Bates 2010-08-15 14:57:26 PDT
(In reply to comment #5)
> Attachment 64456 [details] did not pass style-queue:
> 
> Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
> WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:394:  Line contains tab character.  [whitespace/tab] [5]
> [...]
> WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:939:  Line contains tab character.  [whitespace/tab] [5]
> WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl:940:  Line contains tab character.  [whitespace/tab] [5]
> Total errors found: 14 in 3 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

These errors are expected as parseDiff.pl contains sample diffs. These diffs contain tab characters.
Comment 8 Eric Seidel (no email) 2010-08-20 07:01:20 PDT
Comment on attachment 64456 [details]
Patch 2 of 2: Unit tests

Thanks again Dan!
Comment 9 Daniel Bates 2010-08-20 07:44:58 PDT
Comment on attachment 64455 [details]
Patch 1 of 2: Detect empty line with Windows line ending

Clearing commit-queue flag. Will land by hand so that I can combine the two patches.
Comment 10 Daniel Bates 2010-08-20 07:45:12 PDT
Comment on attachment 64456 [details]
Patch 2 of 2: Unit tests

Clearing commit-queue flag. Will land by hand so that I can combine the two patches.
Comment 11 WebKit Commit Bot 2010-08-20 07:45:27 PDT
Comment on attachment 64455 [details]
Patch 1 of 2: Detect empty line with Windows line ending

Clearing flags on attachment: 64455

Committed r65732: <http://trac.webkit.org/changeset/65732>
Comment 12 Eric Seidel (no email) 2010-08-20 07:53:27 PDT
Can't stop the bot once started :(  It's a bad bug.
Comment 13 Daniel Bates 2010-08-20 08:08:59 PDT
Committed unit tests in changeset 65734 <http://trac.webkit.org/changeset/65734>.
Comment 14 Andras Becsi 2010-08-26 09:58:11 PDT
svn-apply in cygwin still seems to have problems with patches which also touch vcproj files.
During working on this bug: https://bugs.webkit.org/show_bug.cgi?id=29244 I tried to create a patch in cygwin and after cleanup apply the created patch using svn-apply and it couldn't apply.
I'm unsure where the problem exactly lies, but it seems the issue is related to the same problem this bug tried to fix.
Daniel, could you please take a look at the problem?
Thanks in advance.