Bug 22611 - Clean up run-javascriptcore-tests in preparation for adding --chromium support
Summary: Clean up run-javascriptcore-tests in preparation for adding --chromium support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-02 16:51 PST by Eric Seidel (no email)
Modified: 2008-12-03 13:36 PST (History)
1 user (show)

See Also:


Attachments
Cleanups to our scripts in preparation for adding more --chromium support (11.94 KB, patch)
2008-12-02 16:56 PST, Eric Seidel (no email)
hyatt: review+
Details | Formatted Diff | Diff

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