Summary: | Add "--cairo" flag to 'build-webkit' command | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | ||||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Brent Fulgham
2008-03-19 11:41:07 PDT
Created attachment 19887 [details]
Extend build-webkit with new "--cairo" build option
* Add "--cairo" to help screen for build-webkit script.
* Add "isCairo" test and when present for the Visual Studio build switch to the Cairo debug/release targets.
I'm confused as to why this is necessary? (Why do the executables need a _cairo suffix?) (In reply to comment #2) > I'm confused as to why this is necessary? (Why do the executables need a > _cairo suffix?) The executables don't need a _cairo suffix. However, the naming of the Cairo-based build targets in Visual Studio are constructed with "_cairo" suffixes. This patch causes the build script to build using the "Release_cairo" and "Debug_cairo" build targets. Any executable output names are generated outside the scope of this patch. Comment on attachment 19887 [details]
Extend build-webkit with new "--cairo" build option
+ if (checkArgv("--cairo")) {
+ $isCairo = 1;
+ } else {
+ $isWx = 0;
+ }
Seems wrong to set $isWx here.
But also, I think you'd want this setting to be a persistent one. It seems really inconvenient to have to pass --cairo every single time. It should work more like configuration where you can set it once and then it's persistent until you reset it.
review- because of the $isWx mistake, but please consider my suggestion.
Created attachment 20445 [details]
Support cairo via command-line flag
Script modified per Darin's comments:
* --cairo flag handled more like the --debug/--release flags, such that they persist in the Configuration file if the "set-webkit-configuration" script is used.
* No longer incorrectly set the "isWx" value.
Comment on attachment 20445 [details]
Support cairo via command-line flag
+ my $isCairo = grep(/^--cairo$/i, @ARGV);
+
for my $i (0 .. $#ARGV) {
my $opt = $ARGV[$i];
if ($opt =~ /^--debug$/i || $opt =~ /^--devel/i) {
splice(@ARGV, $i, 1);
$passedConfiguration = "Debug";
+ $passedConfiguration .= "_Cairo" if $isCairo;
return;
}
if ($opt =~ /^--release$/i || $opt =~ /^--deploy/i) {
splice(@ARGV, $i, 1);
$passedConfiguration = "Release";
+ $passedConfiguration .= "_Cairo" if $isCairo;
return;
}
This seems very Windows-specific, since only the Windows .vcproj files have these _Cairo configurations. Perhaps the flag should only have an effect on Windows? Perhaps it should be named something more specific, like --cairo-win32?
Created attachment 21650 [details]
Extend build-webkit with new "--cairo-win32" build option
Updated based on Adam's comments:
* Changed --cairo flag to --cairo-win32
* --cairo-win32 is only honored if "isCygwin()" is true
Created attachment 21651 [details]
Minor update to merge against TOT
Same as above, with minor update to build against TOT.
Comment on attachment 21651 [details]
Minor update to merge against TOT
r=me
Committed revision 34706. |