Unverified Commit 30692ee4 authored by Andrew Kolos's avatar Andrew Kolos Committed by GitHub

make `--dart-define` override redundant values in `--dart-define-from-file` (#131088)

Fixes #130604
parent b2381348
...@@ -646,7 +646,8 @@ abstract class FlutterCommand extends Command<void> { ...@@ -646,7 +646,8 @@ abstract class FlutterCommand extends Command<void> {
help: help:
'The path of a .json or .env file containing key-value pairs that will be available as environment variables.\n' 'The path of a .json or .env file containing key-value pairs that will be available as environment variables.\n'
'These can be accessed using the String.fromEnvironment, bool.fromEnvironment, and int.fromEnvironment constructors.\n' 'These can be accessed using the String.fromEnvironment, bool.fromEnvironment, and int.fromEnvironment constructors.\n'
'Multiple defines can be passed by repeating "--${FlutterOptions.kDartDefineFromFileOption}" multiple times.', 'Multiple defines can be passed by repeating "--${FlutterOptions.kDartDefineFromFileOption}" multiple times.\n'
'Entries from "--${FlutterOptions.kDartDefinesOption}" with identical keys take precedence over entries from these files.',
valueHelp: 'use-define-config.json|.env', valueHelp: 'use-define-config.json|.env',
splitCommas: false, splitCommas: false,
); );
...@@ -1351,14 +1352,14 @@ abstract class FlutterCommand extends Command<void> { ...@@ -1351,14 +1352,14 @@ abstract class FlutterCommand extends Command<void> {
List<String> extractDartDefines({required Map<String, Object?> defineConfigJsonMap}) { List<String> extractDartDefines({required Map<String, Object?> defineConfigJsonMap}) {
final List<String> dartDefines = <String>[]; final List<String> dartDefines = <String>[];
if (argParser.options.containsKey(FlutterOptions.kDartDefinesOption)) {
dartDefines.addAll(stringsArg(FlutterOptions.kDartDefinesOption));
}
defineConfigJsonMap.forEach((String key, Object? value) { defineConfigJsonMap.forEach((String key, Object? value) {
dartDefines.add('$key=$value'); dartDefines.add('$key=$value');
}); });
if (argParser.options.containsKey(FlutterOptions.kDartDefinesOption)) {
dartDefines.addAll(stringsArg(FlutterOptions.kDartDefinesOption));
}
return dartDefines; return dartDefines;
} }
...@@ -1372,9 +1373,8 @@ abstract class FlutterCommand extends Command<void> { ...@@ -1372,9 +1373,8 @@ abstract class FlutterCommand extends Command<void> {
for (final String path in configFilePaths) { for (final String path in configFilePaths) {
if (!globals.fs.isFileSync(path)) { if (!globals.fs.isFileSync(path)) {
throwToolExit('Json config define file "--${FlutterOptions throwToolExit('Did not find the file passed to "--${FlutterOptions
.kDartDefineFromFileOption}=$path" is not a file, ' .kDartDefineFromFileOption}". Path: $path');
'please fix first!');
} }
final String configRaw = globals.fs.file(path).readAsStringSync(); final String configRaw = globals.fs.file(path).readAsStringSync();
......
...@@ -302,7 +302,7 @@ void main() { ...@@ -302,7 +302,7 @@ void main() {
'debug_macos_bundle_flutter_assets', 'debug_macos_bundle_flutter_assets',
'--dart-define=k=v', '--dart-define=k=v',
'--dart-define-from-file=config']), '--dart-define-from-file=config']),
throwsToolExit(message: 'Json config define file "--dart-define-from-file=config" is not a file, please fix first!')); throwsToolExit(message: 'Did not find the file passed to "--dart-define-from-file". Path: config'));
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
Cache: () => Cache.test(processManager: FakeProcessManager.any()), Cache: () => Cache.test(processManager: FakeProcessManager.any()),
FileSystem: () => MemoryFileSystem.test(), FileSystem: () => MemoryFileSystem.test(),
......
...@@ -559,6 +559,42 @@ void main() { ...@@ -559,6 +559,42 @@ void main() {
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
}); });
testUsingContext('values from --dart-define supersede values from --dart-define-from-file', () async {
globals.fs
.file(globals.fs.path.join('lib', 'main.dart'))
.createSync(recursive: true);
globals.fs.file('pubspec.yaml').createSync();
globals.fs.file('.packages').createSync();
globals.fs.file('.env').writeAsStringSync('''
MY_VALUE=VALUE_FROM_ENV_FILE
''');
final CommandRunner<void> runner =
createTestCommandRunner(BuildBundleCommand(
logger: BufferLogger.test(),
));
await runner.run(<String>[
'bundle',
'--no-pub',
'--dart-define=MY_VALUE=VALUE_FROM_COMMAND',
'--dart-define-from-file=.env',
]);
}, overrides: <Type, Generator>{
BuildSystem: () => TestBuildSystem.all(BuildResult(success: true),
(Target target, Environment environment) {
expect(
_decodeDartDefines(environment),
containsAllInOrder(const <String>[
'MY_VALUE=VALUE_FROM_ENV_FILE',
'MY_VALUE=VALUE_FROM_COMMAND',
]),
);
}),
FileSystem: fsFactory,
ProcessManager: () => FakeProcessManager.any(),
});
testUsingContext('--dart-define-from-file correctly parses a valid env file', () async { testUsingContext('--dart-define-from-file correctly parses a valid env file', () async {
globals.fs globals.fs
.file(globals.fs.path.join('lib', 'main.dart')) .file(globals.fs.path.join('lib', 'main.dart'))
...@@ -763,7 +799,7 @@ void main() { ...@@ -763,7 +799,7 @@ void main() {
'bundle', 'bundle',
'--no-pub', '--no-pub',
'--dart-define-from-file=config', '--dart-define-from-file=config',
]), throwsToolExit(message: 'Json config define file "--dart-define-from-file=config" is not a file, please fix first!')); ]), throwsToolExit(message: 'Did not find the file passed to "--dart-define-from-file". Path: config'));
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FileSystem: fsFactory, FileSystem: fsFactory,
BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)),
......
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