Bug 14126

Summary: Shell scripts to build WebKit should be forced to LF endings
Product: WebKit Reporter: Peter Kasting <pkasting>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: ap, ddkilzer, mrowe
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
Trim EOL code
ddkilzer: review-
Patch v2
ddkilzer: review-
Patch v3 aroben: review+

Description Peter Kasting 2007-06-13 14:29:27 PDT
Checking out WebKit on Windows can lead to problems with line endings that cause the build to break, because the shell scripts used in the build process are checkout out with native line endings rather than LF.  aroben noted (on IRC) that while people who use the SVN provided with Cygwin should not suffer from this problem, the line endings should probably be fixed anyway.

Fixing this is as simple as the following:
$ svn propset svn:eol-style LF <files>
...Wherer <files> is the following set:
JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/build-generated-files.sh
JavaScriptCore/make-generated-sources.sh
WebCore/css/makeprop
WebCore/css/maketokenizer
WebCore/css/makevalues
WebCore/make-generated-sources.sh
WebCore/move-js-headers.sh
WebCore/WebCore.vcproj/build-generated-files.sh
WebKit/win/WebKit.vcproj/auto-version.sh
WebKitTools/CygwinDownloader/make-zip.sh

(The files in WebCore/css/ have no shell specifier at the top of the file, but need to be modified as well, as far as I can tell.  I don't know if there are even more files in the build process that need this tweak; I haven't gotten my build to complete successfully yet.)

I tried to write a patch to do this, but got errors from svn-create-patch :(.  So someone with commit access will have to handle this one.
Comment 1 Peter Kasting 2007-06-13 15:56:24 PDT
At least these files seem to also need changing to LF endings as well:

WebKit\WebCore\css\CSSPropertyNames.in
WebKit\WebCore\css\CSSValueKeywords.in
WebKit\WebCore\ksvg2\css\CSSPropertyNames.in
WebKit\WebCore\ksvg2\css\CSSValueKeywords.in
Comment 2 David Kilzer (:ddkilzer) 2007-06-13 17:30:52 PDT
(In reply to comment #0)
> I tried to write a patch to do this, but got errors from svn-create-patch :(. 
> So someone with commit access will have to handle this one.

Please file another Bugzilla bug for this issue, with steps to reproduce and any error output you saw.  It should be put in the "Tools / Tests" component.  Thanks!

Comment 3 Satoshi Ueyama 2007-09-27 23:06:48 PDT
Created attachment 16428 [details]
Trim EOL code

I'm using cygwin with CR-LF endings.
Build failed at first due to that EOL code issue. So I made changes show in the patch.
After this change, build succeeded and produced code works well.

This patch does nothing when run with LF endings so won't balk build process on Mac.
Comment 4 David Kilzer (:ddkilzer) 2007-09-28 06:16:57 PDT
Comment on attachment 16428 [details]
Trim EOL code

Needs a ChangeLog:

http://webkit.org/coding/contributing.html

Also, please set the "review?" flag to make sure this gets reviewed.

Thanks for the patch!
Comment 5 Satoshi Ueyama 2007-09-28 09:14:54 PDT
Created attachment 16432 [details]
Patch v2

Sorry..
Here's a new patch including ChangeLog.
Comment 6 David Kilzer (:ddkilzer) 2007-09-29 14:46:29 PDT
Comment on attachment 16432 [details]
Patch v2

>@@ -68,6 +68,8 @@ while (<IN>) {
>     chomp;
>     my $prop = $_;
> 
>+	$prop =~ s/[\r\n]+$//;

The chomp statement isn't necessary with the added line.  (Sorry, I should have caught this with the first review.)

Please remove the chomp statement, repost the patch, and r=me.
Comment 7 Satoshi Ueyama 2007-09-29 20:37:42 PDT
Created attachment 16463 [details]
Patch v3

Thanks, here's a new patch.
Removed chomp statement and replaced with that regexp.
Comment 8 Adam Roben (:aroben) 2007-09-29 21:00:58 PDT
Comment on attachment 16463 [details]
Patch v3

r=me
Comment 9 Mark Rowe (bdash) 2007-10-14 04:31:03 PDT
The script being patched no longer exists on trunk, so this patch is not needed.