Bug 51509

Summary: [Qt] [Symbian] Fix build-webkit script for Symbian
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates, gyuyoung.kim, mrobinson
Priority: P3 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: S60 3rd edition   
Attachments:
Description Flags
proposed patch
eric: review-
created a separate function none

Description Laszlo Gombos 2010-12-22 17:02:47 PST
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.
Comment 1 Laszlo Gombos 2010-12-22 17:08:14 PST
Created attachment 77281 [details]
proposed patch
Comment 2 Eric Seidel (no email) 2010-12-23 01:03:51 PST
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?
Comment 3 Laszlo Gombos 2010-12-23 08:28:50 PST
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.
Comment 4 Laszlo Gombos 2010-12-23 08:30:31 PST
Created attachment 77336 [details]
created a separate function
Comment 5 David Kilzer (:ddkilzer) 2010-12-30 14:11:09 PST
Comment on attachment 77336 [details]
created a separate function

r=me
Comment 6 WebKit Commit Bot 2010-12-30 15:30:45 PST
Comment on attachment 77336 [details]
created a separate function

Clearing flags on attachment: 77336

Committed r74811: <http://trac.webkit.org/changeset/74811>
Comment 7 WebKit Commit Bot 2010-12-30 15:30:52 PST
All reviewed patches have been landed.  Closing bug.