Summary: | [GTK] Feature enabling/disabling should be possible through build-webkit | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
Component: | WebKitGTK | Assignee: | Zan Dobersek <zan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, dbates, dpranke, gustavo, levin, mrobinson, ojan, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 87126 | ||||||||
Attachments: |
|
Description
Zan Dobersek
2012-10-14 13:11:58 PDT
Created attachment 168933 [details]
WIP patch
A work-in-progress patch, using a very hack-ish approach.
Created attachment 169387 [details]
Patch
Hi Martin, kov, I'd appreciate a review of this. Comment on attachment 169387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169387&action=review > Tools/Scripts/webkitdirs.pm:2032 > + # Configurable features listed here should be kept in sync with the > + # features for which there exists a configuration option in configure.ac. > + my %configurableFeatures = ( > + "filters" => 1, > + "gamepad" => 1, > + "geolocation" => 1, > + "indexed-database" => 1, > + "media-stream" => 1, > + "svg" => 1, > + "svg-fonts" => 1, > + "video" => 1, > + "webgl" => 1, > + "web-audio" => 1, > + "xslt" => 1, > + ); > + my @overridableFeatures = (); > + foreach (@features) { > + if ($configurableFeatures{$_->{option}}) { > + push @buildArgs, autotoolsFlag(${$_->{value}}, $_->{option});; In general I really like the way you're separating stuff that we allow configuration for and stuff build-webkit allows configuration for. I think it would be useful to have a comment around here explaining the overall architecture: that build-webkit parameters will in create a new GNUmakefile.features.am instead of passing parameters to autogen/configure. I think there's also the problem of we having to be more careful when generating the dist tarball - if we pass any parameters to build-webkit we'll get different defaults in the distributed tarball, so be good to make that explicit somewhere, this might be a good place for all that. Thanks for the review. If it's okay with you I'd also like to run a small test of creating the release tarball just to be sure things don't go fubar in that scenario. Also, as discussed with Martin, this change opens the door for again using the build-webkit script (and the FeatureList module) as the preferred way of enabling and disabling the features in developer builds instead of using a configuration flag that overrides the feature defines in Source/WebCore/GNUmakefile.am (In reply to comment #4) > In general I really like the way you're separating stuff that we allow configuration for and stuff build-webkit allows configuration for. I think it would be useful to have a comment around here explaining the overall architecture: that build-webkit parameters will in create a new GNUmakefile.features.am instead of passing parameters to autogen/configure. I think there's also the problem of we having to be more careful when generating the dist tarball - if we pass any parameters to build-webkit we'll get different defaults in the distributed tarball, so be good to make that explicit somewhere, this might be a good place for all that. Announcing on the mailing list that people shouldn't use build-webkit for creating the tarballs. The patch otherwise landed in r137270. http://trac.webkit.org/changeset/137270 |