Unverified Commit 25bed918 authored by Andrew Kolos's avatar Andrew Kolos Committed by GitHub

do not include entries from `--dart-define-from-file` files in the gradle...

do not include entries from `--dart-define-from-file` files in the gradle config or environment during build (#136865)

Fixes https://github.com/flutter/flutter/issues/130599 and https://github.com/flutter/flutter/issues/136444
parent b5d8d77e
...@@ -39,7 +39,6 @@ class BuildInfo { ...@@ -39,7 +39,6 @@ class BuildInfo {
this.webRenderer = WebRendererMode.auto, this.webRenderer = WebRendererMode.auto,
required this.treeShakeIcons, required this.treeShakeIcons,
this.performanceMeasurementFile, this.performanceMeasurementFile,
this.dartDefineConfigJsonMap = const <String, Object?>{},
this.packagesPath = '.dart_tool/package_config.json', // TODO(zanderso): make this required and remove the default. this.packagesPath = '.dart_tool/package_config.json', // TODO(zanderso): make this required and remove the default.
this.nullSafetyMode = NullSafetyMode.sound, this.nullSafetyMode = NullSafetyMode.sound,
this.codeSizeDirectory, this.codeSizeDirectory,
...@@ -141,17 +140,6 @@ class BuildInfo { ...@@ -141,17 +140,6 @@ class BuildInfo {
/// rerun tasks. /// rerun tasks.
final String? performanceMeasurementFile; final String? performanceMeasurementFile;
/// Configure a constant pool file.
/// Additional constant values to be made available in the Dart program.
///
/// These values can be used with the const `fromEnvironment` constructors of
/// [String] the key and field are json values
/// json value
///
/// An additional field `dartDefineConfigJsonMap` is provided to represent the native JSON value of the configuration file
///
final Map<String, Object?> dartDefineConfigJsonMap;
/// If provided, an output directory where one or more v8-style heap snapshots /// If provided, an output directory where one or more v8-style heap snapshots
/// will be written for code size profiling. /// will be written for code size profiling.
final String? codeSizeDirectory; final String? codeSizeDirectory;
...@@ -277,11 +265,7 @@ class BuildInfo { ...@@ -277,11 +265,7 @@ class BuildInfo {
/// ///
/// Fields that are `null` are excluded from this configuration. /// Fields that are `null` are excluded from this configuration.
Map<String, String> toEnvironmentConfig() { Map<String, String> toEnvironmentConfig() {
final Map<String, String> map = <String, String>{}; return <String, String>{
dartDefineConfigJsonMap.forEach((String key, Object? value) {
map[key] = '$value';
});
final Map<String, String> environmentMap = <String, String>{
if (dartDefines.isNotEmpty) if (dartDefines.isNotEmpty)
'DART_DEFINES': encodeDartDefines(dartDefines), 'DART_DEFINES': encodeDartDefines(dartDefines),
'DART_OBFUSCATION': dartObfuscation.toString(), 'DART_OBFUSCATION': dartObfuscation.toString(),
...@@ -303,23 +287,13 @@ class BuildInfo { ...@@ -303,23 +287,13 @@ class BuildInfo {
if (codeSizeDirectory != null) if (codeSizeDirectory != null)
'CODE_SIZE_DIRECTORY': codeSizeDirectory!, 'CODE_SIZE_DIRECTORY': codeSizeDirectory!,
}; };
map.forEach((String key, String value) {
if (environmentMap.containsKey(key)) {
globals.printWarning(
'The key: [$key] already exists, you cannot use environment variables that have been used by the system!');
} else {
// System priority is greater than user priority
environmentMap[key] = value;
}
});
return environmentMap;
} }
/// Convert this config to a series of project level arguments to be passed /// Convert this config to a series of project level arguments to be passed
/// on the command line to gradle. /// on the command line to gradle.
List<String> toGradleConfig() { List<String> toGradleConfig() {
// PACKAGE_CONFIG not currently supported. // PACKAGE_CONFIG not currently supported.
final List<String> result = <String>[ return <String>[
if (dartDefines.isNotEmpty) if (dartDefines.isNotEmpty)
'-Pdart-defines=${encodeDartDefines(dartDefines)}', '-Pdart-defines=${encodeDartDefines(dartDefines)}',
'-Pdart-obfuscation=$dartObfuscation', '-Pdart-obfuscation=$dartObfuscation',
...@@ -342,16 +316,6 @@ class BuildInfo { ...@@ -342,16 +316,6 @@ class BuildInfo {
for (final String projectArg in androidProjectArgs) for (final String projectArg in androidProjectArgs)
'-P$projectArg', '-P$projectArg',
]; ];
final Iterable<String> gradleConfKeys = result.map((final String gradleConf) => gradleConf.split('=')[0].substring(2));
dartDefineConfigJsonMap.forEach((String key, Object? value) {
if (gradleConfKeys.contains(key)) {
globals.printWarning(
'The key: [$key] already exists, you cannot use gradle variables that have been used by the system!');
} else {
result.add('-P$key=$value');
}
});
return result;
} }
} }
......
...@@ -260,7 +260,7 @@ class AssembleCommand extends FlutterCommand { ...@@ -260,7 +260,7 @@ class AssembleCommand extends FlutterCommand {
final Map<String, Object?> defineConfigJsonMap = extractDartDefineConfigJsonMap(); final Map<String, Object?> defineConfigJsonMap = extractDartDefineConfigJsonMap();
final List<String> dartDefines = extractDartDefines(defineConfigJsonMap: defineConfigJsonMap); final List<String> dartDefines = extractDartDefines(defineConfigJsonMap: defineConfigJsonMap);
if (dartDefines.isNotEmpty){ if (dartDefines.isNotEmpty) {
results[kDartDefines] = dartDefines.join(','); results[kDartDefines] = dartDefines.join(',');
} }
......
...@@ -1304,7 +1304,6 @@ abstract class FlutterCommand extends Command<void> { ...@@ -1304,7 +1304,6 @@ abstract class FlutterCommand extends Command<void> {
dartExperiments: experiments, dartExperiments: experiments,
webRenderer: webRenderer, webRenderer: webRenderer,
performanceMeasurementFile: performanceMeasurementFile, performanceMeasurementFile: performanceMeasurementFile,
dartDefineConfigJsonMap: defineConfigJsonMap,
packagesPath: packagesPath ?? globals.fs.path.absolute('.dart_tool', 'package_config.json'), packagesPath: packagesPath ?? globals.fs.path.absolute('.dart_tool', 'package_config.json'),
nullSafetyMode: nullSafetyMode, nullSafetyMode: nullSafetyMode,
codeSizeDirectory: codeSizeDirectory, codeSizeDirectory: codeSizeDirectory,
......
...@@ -243,7 +243,6 @@ void main() { ...@@ -243,7 +243,6 @@ void main() {
treeShakeIcons: true, treeShakeIcons: true,
trackWidgetCreation: true, trackWidgetCreation: true,
dartDefines: <String>['foo=2', 'bar=2'], dartDefines: <String>['foo=2', 'bar=2'],
dartDefineConfigJsonMap: <String, Object>{'baz': '2'},
dartObfuscation: true, dartObfuscation: true,
splitDebugInfoPath: 'foo/', splitDebugInfoPath: 'foo/',
frontendServerStarterPath: 'foo/bar/frontend_server_starter.dart', frontendServerStarterPath: 'foo/bar/frontend_server_starter.dart',
...@@ -268,7 +267,6 @@ void main() { ...@@ -268,7 +267,6 @@ void main() {
'-Pcode-size-directory=foo/code-size', '-Pcode-size-directory=foo/code-size',
'-Pfoo=bar', '-Pfoo=bar',
'-Pfizz=bazz', '-Pfizz=bazz',
'-Pbaz=2',
]); ]);
}); });
...@@ -297,70 +295,4 @@ void main() { ...@@ -297,70 +295,4 @@ void main() {
kDartDefines: 'MTIzMiw0NTY=,Mg==', kDartDefines: 'MTIzMiw0NTY=,Mg==',
}, kDartDefines), <String>['1232,456', '2']); }, kDartDefines), <String>['1232,456', '2']);
}); });
group('Check repeated buildInfo variables', () {
testUsingContext('toEnvironmentConfig repeated variable', () async {
const BuildInfo buildInfo = BuildInfo(BuildMode.debug, '',
treeShakeIcons: true,
trackWidgetCreation: true,
dartDefines: <String>['foo=2', 'bar=2'],
dartDefineConfigJsonMap: <String, Object>{'DART_DEFINES': 'Define a variable, but it occupies the variable name of the system'},
dartObfuscation: true,
);
buildInfo.toEnvironmentConfig();
expect(testLogger.warningText, contains('The key: [DART_DEFINES] already exists, you cannot use environment variables that have been used by the system'));
});
testUsingContext('toEnvironmentConfig repeated variable with DART_DEFINES not set', () async {
// Simulate operation flutterCommand.getBuildInfo with `dart-define-from-file` set dartDefines
const BuildInfo buildInfo = BuildInfo(BuildMode.debug, '',
treeShakeIcons: true,
dartDefines: <String>['DART_DEFINES=Define a variable, but it occupies the variable name of the system'],
trackWidgetCreation: true,
dartDefineConfigJsonMap: <String, Object>{ 'DART_DEFINES' : 'Define a variable, but it occupies the variable name of the system'},
dartObfuscation: true,
);
buildInfo.toEnvironmentConfig();
expect(testLogger.warningText, contains('The key: [DART_DEFINES] already exists, you cannot use environment variables that have been used by the system'));
});
testUsingContext('toGradleConfig repeated variable', () async {
const BuildInfo buildInfo = BuildInfo(BuildMode.debug, '',
treeShakeIcons: true,
trackWidgetCreation: true,
dartDefines: <String>['foo=2', 'bar=2'],
dartDefineConfigJsonMap: <String, Object>{'dart-defines': 'Define a variable, but it occupies the variable name of the system'},
dartObfuscation: true,
);
buildInfo.toGradleConfig();
expect(testLogger.warningText, contains('The key: [dart-defines] already exists, you cannot use gradle variables that have been used by the system'));
});
testUsingContext('toGradleConfig repeated variable with not set', () async {
// Simulate operation flutterCommand.getBuildInfo with `dart-define-from-file` set dartDefines
const BuildInfo buildInfo = BuildInfo(BuildMode.debug, '',
treeShakeIcons: true,
trackWidgetCreation: true,
dartDefines: <String>['dart-defines=Define a variable, but it occupies the variable name of the system'],
dartDefineConfigJsonMap: <String, Object>{'dart-defines': 'Define a variable, but it occupies the variable name of the system'},
dartObfuscation: true,
);
buildInfo.toGradleConfig();
expect(testLogger.warningText, contains('The key: [dart-defines] already exists, you cannot use gradle variables that have been used by the system'));
});
testUsingContext('toGradleConfig with androidProjectArgs override gradle project variant', () async {
const BuildInfo buildInfo = BuildInfo(BuildMode.debug, '',
treeShakeIcons: true,
trackWidgetCreation: true,
androidProjectArgs: <String>['applicationId=com.google'],
dartDefineConfigJsonMap: <String, Object>{'applicationId': 'override applicationId'},
dartObfuscation: true,
);
buildInfo.toGradleConfig();
expect(testLogger.warningText, contains('The key: [applicationId] already exists, you cannot use gradle variables that have been used by the system'));
});
});
} }
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