Bug 43981 – svn-apply doesn't detect empty line with Windows line endings after property value

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

[Patrick R. Gansterer]

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

[Daniel Bates]

*** Bug 42868 has been marked as a duplicate of this bug. ***

[Daniel Bates]

Created attachment 64455 [details]
Patch 1 of 2: Detect empty line with Windows line ending

[Daniel Bates]

Created attachment 64456 [details]
Patch 2 of 2: Unit tests

[Daniel Bates]

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

[WebKit Review Bot]

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.

[Daniel Bates]

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.

[Daniel Bates]

(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.

[Eric Seidel (no email)]

Comment on attachment 64456 [details]
Patch 2 of 2: Unit tests

Thanks again Dan!

[Daniel Bates]

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.

[Daniel Bates]

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.

[WebKit Commit Bot]

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>

[Eric Seidel (no email)]

Can't stop the bot once started :(  It's a bad bug.

[Daniel Bates]

Committed unit tests in changeset 65734 <http://trac.webkit.org/changeset/65734>.

[Andras Becsi]

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.