[Webkit-unassigned] [Bug 27619] Switch wx to using waf for builds

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27619


Mark Rowe (bdash) <mrowe at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34326|review?                     |review-
               Flag|                            |




--- Comment #29 from Mark Rowe (bdash) <mrowe at apple.com>  2009-08-08 17:53:31 PDT ---
(From update of attachment 34326)
> +    # we don't support SVG yet.
> +    ($svgSupport, $svgAnimationSupport, $svgAsImageSupport, $svgDOMObjCBindingsSupport) = (0, 0, 0, 0);
> +    ($svgFontsSupport, $svgForeignObjectSupport, $svgUseSupport) = (0, 0, 0);
> +}

You should take a look at how the Qt project modifies the default values of
these build options.  Overriding them in this manner will result in their
default values in 'build-webkit --help' being inconsistent with the values that
are actually used.

>  # Don't report the "WebKit is now built" message after a clean operation.
> Index: WebKitTools/Scripts/run-launcher
> ===================================================================
> --- WebKitTools/Scripts/run-launcher	(revision 46910)
> +++ WebKitTools/Scripts/run-launcher	(working copy)
> @@ -42,7 +42,9 @@
>  my @args = @ARGV;
>  
>  # Check to see that all the frameworks are built.
> -checkFrameworks();
> +if (!isWx()) {
> +    checkFrameworks();
> +}

We should fix the implementation of checkFrameworks rather than if'ing the call
site.

> +    # get / update waf if needed
> +    my $waf = "$sourceDir/WebKitTools/wx/waf";
> +    my $wafURL = 'http://wxwebkit.wxcommunity.com/downloads/deps/waf';
> +    if (!-f $waf) {
> +        my $result = system "curl -o $waf $wafURL";
> +        chmod 0755, $waf;
> +    }

Is it really necessary to attempt to check if this tool needs updated every
time a project is built?  This seems like something we'd typically have
update-webkit do.  Either way it should have some more error handling and fewer
redundant variables.

> +    print "Building $project\n";
> +
> +    my $wafCommand = $waf;
> +    if (isCygwin()) {
> +        $wafCommand = `cygpath --windows "$wafCommand"`;
> +        chomp($wafCommand);
> +    }
> +    if ($shouldClean) {
> +        return system $wafCommand, "clean";
> +    }
> +    
> +    my $result = system $wafCommand, 'configure', 'build', 'install', @options;
> +    
> +    return $result;

The temporary $result variable here isn't needed.

There are no major issues, but it does need revision.  Marking as r-.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.