diff --git a/.ci.yaml b/.ci.yaml index 129c4054476..c6686ff9828 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -1164,23 +1164,6 @@ targets: ["devicelab", "hostonly", "linux"] task_name: linux_desktop_impeller - - name: Linux linux_feature_flags_test - recipe: devicelab/devicelab_drone - timeout: 60 - bringup: true - presubmit: false - properties: - xvfb: "1" - dependencies: >- - [ - {"dependency": "clang", "version": "git_revision:5d5aba78dbbee75508f01bcaa69aedb2ab79065a"}, - {"dependency": "cmake", "version": "build_id:8787856497187628321"}, - {"dependency": "ninja", "version": "version:1.9.0"} - ] - tags: > - ["devicelab", "hostonly", "linux"] - task_name: linux_feature_flags_test - - name: Linux_android_emu android_display_cutout recipe: devicelab/devicelab_drone bringup: true # LUCI failing KVM access https://github.com/flutter/flutter/issues/170529 diff --git a/TESTOWNERS b/TESTOWNERS index c3ae76bbd6c..23a8d7c1992 100644 --- a/TESTOWNERS +++ b/TESTOWNERS @@ -303,12 +303,11 @@ /dev/devicelab/bin/tasks/web_benchmarks_ddc_hot_reload.dart @yjbanov @flutter/web /dev/devicelab/bin/tasks/web_benchmarks_skwasm.dart @eyebrowsoffire @flutter/web /dev/devicelab/bin/tasks/web_benchmarks_skwasm_st.dart @eyebrowsoffire @flutter/web -/dev/devicelab/bin/tasks/windows_desktop_impeller.dart @jonahwilliams @flutter/engine /dev/devicelab/bin/tasks/windows_home_scroll_perf__timeline_summary.dart @jonahwilliams @flutter/engine /dev/devicelab/bin/tasks/windows_startup_test.dart @loic-sharma @flutter/desktop +/dev/devicelab/bin/tasks/windows_desktop_impeller.dart @jonahwilliams @flutter/engine /dev/devicelab/bin/tasks/mac_desktop_impeller.dart @jonahwilliams @flutter/engine /dev/devicelab/bin/tasks/linux_desktop_impeller.dart @jonahwilliams @flutter/engine -/dev/devicelab/bin/tasks/linux_feature_flags_test.dart @loic-sharma @flutter/desktop /dev/devicelab/bin/tasks/android_display_cutout.dart @reidbaker @flutter/android ## Host only framework tests diff --git a/dev/devicelab/bin/tasks/linux_feature_flags_test.dart b/dev/devicelab/bin/tasks/linux_feature_flags_test.dart deleted file mode 100644 index 24af236e68d..00000000000 --- a/dev/devicelab/bin/tasks/linux_feature_flags_test.dart +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:flutter_devicelab/framework/devices.dart'; -import 'package:flutter_devicelab/framework/framework.dart'; -import 'package:flutter_devicelab/tasks/integration_tests.dart'; - -/// Verify that feature flags work on Linux. -Future main() async { - deviceOperatingSystem = DeviceOperatingSystem.linux; - await task(featureFlagsTask()); -} diff --git a/dev/devicelab/lib/tasks/integration_tests.dart b/dev/devicelab/lib/tasks/integration_tests.dart index f791e0c7757..4687dc4ef8c 100644 --- a/dev/devicelab/lib/tasks/integration_tests.dart +++ b/dev/devicelab/lib/tasks/integration_tests.dart @@ -188,16 +188,6 @@ TaskFunction dartDefinesTask() { ).call; } -TaskFunction featureFlagsTask() { - return DriverTest( - '${flutterDirectory.path}/dev/integration_tests/ui', - 'lib/feature_flags.dart', - // TODO(loic-sharma): Turn on a framework feature flag once one exists. - // https://github.com/flutter/flutter/issues/167668 - environment: const {}, - ).call; -} - TaskFunction createEndToEndIntegrationTest() { return IntegrationTest( '${flutterDirectory.path}/dev/integration_tests/ui', diff --git a/dev/integration_tests/ui/lib/feature_flags.dart b/dev/integration_tests/ui/lib/feature_flags.dart deleted file mode 100644 index 27463b54a24..00000000000 --- a/dev/integration_tests/ui/lib/feature_flags.dart +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// ignore: implementation_imports -import 'package:flutter/src/foundation/_features.dart'; -import 'package:flutter/widgets.dart'; -import 'package:flutter_driver/driver_extension.dart'; - -/// This application displays the framework's enabled feature flags as a string. -void main() { - enableFlutterDriverExtension(); - runApp( - Center( - child: Text( - // ignore: invalid_use_of_internal_member - 'Feature flags: "$debugEnabledFeatureFlags"', - textDirection: TextDirection.ltr, - ), - ), - ); -} diff --git a/dev/integration_tests/ui/test_driver/feature_flags_test.dart b/dev/integration_tests/ui/test_driver/feature_flags_test.dart deleted file mode 100644 index c7e8f9447b3..00000000000 --- a/dev/integration_tests/ui/test_driver/feature_flags_test.dart +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:flutter_driver/flutter_driver.dart'; -import 'package:test/test.dart' hide TypeMatcher, isInstanceOf; - -void main() { - FlutterDriver? driver; - - setUpAll(() async { - driver = await FlutterDriver.connect(); - }); - - tearDownAll(() async { - await driver?.close(); - }); - - test('Can run with feature flags enabled', () async { - // TODO(loic-sharma): Turn on a framework feature flag once one exists. - // https://github.com/flutter/flutter/issues/167668 - await driver?.waitFor(find.text('Feature flags: "{}"')); - }, timeout: Timeout.none); -} diff --git a/docs/contributing/Feature-flags.md b/docs/contributing/Feature-flags.md index 7de3dbcaf43..9043429ece1 100644 --- a/docs/contributing/Feature-flags.md +++ b/docs/contributing/Feature-flags.md @@ -20,7 +20,6 @@ Table of Contents - [Allowing flags to be enabled](#allowing-flags-to-be-enabled) - [Enabling a flag by default](#enabling-a-flag-by-default) - [Removing a flag](#removing-a-flag) - - [Precedence](#precedence) - [Using a flag to drive behavior](#using-a-flag-to-drive-behavior) - [Tool](#tool) - [Framework](#framework) @@ -207,43 +206,6 @@ integration tests as well. [^1]: Some flags might have a longer or indefinite lifespan, but this is rare. -### Precedence - -Users have several options to configure flags. Assuming the following feature: - -```dart -const Feature unicornEmojis = Feature( - name: 'add unicorn emojis in lots of fun places', - configSetting: 'enable-unicorn-emojis', - environmentOverride: 'FLUTTER_ENABLE_UNICORN_EMOJIS', -); -``` - -Flutter uses the following precendence order: - -1. The app's `pubspec.yaml` file: - - ```yaml - flutter: - config: - enable-unicorn-emojis: true - ``` - -2. The tool's global configuration: - - ```sh - flutter config --enable-unicorn-emojis - ``` - -3. Environment variables: - - ```sh - FLUTTER_ENABLE_UNICORN_EMOJIS=true flutter some-command - ``` - -If none of these are set, Flutter falls back to the feature's -default value for the current release channel. - ## Using a flag to drive behavior Once you have a flag, you can use it to conditionally enable something or @@ -311,10 +273,6 @@ final class SensitiveContent extends StatelessWidget { Note that feature flag usage in the framework runtime is very new, and is likely to evolve over time. -Feature flags are not designed to help tree shaking. For example, you -cannot conditionally import Dart code depending on the enabled feature flags. -Tree shaking might not remove code that is feature flagged off. - ### Tests #### Integration tests diff --git a/packages/flutter/lib/src/foundation/_features.dart b/packages/flutter/lib/src/foundation/_features.dart deleted file mode 100644 index 801253817ad..00000000000 --- a/packages/flutter/lib/src/foundation/_features.dart +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:meta/meta.dart'; - -/// The feature flags this app was built with. -/// -/// Do not use this API. Flutter can and will make breaking changes to this API. -@internal -Set debugEnabledFeatureFlags = { - ...const String.fromEnvironment('FLUTTER_ENABLED_FEATURE_FLAGS').split(','), -}; diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart index e4ec92c3378..5b1c19aacda 100644 --- a/packages/flutter_tools/lib/src/build_info.dart +++ b/packages/flutter_tools/lib/src/build_info.dart @@ -1014,12 +1014,6 @@ const String kFlavor = 'Flavor'; /// by the `appFlavor` service. const String kAppFlavor = 'FLUTTER_APP_FLAVOR'; -/// Environment variable of the enabled feature flags to be set in the -/// dartDefines. -/// -/// This tells the framework which features are on. -const String kEnabledFeatureFlags = 'FLUTTER_ENABLED_FEATURE_FLAGS'; - /// The Xcode configuration used to build the project. const String kXcodeConfiguration = 'Configuration'; diff --git a/packages/flutter_tools/lib/src/features.dart b/packages/flutter_tools/lib/src/features.dart index 277eb3c5357..b2ebc93a5f3 100644 --- a/packages/flutter_tools/lib/src/features.dart +++ b/packages/flutter_tools/lib/src/features.dart @@ -184,7 +184,6 @@ class Feature { required this.name, this.environmentOverride, this.configSetting, - this.runtimeId, this.extraHelpText, this.master = const FeatureChannelSetting(), this.beta = const FeatureChannelSetting(), @@ -196,7 +195,6 @@ class Feature { required this.name, this.environmentOverride, this.configSetting, - this.runtimeId, this.extraHelpText, }) : master = const FeatureChannelSetting(available: true, enabledByDefault: true), beta = const FeatureChannelSetting(available: true, enabledByDefault: true), @@ -228,12 +226,6 @@ class Feature { /// If not provided, defaults to `null` meaning there is no config setting. final String? configSetting; - /// The unique identifier for this feature at runtime. - /// - /// If not `null`, the Flutter framework's enabled feature flags will - /// contain this value if this feature is enabled. - final String? runtimeId; - /// Additional text to add to the end of the help message. /// /// If not provided, defaults to `null` meaning there is no additional text. diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 82d70abbc9f..bc6f4e9c43f 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -1446,7 +1446,6 @@ abstract class FlutterCommand extends Command { dartDefines.add('$kAppFlavor=$flavor'); } _addFlutterVersionToDartDefines(globals.flutterVersion, dartDefines); - _addFeatureFlagsToDartDefines(dartDefines); return BuildInfo( buildMode, @@ -1509,27 +1508,6 @@ abstract class FlutterCommand extends Command { ]); } - void _addFeatureFlagsToDartDefines(List dartDefines) { - if (dartDefines.any((String define) => define.startsWith(kEnabledFeatureFlags))) { - throwToolExit( - '$kEnabledFeatureFlags is used by the framework and cannot be ' - 'set using --${FlutterOptions.kDartDefinesOption} or --${FlutterOptions.kDartDefineFromFileOption}.\n' - '\n' - 'Use the "flutter config" command to enable feature flags.', - ); - } - - final String enabledFeatureFlags = featureFlags.allFeatures - .where((Feature feature) => featureFlags.isEnabled(feature)) - .where((Feature feature) => feature.runtimeId != null) - .map((Feature feature) => feature.runtimeId!) - .join(','); - - if (enabledFeatureFlags.isNotEmpty) { - dartDefines.add('$kEnabledFeatureFlags=$enabledFeatureFlags'); - } - } - void setupApplicationPackages() { applicationPackages ??= ApplicationPackageFactory.instance; } diff --git a/packages/flutter_tools/test/general.shard/features_test.dart b/packages/flutter_tools/test/general.shard/features_test.dart index 96117171b7d..b1b74bba4d9 100644 --- a/packages/flutter_tools/test/general.shard/features_test.dart +++ b/packages/flutter_tools/test/general.shard/features_test.dart @@ -137,22 +137,6 @@ void main() { expect(featureNames, unorderedEquals(testFeatureNames)); }); - - testUsingContext('Feature runtime IDs are valid', () { - // Verify features' runtime IDs can be encoded into a Dart define. - final RegExp runtimeIdPattern = RegExp(r'^[a-zA-Z_]+$'); - assert(runtimeIdPattern.hasMatch('multi_window')); - assert(!runtimeIdPattern.hasMatch('multi-window')); - - final Iterable runtimeIds = - featureFlags.allFeatures.map((Feature feature) => feature.runtimeId).nonNulls; - - expect( - runtimeIds, - everyElement(matches(runtimeIdPattern)), - reason: 'Feature runtime ID must contain only alphabetical or underscore characters', - ); - }); }); group('Linux Destkop', () { diff --git a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart index 28557079959..fbda6491f99 100644 --- a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart @@ -20,7 +20,6 @@ import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/run.dart' show RunCommand; import 'package:flutter_tools/src/device.dart'; -import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/pre_run_validator.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; @@ -1551,88 +1550,6 @@ flutter: }, ); }); - - group('feature flags', () { - testUsingContext( - 'tool exits when FLUTTER_ENABLED_FEATURE_FLAGS is set in --dart-define or --dart-define-from-file', - () async { - final CommandRunner runner = createTestCommandRunner( - _TestRunCommandThatOnlyValidates(), - ); - - expect( - runner.run([ - 'run', - '--dart-define=FLUTTER_ENABLED_FEATURE_FLAGS=AlreadySet', - '--no-pub', - '--no-hot', - ]), - throwsToolExit( - message: ''' -FLUTTER_ENABLED_FEATURE_FLAGS is used by the framework and cannot be set using --dart-define or --dart-define-from-file. - -Use the "flutter config" command to enable feature flags.''', - ), - ); - - expect( - runner.run([ - 'run', - '--dart-define-from-file=config.json', - '--no-pub', - '--no-hot', - ]), - throwsToolExit( - message: ''' -FLUTTER_ENABLED_FEATURE_FLAGS is used by the framework and cannot be set using --dart-define or --dart-define-from-file. - -Use the "flutter config" command to enable feature flags.''', - ), - ); - }, - overrides: { - DeviceManager: - () => FakeDeviceManager()..attachedDevices = [FakeDevice('name', 'id')], - Platform: () => FakePlatform(), - Cache: () => Cache.test(processManager: FakeProcessManager.any()), - FileSystem: () { - final MemoryFileSystem fileSystem = MemoryFileSystem.test(); - fileSystem - ..file('lib/main.dart').createSync(recursive: true) - ..file('pubspec.yaml').createSync() - ..file('.packages').createSync(); - fileSystem.file('config.json') - ..createSync() - ..writeAsStringSync('{"FLUTTER_ENABLED_FEATURE_FLAGS": "AlreadySet"}'); - return fileSystem; - }, - ProcessManager: () => FakeProcessManager.any(), - FlutterVersion: () => FakeFlutterVersion(), - }, - ); - - testUsingContext( - 'FLUTTER_ENABLED_FEATURE_FLAGS is set in dartDefines', - () async { - final DummyFlutterCommand flutterCommand = DummyFlutterCommand(packagesPath: 'foo'); - final BuildInfo buildInfo = await flutterCommand.getBuildInfo( - forcedBuildMode: BuildMode.debug, - ); - expect(buildInfo.dartDefines, contains('FLUTTER_ENABLED_FEATURE_FLAGS=buzz_feature')); - }, - overrides: { - ProcessManager: () => FakeProcessManager.any(), - FeatureFlags: - () => const FakeFeatureFlags( - allFeatures: [ - FakeFeature(name: 'Foo', enabled: true), - FakeFeature(name: 'Bar', runtimeId: 'bar_feature', enabled: false), - FakeFeature(name: 'Buzz', runtimeId: 'buzz_feature', enabled: true), - ], - ), - }, - ); - }); }); } @@ -1754,22 +1671,3 @@ class _TestRunCommandThatOnlyValidates extends RunCommand { @override bool get shouldRunPub => false; } - -class FakeFeature extends Feature { - const FakeFeature({required super.name, super.runtimeId, required this.enabled}); - - final bool enabled; -} - -class FakeFeatureFlags implements FeatureFlags { - const FakeFeatureFlags({required this.allFeatures}); - - @override - final List allFeatures; - - @override - bool isEnabled(Feature feature) => (feature as FakeFeature).enabled; - - @override - Object? noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); -}