Bug 51509 – [Qt] [Symbian] Fix build-webkit script for Symbian

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Laszlo Gombos]

Symbian port of QtWebKit does not support out of source builds - as it has been also established by r53266. On Symbian OUTPUT_DIR should be set to the source directory as Release and Debug configuration can not be built side-by-side. In addition OUTPUT_DIR is really only used for generated files for Symbian builds, which are the same for both Release and Debug builds, so there is no reason to use the actual configuration in the OUTPUT_DIR.

[Laszlo Gombos]

Created attachment 77281 [details]
proposed patch

[Eric Seidel (no email)]

Comment on attachment 77281 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77281&action=review

Review tool can't seem to apply the patch, so I doubt this will land w/o an update.

> Tools/Scripts/webkitdirs.pm:303
> +        # Symbian builds do not have Release/Debug configurations either.
> +        if (($ENV{"WEBKITOUTPUTDIR"} && (isGtk() || isEfl())) || isSymbian()) {

We need to make this into a function.  usesPerConfigurationBuildDirectory() or similar.

> Tools/Scripts/webkitdirs.pm:1526
> -    my $dir = File::Spec->canonpath(baseProductDir());
> -    $dir = File::Spec->catfile($dir, $config) unless isSymbian();
> +    my $dir = File::Spec->canonpath(productDir());

Is this a symbian specific code path? Otherwise why is this OK for other ports?

> Tools/Scripts/webkitdirs.pm:1532
> -    push @buildArgs, "OUTPUT_DIR=" . baseProductDir() . "/$config";
> +    push @buildArgs, "OUTPUT_DIR=" . $dir;

Why is this OK for other ports?  Or is this just reverting previously added symbian-specific code?

[Laszlo Gombos]

Thanks for the review Eric!
.
(In reply to comment #2)
> (From update of attachment 77281 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77281&action=review
> 
> Review tool can't seem to apply the patch, so I doubt this will land w/o an update.

EWS seems to be green.

> 
> > Tools/Scripts/webkitdirs.pm:303
> > +        # Symbian builds do not have Release/Debug configurations either.
> > +        if (($ENV{"WEBKITOUTPUTDIR"} && (isGtk() || isEfl())) || isSymbian()) {
> 
> We need to make this into a function.  usesPerConfigurationBuildDirectory() or similar.

Will do.

> 
> > Tools/Scripts/webkitdirs.pm:1526
> > -    my $dir = File::Spec->canonpath(baseProductDir());
> > -    $dir = File::Spec->catfile($dir, $config) unless isSymbian();
> > +    my $dir = File::Spec->canonpath(productDir());
> 
> Is this a symbian specific code path? Otherwise why is this OK for other ports?

This is a Qt specific code path (Symbian being one of the QtWebKit ports). For QtWebKit productDir() returns $baseProductDir/$configuration for all Qt ports except for Symbian (see earlier part of the patch). 

> 
> > Tools/Scripts/webkitdirs.pm:1532
> > -    push @buildArgs, "OUTPUT_DIR=" . baseProductDir() . "/$config";
> > +    push @buildArgs, "OUTPUT_DIR=" . $dir;
> 
> Why is this OK for other ports?  Or is this just reverting previously added symbian-specific code?

Same as above.

[Laszlo Gombos]

Created attachment 77336 [details]
created a separate function

[David Kilzer (:ddkilzer)]

Comment on attachment 77336 [details]
created a separate function

r=me

[WebKit Commit Bot]

Comment on attachment 77336 [details]
created a separate function

Clearing flags on attachment: 77336

Committed r74811: <http://trac.webkit.org/changeset/74811>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.