Unverified Commit 36c37cca authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

Implement feature flag system for flutter tools (#36138)

parent 3a60713c
......@@ -27,7 +27,7 @@ class Config {
dynamic getValue(String key) => _values[key];
void setValue(String key, String value) {
void setValue(String key, Object value) {
_values[key] = value;
_flushValues();
}
......
......@@ -7,9 +7,11 @@ import 'dart:async';
import '../android/android_sdk.dart';
import '../android/android_studio.dart';
import '../convert.dart';
import '../features.dart';
import '../globals.dart';
import '../reporting/usage.dart';
import '../runner/flutter_command.dart';
import '../version.dart';
class ConfigCommand extends FlutterCommand {
ConfigCommand({ bool verboseHelp = false }) {
......@@ -26,6 +28,21 @@ class ConfigCommand extends FlutterCommand {
negatable: false,
hide: !verboseHelp,
help: 'Print config values as json.');
for (Feature feature in allFeatures) {
if (feature.configSetting == null) {
continue;
}
argParser.addFlag(
feature.configSetting,
help: feature.generateHelpMessage(),
negatable: true,
);
}
argParser.addFlag(
'clear-features',
help: 'Remove all configured features and restore them to the default values.',
negatable: false,
);
}
@override
......@@ -44,14 +61,27 @@ class ConfigCommand extends FlutterCommand {
@override
bool get shouldUpdateCache => false;
@override
Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{};
@override
String get usageFooter {
// List all config settings.
String values = config.keys.map<String>((String key) {
return ' $key: ${config.getValue(key)}';
// List all config settings. for feature flags, include whether they
// are available.
final Map<String, Feature> featuresByName = <String, Feature>{};
final String channel = FlutterVersion.instance.channel;
for (Feature feature in allFeatures) {
if (feature.configSetting != null) {
featuresByName[feature.configSetting] = feature;
}
}
String values = config.keys
.map<String>((String key) {
String configFooter = '';
if (featuresByName.containsKey(key)) {
final FeatureChannelSetting setting = featuresByName[key].getSettingForChannel(channel);
if (!setting.available) {
configFooter = '(Unavailable)';
}
}
return ' $key: ${config.getValue(key)} $configFooter';
}).join('\n');
if (values.isEmpty)
values = ' No settings have been configured.';
......@@ -71,6 +101,15 @@ class ConfigCommand extends FlutterCommand {
return null;
}
if (argResults['clear-features']) {
for (Feature feature in allFeatures) {
if (feature.configSetting != null) {
config.removeValue(feature.configSetting);
}
}
return null;
}
if (argResults.wasParsed('analytics')) {
final bool value = argResults['analytics'];
flutterUsage.enabled = value;
......@@ -89,6 +128,17 @@ class ConfigCommand extends FlutterCommand {
if (argResults.wasParsed('clear-ios-signing-cert'))
_updateConfig('ios-signing-cert', '');
for (Feature feature in allFeatures) {
if (feature.configSetting == null) {
continue;
}
if (argResults.wasParsed(feature.configSetting)) {
final bool keyValue = argResults[feature.configSetting];
config.setValue(feature.configSetting, keyValue);
printStatus('Setting "${feature.configSetting}" value to "$keyValue".');
}
}
if (argResults.arguments.isEmpty)
printStatus(usage);
......
......@@ -28,6 +28,7 @@ import 'devfs.dart';
import 'device.dart';
import 'doctor.dart';
import 'emulator.dart';
import 'features.dart';
import 'fuchsia/fuchsia_device.dart' show FuchsiaDeviceTools;
import 'fuchsia/fuchsia_sdk.dart' show FuchsiaSdk, FuchsiaArtifacts;
import 'fuchsia/fuchsia_workflow.dart' show FuchsiaWorkflow;
......@@ -79,6 +80,7 @@ Future<T> runInContext<T>(
Doctor: () => const Doctor(),
DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance,
EmulatorManager: () => EmulatorManager(),
FeatureFlags: () => const FeatureFlags(),
Flags: () => const EmptyFlags(),
FlutterVersion: () => FlutterVersion(const SystemClock()),
FuchsiaArtifacts: () => FuchsiaArtifacts.find(),
......
// Copyright 2019 The Chromium 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';
import 'base/config.dart';
import 'base/context.dart';
import 'base/platform.dart';
import 'version.dart';
/// The current [FeatureFlags] implementation.
///
/// If not injected, a default implementation is provided.
FeatureFlags get featureFlags => context.get<FeatureFlags>();
/// The interface used to determine if a particular [Feature] is enabled.
///
/// The rest of the tools code should use this class instead of looking up
/// features directly. To faciliate rolls to google3 and other clients, all
/// flags should be provided with a default implementation here. Clients that
/// use this class should extent instead of implement, so that new flags are
/// picked up automatically.
class FeatureFlags {
const FeatureFlags();
/// Whether flutter desktop for linux is enabled.
bool get isLinuxEnabled => _isEnabled(flutterLinuxDesktopFeature);
/// Whether flutter desktop for macOS is enabled.
bool get isMacOSEnabled => _isEnabled(flutterMacOSDesktopFeature);
/// Whether flutter web is enabled.
bool get isWebEnabled => _isEnabled(flutterWebFeature);
/// Whether flutter desktop for Windows is enabled.
bool get isWindowsEnabled => _isEnabled(flutterWindowsDesktopFeature);
// Calculate whether a particular feature is enabled for the current channel.
static bool _isEnabled(Feature feature) {
final String currentChannel = FlutterVersion.instance.channel;
final FeatureChannelSetting featureSetting = feature.getSettingForChannel(currentChannel);
if (!featureSetting.available) {
return false;
}
bool isEnabled = featureSetting.enabledByDefault;
if (feature.configSetting != null) {
final bool configOverride = Config.instance.getValue(feature.configSetting);
if (configOverride != null) {
isEnabled = configOverride;
}
}
if (feature.environmentOverride != null) {
if (platform.environment[feature.environmentOverride]?.toLowerCase() == 'true') {
isEnabled = true;
}
}
return isEnabled;
}
}
/// All current Flutter feature flags.
const List<Feature> allFeatures = <Feature>[
flutterWebFeature,
flutterLinuxDesktopFeature,
flutterMacOSDesktopFeature,
flutterWindowsDesktopFeature,
];
/// The [Feature] for flutter web.
const Feature flutterWebFeature = Feature(
name: 'Flutter Web',
configSetting: 'enable-web',
environmentOverride: 'FLUTTER_WEB',
master: FeatureChannelSetting(
available: true,
enabledByDefault: false,
),
dev: FeatureChannelSetting(
available: true,
enabledByDefault: false,
),
);
/// The [Feature] for macOS desktop.
const Feature flutterMacOSDesktopFeature = Feature(
name: 'Flutter Desktop for macOS',
configSetting: 'enable-macos-desktop',
environmentOverride: 'ENABLE_FLUTTER_DESKTOP',
master: FeatureChannelSetting(
available: true,
enabledByDefault: false,
),
);
/// The [Feature] for Linux desktop.
const Feature flutterLinuxDesktopFeature = Feature(
name: 'Flutter Desktop for Linux',
configSetting: 'enable-linux-desktop',
environmentOverride: 'ENABLE_FLUTTER_DESKTOP',
master: FeatureChannelSetting(
available: true,
enabledByDefault: false,
),
);
/// The [Feature] for Windows desktop.
const Feature flutterWindowsDesktopFeature = Feature(
name: 'Flutter Desktop for Windows',
configSetting: 'enable-windows-desktop',
environmentOverride: 'ENABLE_FLUTTER_DESKTOP',
master: FeatureChannelSetting(
available: true,
enabledByDefault: false,
),
);
/// A [Feature] is a process for conditionally enabling tool features.
///
/// All settings are optional, and if not provided will generally default to
/// a "safe" value, such as being off.
///
/// The top level feature settings can be provided to apply to all channels.
/// Otherwise, more specific settings take precidence over higher level
/// settings.
class Feature {
/// Creates a [Feature].
const Feature({
@required this.name,
this.environmentOverride,
this.configSetting,
this.master = const FeatureChannelSetting(),
this.dev = const FeatureChannelSetting(),
this.beta = const FeatureChannelSetting(),
this.stable = const FeatureChannelSetting(),
});
/// The user visible name for this feature.
final String name;
/// The settings for the master branch and other unknown channels.
final FeatureChannelSetting master;
/// The settings for the dev branch.
final FeatureChannelSetting dev;
/// The settings for the beta branch.
final FeatureChannelSetting beta;
/// The settings for the stable branch.
final FeatureChannelSetting stable;
/// The name of an environment variable that can override the setting.
///
/// The environment variable needs to be set to the value 'true'. This is
/// only intended for usage by CI and not as an advertised method to enable
/// a feature.
///
/// If not provided, defaults to `null` meaning there is no override.
final String environmentOverride;
/// The name of a setting that can be used to enable this feature.
///
/// If not provided, defaults to `null` meaning there is no config setting.
final String configSetting;
/// A help message for the `flutter config` command, or null if unsupported.
String generateHelpMessage() {
if (configSetting == null) {
return null;
}
final StringBuffer buffer = StringBuffer('Enable or disable $name on ');
final List<String> channels = <String>[
if (master.available) 'master',
if (dev.available) 'dev',
if (beta.available) 'beta',
if (stable.available) 'stable',
];
if (channels.length == 1) {
buffer.write('the ${channels.single} channel.');
} else {
buffer.write('${channels.join(', ')} channels.');
}
return buffer.toString();
}
/// Retrieve the correct setting for the provided `channel`.
FeatureChannelSetting getSettingForChannel(String channel) {
switch (channel) {
case 'stable':
return stable;
case 'beta':
return beta;
case 'dev':
return dev;
case 'master':
default:
return master;
}
}
}
/// A description of the conditions to enable a feature for a particular channel.
class FeatureChannelSetting {
const FeatureChannelSetting({
this.available = false,
this.enabledByDefault = false,
});
/// Whether the feature is available on this channel.
///
/// If not provded, defaults to `false`. This implies that the feature
/// cannot be enabled even by the settings below.
final bool available;
/// Whether the feature is enabled by default.
///
/// If not provided, defaults to `false`.
final bool enabledByDefault;
}
......@@ -7,11 +7,13 @@ import 'dart:async';
import 'package:meta/meta.dart';
import 'package:usage/usage_io.dart';
import '../base/config.dart';
import '../base/context.dart';
import '../base/file_system.dart';
import '../base/os.dart';
import '../base/platform.dart';
import '../base/utils.dart';
import '../features.dart';
import '../globals.dart';
import '../version.dart';
......@@ -55,7 +57,9 @@ const String reloadExceptionTargetPlatform = 'cd27';
const String reloadExceptionSdkName = 'cd28';
const String reloadExceptionEmulator = 'cd29';
const String reloadExceptionFullRestart = 'cd30';
// Next ID: cd32
const String enabledFlutterFeatures = 'cd32';
// Next ID: cd33
Usage get flutterUsage => Usage.instance;
......@@ -72,6 +76,17 @@ class Usage {
_analytics.setSessionValue(kSessionHostOsDetails, os.name);
// Send the branch name as the "channel".
_analytics.setSessionValue(kSessionChannelName, flutterVersion.getBranchName(redactUnknownBranches: true));
// For each flutter experimental feature, record a session value in a comma
// separated list.
final String enabledFeatures = allFeatures
.where((Feature feature) {
return feature.configSetting != null &&
Config.instance.getValue(feature.configSetting) == true;
})
.map((Feature feature) => feature.configSetting)
.join(',');
_analytics.setSessionValue(enabledFlutterFeatures, enabledFeatures);
// Record the host as the application installer ID - the context that flutter_tools is running in.
if (platform.environment.containsKey('FLUTTER_HOST')) {
_analytics.setSessionValue('aiid', platform.environment['FLUTTER_HOST']);
......
......@@ -4,11 +4,15 @@
import 'dart:convert';
import 'package:args/command_runner.dart';
import 'package:flutter_tools/src/android/android_sdk.dart';
import 'package:flutter_tools/src/android/android_studio.dart';
import 'package:flutter_tools/src/base/config.dart';
import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/commands/config.dart';
import 'package:flutter_tools/src/version.dart';
import 'package:mockito/mockito.dart';
import '../../src/common.dart';
......@@ -17,10 +21,16 @@ import '../../src/context.dart';
void main() {
MockAndroidStudio mockAndroidStudio;
MockAndroidSdk mockAndroidSdk;
MockFlutterVersion mockFlutterVersion;
setUpAll(() {
Cache.disableLocking();
});
setUp(() {
mockAndroidStudio = MockAndroidStudio();
mockAndroidSdk = MockAndroidSdk();
mockFlutterVersion = MockFlutterVersion();
});
group('config', () {
......@@ -42,6 +52,77 @@ void main() {
AndroidStudio: () => mockAndroidStudio,
AndroidSdk: () => mockAndroidSdk,
});
testUsingContext('allows setting and removing feature flags', () async {
final ConfigCommand configCommand = ConfigCommand();
final CommandRunner<void> commandRunner = createTestCommandRunner(configCommand);
await commandRunner.run(<String>[
'config',
'--enable-web',
'--enable-linux-desktop',
'--enable-windows-desktop',
'--enable-macos-desktop'
]);
expect(Config.instance.getValue('enable-web'), true);
expect(Config.instance.getValue('enable-linux-desktop'), true);
expect(Config.instance.getValue('enable-windows-desktop'), true);
expect(Config.instance.getValue('enable-macos-desktop'), true);
await commandRunner.run(<String>[
'config', '--clear-features',
]);
expect(Config.instance.getValue('enable-web'), null);
expect(Config.instance.getValue('enable-linux-desktop'), null);
expect(Config.instance.getValue('enable-windows-desktop'), null);
expect(Config.instance.getValue('enable-macos-desktop'), null);
await commandRunner.run(<String>[
'config',
'--no-enable-web',
'--no-enable-linux-desktop',
'--no-enable-windows-desktop',
'--no-enable-macos-desktop'
]);
expect(Config.instance.getValue('enable-web'), false);
expect(Config.instance.getValue('enable-linux-desktop'), false);
expect(Config.instance.getValue('enable-windows-desktop'), false);
expect(Config.instance.getValue('enable-macos-desktop'), false);
}, overrides: <Type, Generator>{
AndroidStudio: () => mockAndroidStudio,
AndroidSdk: () => mockAndroidSdk,
});
testUsingContext('displays which config settings are available on stable', () async {
final BufferLogger logger = context.get<Logger>();
when(mockFlutterVersion.channel).thenReturn('stable');
final ConfigCommand configCommand = ConfigCommand();
final CommandRunner<void> commandRunner = createTestCommandRunner(configCommand);
await commandRunner.run(<String>[
'config',
'--enable-web',
'--enable-linux-desktop',
'--enable-windows-desktop',
'--enable-macos-desktop'
]);
await commandRunner.run(<String>[
'config',
]);
expect(logger.statusText, contains('enable-web: true (Unavailable)'));
expect(logger.statusText, contains('enable-linux-desktop: true (Unavailable)'));
expect(logger.statusText, contains('enable-windows-desktop: true (Unavailable)'));
expect(logger.statusText, contains('enable-macos-desktop: true (Unavailable)'));
}, overrides: <Type, Generator>{
AndroidStudio: () => mockAndroidStudio,
AndroidSdk: () => mockAndroidSdk,
FlutterVersion: () => mockFlutterVersion,
});
});
}
......@@ -54,3 +135,5 @@ class MockAndroidSdk extends Mock implements AndroidSdk {
@override
String get directory => 'path/to/android/sdk';
}
class MockFlutterVersion extends Mock implements FlutterVersion {}
\ No newline at end of file
......@@ -29,6 +29,13 @@ void main() {
expect(config.keys, contains('foo'));
});
test('get set bool value', () async {
expect(config.getValue('foo'), null);
config.setValue('foo', true);
expect(config.getValue('foo'), true);
expect(config.keys, contains('foo'));
});
test('containsKey', () async {
expect(config.containsKey('foo'), false);
config.setValue('foo', 'bar');
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment