Bug 22611

Summary: Clean up run-javascriptcore-tests in preparation for adding --chromium support
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Cleanups to our scripts in preparation for adding more --chromium support hyatt: review+

Description Eric Seidel (no email) 2008-12-02 16:51:41 PST
Clean up run-javascriptcore-tests in preparation for adding --chromium support

Sigh.  Many of our scripts are such a mess.  Instead of just adding --chromium to the argument whitelist, i chose to actually clean up how jsDriver arguments were passed into the script so that we now transparently support any new build-jsc or webkitdirs.pm options.   (It seems we never use this functionality anyway, at least grepping svn I didn't find anything.)
Comment 1 Eric Seidel (no email) 2008-12-02 16:56:32 PST
Created attachment 25691 [details]
Cleanups to our scripts in preparation for adding more --chromium support

 WebKitTools/ChangeLog                        |   26 ++++++++
 WebKitTools/Scripts/build-jsc                |   24 +++++++-
 WebKitTools/Scripts/build-webkit             |   45 +++++----------
 WebKitTools/Scripts/run-javascriptcore-tests |   80 ++++++++++++--------------
 WebKitTools/Scripts/webkitdirs.pm            |   23 +++++++
 5 files changed, 123 insertions(+), 75 deletions(-)
Comment 2 Mark Rowe (bdash) 2008-12-02 17:09:30 PST
Comment on attachment 25691 [details]
Cleanups to our scripts in preparation for adding more --chromium support

>  chdir "JavaScriptCore" or die "Can't find JavaScriptCore directory to build from";
>  my $result;
>  if (isAppleMacWebKit()) {
> -    $result = system "sh", "-c", 'xcodebuild -project JavaScriptCore.xcodeproj -target jsc "$@" | grep -v setenv && exit ${PIPESTATUS[0]}', "xcodebuild",  @options, @ARGV;
> +    $result = system "sh", "-c", 'xcodebuild -project JavaScriptCore.xcodeproj -target jsc "$@" | grep -v setenv && exit ${PIPESTATUS[0]}', "xcodebuild",  @options, @ARGV, @coverageSupportOptions;
>  } elsif (isAppleWinWebKit()) {

Why is this not using buildXCodeProject?
Comment 3 Eric Seidel (no email) 2008-12-02 17:24:04 PST
(In reply to comment #2)
> (From update of attachment 25691 [details] [review])
> >  chdir "JavaScriptCore" or die "Can't find JavaScriptCore directory to build from";
> >  my $result;
> >  if (isAppleMacWebKit()) {
> > -    $result = system "sh", "-c", 'xcodebuild -project JavaScriptCore.xcodeproj -target jsc "$@" | grep -v setenv && exit ${PIPESTATUS[0]}', "xcodebuild",  @options, @ARGV;
> > +    $result = system "sh", "-c", 'xcodebuild -project JavaScriptCore.xcodeproj -target jsc "$@" | grep -v setenv && exit ${PIPESTATUS[0]}', "xcodebuild",  @options, @ARGV, @coverageSupportOptions;
> >  } elsif (isAppleWinWebKit()) {
> 
> Why is this not using buildXCodeProject?
> 

It could, certainly.  It is grepping out setenv.  That's probably what we wnt to do for all calls to xcodebuild, but I didn't want to make the decision at this moment.  I figure I'll come back in a later patch and abstract all calls to xcodebuild to use buildXCodeProject() instead.  buildXCodeProject also should be made more robust to handle things like --64-bit, --universal, etc. for all projects, instead of build-webkit having one-offs for all of these standard build options.

We also should eventually probably break all of the building logic out of webkitdirs.pm and into a separate WebKitBuildSupport.pm or something.  Right now prepare-ChangeLog pulls in all of the build support stuff which is silly. :)
Comment 4 Eric Seidel (no email) 2008-12-02 17:27:10 PST
My grand vision would be for there one day for there to just be a buildProject(directory, target, clean, argv) call which just knew how to handle all of our different build systems.  But that's a while off.
Comment 5 Dave Hyatt 2008-12-03 12:50:46 PST
Comment on attachment 25691 [details]
Cleanups to our scripts in preparation for adding more --chromium support

r=me
Comment 6 Eric Seidel (no email) 2008-12-03 13:36:02 PST
http://trac.webkit.org/changeset/38960