Bug 22611 – Clean up run-javascriptcore-tests in preparation for adding --chromium support

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

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

[Eric Seidel (no email)]

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

[Mark Rowe (bdash)]

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?

[Eric Seidel (no email)]

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

[Eric Seidel (no email)]

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.

[Dave Hyatt]

Comment on attachment 25691 [details]
Cleanups to our scripts in preparation for adding more --chromium support

r=me

[Eric Seidel (no email)]

http://trac.webkit.org/changeset/38960