Bug 14126 - Shell scripts to build WebKit should be forced to LF endings
Summary: Shell scripts to build WebKit should be forced to LF endings
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-13 14:29 PDT by Peter Kasting
Modified: 2007-10-14 04:31 PDT (History)
3 users (show)

See Also:


Attachments
Trim EOL code (431 bytes, patch)
2007-09-27 23:06 PDT, Satoshi Ueyama
ddkilzer: review-
Details | Formatted Diff | Diff
Patch v2 (929 bytes, patch)
2007-09-28 09:14 PDT, Satoshi Ueyama
ddkilzer: review-
Details | Formatted Diff | Diff
Patch v3 (930 bytes, patch)
2007-09-29 20:37 PDT, Satoshi Ueyama
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.