Remove build-webkit's notion of feature flags having a default value

https://bugs.webkit.org/show_bug.cgi?id=177338 Reviewed by Carlos Alberto Lopez Perez. .: Add an ENABLE_EXPERIMENTAL_FEATURES flag and use it to enable certain features. * Source/cmake/OptionsGTK.cmake: * Source/cmake/WebKitFeatures.cmake: Source/WebKit: Use ENABLE_EXPERIMENTAL_FEATURES instead of ENABLE_DEVELOPER_MODE to enable runtime experimental features. * Shared/WebPreferencesDefaultValues.h: Tools: Delegate feature flag default values to the build system. (FeatureDefines.xcconfig for Xcode, WebKitFeatures.cmake and Options*.cmake for CMake, and FeatureDefines.h for everyone) * BuildSlaveSupport/build.webkit.org-config/config.json: * BuildSlaveSupport/build.webkit.org-config/master.cfg: (Factory.__init__): Update GTK and WPE buildbot configuration to use --no-experimental-features on old stable bots instead of --default-cmake-features. * Scripts/build-webkit: (cMakeArgsFromFeatures): Deleted. Default feature values to 'undef'. Remove the --default-cmake-features argument; it is now the default. Add --no-experimental-features to replace it. Stop printing the default feature flag value in the help. Avoid propagating undefined values to the build system; only mention flags that are overridden on the command line. This has the nice side effect of using the default CMake features for CMake builds, but still allowing toggling of them using the prettier --FEATURE and --no-FEATURE arguments. * Scripts/webkitdirs.pm: (generateBuildSystemFromCMakeProject): No longer need to suppress CMake warnings about unused arguments. * Scripts/webkitperl/FeatureList.pm: Remove the default values from the feature list. Canonical link: https://commits.webkit.org/195966@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225098 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Nov 22, 2017 · c9cef23e05c0feea4b74131c9894be90c6706e5f · c9cef23
1 parent 9690da7
commit c9cef23e05c0feea4b74131c9894be90c6706e5f
Show file tree

Hide file tree

Showing 11 changed files with 176 additions and 105 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,15 @@
+2017-11-22  Tim Horton <timothy_horton@apple.com> and Michael Catanzaro <mcatanzaro@igalia.com>
+
+        Remove build-webkit's notion of feature flags having a default value
+        https://bugs.webkit.org/show_bug.cgi?id=177338
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        Add an ENABLE_EXPERIMENTAL_FEATURES flag and use it to enable certain features.
+
+        * Source/cmake/OptionsGTK.cmake:
+        * Source/cmake/WebKitFeatures.cmake:
+
 2017-11-21  Zan Dobersek  <zdobersek@igalia.com>
 
         Drop ENABLE_IMAGE_DECODER_DOWN_SAMPLING code

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,15 @@
+2017-11-22  Tim Horton <timothy_horton@apple.com> and Michael Catanzaro <mcatanzaro@igalia.com>
+
+        Remove build-webkit's notion of feature flags having a default value
+        https://bugs.webkit.org/show_bug.cgi?id=177338
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        Use ENABLE_EXPERIMENTAL_FEATURES instead of ENABLE_DEVELOPER_MODE to enable runtime
+        experimental features.
+
+        * Shared/WebPreferencesDefaultValues.h:
+
 2017-11-22  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r225093.

diff --git a/Source/WebKit/Shared/WebPreferencesDefaultValues.h b/Source/WebKit/Shared/WebPreferencesDefaultValues.h
@@ -172,9 +172,8 @@
 
 #endif
 
-// Our Xcode build system does not currently have any concept of DEVELOPER_MODE.
 // Cocoa ports must disable experimental features on release branches for now.
-#if ENABLE(DEVELOPER_MODE) || PLATFORM(COCOA)
+#if ENABLE(EXPERIMENTAL_FEATURES) || PLATFORM(COCOA)
 #define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED true
 #else
 #define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED false

diff --git a/Source/cmake/OptionsGTK.cmake b/Source/cmake/OptionsGTK.cmake
@@ -148,6 +148,8 @@ WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_INDEXED_DATABASE_IN_WORKERS PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_INPUT_TYPE_COLOR PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_CONTROLS_SCRIPT PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MHTML PRIVATE ON)
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_SOURCE PRIVATE ${ENABLE_EXPERIMENTAL_FEATURES})
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_STREAM PRIVATE ${ENABLE_EXPERIMENTAL_FEATURES})
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_NOTIFICATIONS PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_PUBLIC_SUFFIX_LIST PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_REMOTE_INSPECTOR PRIVATE ON)
@@ -156,6 +158,7 @@ WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_USERSELECT_ALL PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_USER_MESSAGE_HANDLERS PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WEBGL PRIVATE ON)
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WEB_RTC PRIVATE ${ENABLE_EXPERIMENTAL_FEATURES})
 
 include(GStreamerDefinitions)
 

diff --git a/Source/cmake/WebKitFeatures.cmake b/Source/cmake/WebKitFeatures.cmake
@@ -347,3 +347,6 @@ macro(SET_CONFIGURATION_FOR_UNIFIED_SOURCE_LISTS)
         endif ()
     endforeach ()
 endmacro()
+
+option(ENABLE_EXPERIMENTAL_FEATURES "Enable experimental features" OFF)
+SET_AND_EXPOSE_TO_BUILD(ENABLE_EXPERIMENTAL_FEATURES ${ENABLE_EXPERIMENTAL_FEATURES})
diff --git a/Tools/BuildSlaveSupport/build.webkit.org-config/config.json b/Tools/BuildSlaveSupport/build.webkit.org-config/config.json
@@ -350,13 +350,13 @@
                     {
                       "name": "GTK Linux 64-bit Release Debian Stable (Build)", "type": "Build", "builddir": "gtk-linux-64-release-debian",
                       "platform": "gtk", "configuration": "release", "architectures": ["x86_64"],
-                      "additionalArguments": ["--default-cmake-features"],
+                      "additionalArguments": ["--no-experimental-features"],
                       "slavenames": ["gtk-linux-slave-10"]
                     },
                     {
                       "name": "GTK Linux 64-bit Release Ubuntu LTS (Build)", "type": "Build", "builddir": "gtk-linux-64-release-ubuntu",
                       "platform": "gtk", "configuration": "release", "architectures": ["x86_64"],
-                      "additionalArguments": ["--default-cmake-features"],
+                      "additionalArguments": ["--no-experimental-features"],
                       "slavenames": ["gtk-linux-slave-11"]
                     },
                     {

diff --git a/Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg b/Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg
@@ -878,7 +878,7 @@ class Factory(factory.BuildFactory):
         self.addStep(DeleteStaleBuildFiles())
         if platform == "win":
             self.addStep(InstallWin32Dependencies())
-        if platform == "gtk" and additionalArguments != ["--default-cmake-features"]:
+        if platform == "gtk" and additionalArguments != ["--no-experimental-features"]:
             self.addStep(InstallGtkDependencies())
         if platform == "wpe":
             self.addStep(InstallWpeDependencies())

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
@@ -1,3 +1,39 @@
+2017-11-22  Tim Horton <timothy_horton@apple.com> and Michael Catanzaro <mcatanzaro@igalia.com>
+
+        Remove build-webkit's notion of feature flags having a default value
+        https://bugs.webkit.org/show_bug.cgi?id=177338
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        Delegate feature flag default values to the build system.
+        (FeatureDefines.xcconfig for Xcode, WebKitFeatures.cmake and Options*.cmake for CMake, and FeatureDefines.h for everyone)
+
+        * BuildSlaveSupport/build.webkit.org-config/config.json:
+        * BuildSlaveSupport/build.webkit.org-config/master.cfg:
+        (Factory.__init__):
+        Update GTK and WPE buildbot configuration to use --no-experimental-features on old stable
+        bots instead of --default-cmake-features.
+
+        * Scripts/build-webkit:
+        (cMakeArgsFromFeatures): Deleted.
+        Default feature values to 'undef'.
+        Remove the --default-cmake-features argument; it is now the default.
+        Add --no-experimental-features to replace it.
+        Stop printing the default feature flag value in the help.
+        Avoid propagating undefined values to the build system;
+        only mention flags that are overridden on the command line.
+
+        This has the nice side effect of using the default CMake features
+        for CMake builds, but still allowing toggling of them using the
+        prettier --FEATURE and --no-FEATURE arguments.
+
+        * Scripts/webkitdirs.pm:
+        (generateBuildSystemFromCMakeProject):
+        No longer need to suppress CMake warnings about unused arguments.
+
+        * Scripts/webkitperl/FeatureList.pm:
+        Remove the default values from the feature list.
+
 2017-11-22  Mark Lam  <mark.lam@apple.com>
 
         [Re-landing] Pass --useDollarVM=true to all JSC tests.

diff --git a/Tools/Scripts/build-webkit b/Tools/Scripts/build-webkit
@@ -53,7 +53,6 @@ chdirWebKit();
 my $showHelp = 0;
 my $verbose = 0;
 my $clean = 0;
-my $defaultCMakeFeatures = 0;
 my $minimal = 0;
 my $installHeaders;
 my $installLibs;
@@ -63,6 +62,7 @@ my @cmakeArgs;
 my $onlyWebKitProject = 0;
 my $coverageSupport = 0;
 my $shouldRunStaticAnalyzer = 0;
+my $noExperimentalFeatures = 0;
 my $startTime = time();
 my $archs32bit = 0;
 my $skipLibraryUpdate = 0;
@@ -82,9 +82,10 @@ foreach (@ARGV) {
     }
 }
 
-# Initialize values from defaults
+# Feature flags default to undefined, where they will inherit the default value
+# specified by the build system, or to 'off' if --minimal is specified.
 foreach (@features) {
-    ${$_->{value}} = ($minimal ? 0 : $_->{default});
+    ${$_->{value}} = ($minimal ? 0 : undef);
 }
 
 my $programName = basename($0);
@@ -110,12 +111,12 @@ Usage: $programName [options] [options to pass to build system]
 
   --inspector-frontend              Copy Web Inspector user interface resources to the build directory
 
-  --prefix=<path>                   Set installation prefix to the given path (Gtk/Efl only)
+  --prefix=<path>                   Set installation prefix to the given path (CMake only, except Windows)
   --makeargs=<arguments>            Optional Makefile flags
   --cmakeargs=<arguments>           One or more optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local")
 
   --minimal                         No optional features, unless explicitly enabled
-  --default-cmake-features          Use the default CMake features enabled for the port (CMake based ports)
+  --no-experimental-features        No experimental features, unless explicitly enabled (CMake only)
 
   --only-webkit                     Build only the WebKit project
 
@@ -138,15 +139,15 @@ my %options = (
     'only-webkit' => \$onlyWebKitProject,
     'coverage' => \$coverageSupport,
     'analyze' => \$shouldRunStaticAnalyzer,
-    'default-cmake-features' => \$defaultCMakeFeatures,
+    'no-experimental-features' => \$noExperimentalFeatures,
     'skip-library-update' => \$skipLibraryUpdate,
     'use-ccache!' => \$useCCache,
 );
 
 # Build usage text and options list from features
 foreach (@features) {
     my $opt = sprintf("%-35s", "  --[no-]$_->{option}");
-    $usage .= "$opt $_->{desc} (default: $_->{default})\n";
+    $usage .= "$opt $_->{desc}\n";
     $options{"$_->{option}!"} = $_->{value};
 }
 
@@ -195,15 +196,15 @@ my @options = ();
 
 if (isAppleCocoaWebKit()) {
     push @options, XcodeOptions();
-    sub option($$$)
+    sub option($$)
     {
-        my ($feature, $isEnabled, $defaultValue) = @_;
-        return "" if $defaultValue == $isEnabled;
+        my ($feature, $isEnabled) = @_;
+        return "" if not defined $isEnabled;
         return $feature . "=" . ($isEnabled ? $feature : "");
     }
 
     foreach (@features) {
-        my $option = option($_->{define}, ${$_->{value}}, $_->{default});
+        my $option = option($_->{define}, ${$_->{value}});
         push @options, $option unless $option eq "";
     }
 
@@ -272,7 +273,7 @@ if (isCMakeBuild() && !isAnyWindows()) {
     my $maxCPULoad = maxCPULoad() if $makeArgs !~ /-l\s*\d+\.?\d*/;
     $makeArgs .= " -l" . maxCPULoad() if defined $maxCPULoad;
 
-    # We remove CMakeCache to avoid the bots to reuse cached flags when
+    # We remove CMakeCache to avoid the bots reusing cached flags when
     # we enable new features. This forces a reconfiguration.
     my @featureArgs = cMakeArgsFromFeatures();
     removeCMakeCache(@featureArgs);
@@ -369,15 +370,22 @@ exit 0;
 sub cMakeArgsFromFeatures()
 {
     my @args;
-    if (!$defaultCMakeFeatures) {
-        foreach (@features) {
-            my $featureName = $_->{define};
-            if ($featureName) {
-                my $featureEnabled = ${$_->{value}} ? "ON" : "OFF";
+
+    if (!$noExperimentalFeatures) {
+        push @args, "-DENABLE_EXPERIMENTAL_FEATURES=ON";
+    }
+
+    foreach (@features) {
+        my $featureName = $_->{define};
+        if ($featureName) {
+            my $featureValue = ${$_->{value}};
+            if (defined $featureValue) {
+                my $featureEnabled = $featureValue ? "ON" : "OFF";
                 push @args, "-D$featureName=$featureEnabled";
             }
         }
     }
+
     return @args;
 }
 

diff --git a/Tools/Scripts/webkitdirs.pm b/Tools/Scripts/webkitdirs.pm
@@ -2236,8 +2236,6 @@ sub generateBuildSystemFromCMakeProject
     # Some ports have production mode, but build-webkit should always use developer mode.
     push @args, "-DDEVELOPER_MODE=ON" if isGtk() || isJSCOnly() || isWPE();
 
-    # Don't warn variables which aren't used by cmake ports.
-    push @args, "--no-warn-unused-cli";
     push @args, @cmakeArgs if @cmakeArgs;
 
     my $cmakeSourceDir = isCygwin() ? windowsSourceDir() : sourceDir();