From acc840e574eb21ebcf33b64bac15eb0bc5c92b04 Mon Sep 17 00:00:00 2001 From: Seiya Kokushi Date: Tue, 7 Mar 2023 03:16:14 +0900 Subject: [PATCH] [tool] Proposal to multiple defines for --dart-define-from-file (#120878) [tool] Proposal to multiple defines for --dart-define-from-file --- .../lib/src/commands/assemble.dart | 20 +--- .../lib/src/runner/flutter_command.dart | 84 +++++++++----- .../hermetic/assemble_test.dart | 20 +++- .../permeable/build_bundle_test.dart | 105 +++++++++++++----- 4 files changed, 156 insertions(+), 73 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/assemble.dart b/packages/flutter_tools/lib/src/commands/assemble.dart index d37e6a2005f..29d96232171 100644 --- a/packages/flutter_tools/lib/src/commands/assemble.dart +++ b/packages/flutter_tools/lib/src/commands/assemble.dart @@ -258,24 +258,8 @@ class AssembleCommand extends FlutterCommand { results[kExtraGenSnapshotOptions] = (argumentResults[FlutterOptions.kExtraGenSnapshotOptions] as List).join(','); } - List dartDefines = []; - if (argumentResults.wasParsed(FlutterOptions.kDartDefinesOption)) { - dartDefines = argumentResults[FlutterOptions.kDartDefinesOption] as List; - } - if (argumentResults.wasParsed(FlutterOptions.kDartDefineFromFileOption)) { - final String? configJsonPath = stringArg(FlutterOptions.kDartDefineFromFileOption); - if (configJsonPath != null && globals.fs.isFileSync(configJsonPath)) { - final String configJsonRaw = globals.fs.file(configJsonPath).readAsStringSync(); - try { - (json.decode(configJsonRaw) as Map).forEach((String key, dynamic value) { - dartDefines.add('$key=$value'); - }); - } on FormatException catch (err) { - throwToolExit('Json config define file "--${FlutterOptions.kDartDefineFromFileOption}=$configJsonPath" format err, ' - 'please fix first! format err:\n$err'); - } - } - } + final Map? defineConfigJsonMap = extractDartDefineConfigJsonMap(); + final List dartDefines = extractDartDefines(defineConfigJsonMap: defineConfigJsonMap); if(dartDefines.isNotEmpty){ results[kDartDefines] = dartDefines.join(','); } diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index b1fac417f24..2182a400e48 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -642,12 +642,14 @@ abstract class FlutterCommand extends Command { } void useDartDefineConfigJsonFileOption() { - argParser.addOption( + argParser.addMultiOption( FlutterOptions.kDartDefineFromFileOption, help: 'The path of a json format file where flutter define a global constant pool. ' 'Json entry will be available as constants from the String.fromEnvironment, bool.fromEnvironment, ' - 'int.fromEnvironment, and double.fromEnvironment constructors; the key and field are json values.', - valueHelp: 'use-define-config.json' + 'int.fromEnvironment, and double.fromEnvironment constructors; the key and field are json values.\n' + 'Multiple defines can be passed by repeating "--${FlutterOptions.kDartDefineFromFileOption}" multiple times.', + valueHelp: 'use-define-config.json', + splitCommas: false, ); } @@ -1185,9 +1187,8 @@ abstract class FlutterCommand extends Command { ? stringArgDeprecated(FlutterOptions.kPerformanceMeasurementFile) : null; - List dartDefines = argParser.options.containsKey(FlutterOptions.kDartDefinesOption) - ? stringsArg(FlutterOptions.kDartDefinesOption) - : []; + final Map? defineConfigJsonMap = extractDartDefineConfigJsonMap(); + List dartDefines = extractDartDefines(defineConfigJsonMap: defineConfigJsonMap); WebRendererMode webRenderer = WebRendererMode.autoDetect; if (argParser.options.containsKey(FlutterOptions.kWebRendererFlag)) { @@ -1198,27 +1199,6 @@ abstract class FlutterCommand extends Command { dartDefines = updateDartDefines(dartDefines, webRenderer); } - Map? defineConfigJsonMap; - if (argParser.options.containsKey(FlutterOptions.kDartDefineFromFileOption)) { - final String? configJsonPath = stringArg(FlutterOptions.kDartDefineFromFileOption); - if (configJsonPath != null && globals.fs.isFileSync(configJsonPath)) { - final String configJsonRaw = globals.fs.file(configJsonPath).readAsStringSync(); - try { - defineConfigJsonMap = {}; - // Fix json convert Object value :type '_InternalLinkedHashMap' is not a subtype of type 'Map' in type cast - (json.decode(configJsonRaw) as Map).forEach((String key, dynamic value) { - defineConfigJsonMap?[key]=value as Object; - }); - defineConfigJsonMap.forEach((String key, Object value) { - dartDefines.add('$key=$value'); - }); - } on FormatException catch (err) { - throwToolExit('Json config define file "--${FlutterOptions.kDartDefineFromFileOption}=$configJsonPath" format err, ' - 'please fix first! format err:\n$err'); - } - } - } - return BuildInfo(buildMode, argParser.options.containsKey('flavor') ? stringArgDeprecated('flavor') @@ -1334,6 +1314,56 @@ abstract class FlutterCommand extends Command { } } + List extractDartDefines({Map? defineConfigJsonMap}) { + final List dartDefines = []; + + if (argParser.options.containsKey(FlutterOptions.kDartDefinesOption)) { + dartDefines.addAll(stringsArg(FlutterOptions.kDartDefinesOption)); + } + + if (defineConfigJsonMap == null) { + return dartDefines; + } + defineConfigJsonMap.forEach((String key, Object value) { + dartDefines.add('$key=$value'); + }); + + return dartDefines; + } + + Map? extractDartDefineConfigJsonMap() { + final Map dartDefineConfigJsonMap = {}; + + if (argParser.options.containsKey(FlutterOptions.kDartDefineFromFileOption)) { + final List configJsonPaths = stringsArg( + FlutterOptions.kDartDefineFromFileOption, + ); + + for (final String path in configJsonPaths) { + if (!globals.fs.isFileSync(path)) { + throwToolExit('Json config define file "--${FlutterOptions + .kDartDefineFromFileOption}=$path" is not a file, ' + 'please fix first!'); + } + + final String configJsonRaw = globals.fs.file(path).readAsStringSync(); + try { + // Fix json convert Object value :type '_InternalLinkedHashMap' is not a subtype of type 'Map' in type cast + (json.decode(configJsonRaw) as Map) + .forEach((String key, dynamic value) { + dartDefineConfigJsonMap[key] = value as Object; + }); + } on FormatException catch (err) { + throwToolExit('Json config define file "--${FlutterOptions + .kDartDefineFromFileOption}=$path" format err, ' + 'please fix first! format err:\n$err'); + } + } + } + + return dartDefineConfigJsonMap; + } + /// Updates dart-defines based on [webRenderer]. @visibleForTesting static List updateDartDefines(List dartDefines, WebRendererMode webRenderer) { diff --git a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart index 22c30a6f780..bf0434b3ca2 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart @@ -291,6 +291,24 @@ void main() { }); }); + testUsingContext('test --dart-define-from-file option with err file format', () { + globals.fs.directory('config').createSync(); + final CommandRunner commandRunner = createTestCommandRunner(AssembleCommand( + buildSystem: TestBuildSystem.all(BuildResult(success: true)), + )); + + expect(commandRunner.run(['assemble', + '-o Output', + 'debug_macos_bundle_flutter_assets', + '--dart-define=k=v', + '--dart-define-from-file=config']), + throwsToolExit(message: 'Json config define file "--dart-define-from-file=config" is not a file, please fix first!')); + }, overrides: { + Cache: () => Cache.test(processManager: FakeProcessManager.any()), + FileSystem: () => MemoryFileSystem.test(), + ProcessManager: () => FakeProcessManager.any(), + }); + testUsingContext('test --dart-define-from-file option with err json format', () async { await globals.fs.file('config.json').writeAsString( ''' @@ -317,6 +335,4 @@ void main() { FileSystem: () => MemoryFileSystem.test(), ProcessManager: () => FakeProcessManager.any(), }); - - } diff --git a/packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart b/packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart index f5f906239af..db99b2b75ef 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/build_bundle_test.dart @@ -4,7 +4,6 @@ import 'package:args/command_runner.dart'; import 'package:file/memory.dart'; -import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/build_info.dart'; @@ -94,15 +93,15 @@ void main() { globals.fs.file('.packages').createSync(recursive: true); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); expect(() => runner.run([ 'bundle', '--no-pub', '--target-platform=windows-x64', - ]), throwsToolExit()); + ]), throwsToolExit(message: 'Windows is not a supported target platform.')); }, overrides: { + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), FileSystem: fsFactory, ProcessManager: () => FakeProcessManager.any(), FeatureFlags: () => TestFeatureFlags(), @@ -114,15 +113,15 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); expect(() => runner.run([ 'bundle', '--no-pub', '--target-platform=linux-x64', - ]), throwsToolExit()); + ]), throwsToolExit(message: 'Linux is not a supported target platform.')); }, overrides: { + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), FileSystem: fsFactory, ProcessManager: () => FakeProcessManager.any(), FeatureFlags: () => TestFeatureFlags(), @@ -134,15 +133,15 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); expect(() => runner.run([ 'bundle', '--no-pub', '--target-platform=darwin', - ]), throwsToolExit()); + ]), throwsToolExit(message: 'macOS is not a supported target platform.')); }, overrides: { + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), FileSystem: fsFactory, ProcessManager: () => FakeProcessManager.any(), FeatureFlags: () => TestFeatureFlags(), @@ -154,7 +153,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); expect(() => runner.run([ @@ -174,7 +172,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -183,6 +180,7 @@ void main() { '--target-platform=windows-x64', ]); }, overrides: { + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), FileSystem: fsFactory, ProcessManager: () => FakeProcessManager.any(), FeatureFlags: () => TestFeatureFlags(isWindowsEnabled: true), @@ -194,7 +192,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -203,6 +200,7 @@ void main() { '--target-platform=linux-x64', ]); }, overrides: { + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), FileSystem: fsFactory, ProcessManager: () => FakeProcessManager.any(), FeatureFlags: () => TestFeatureFlags(isLinuxEnabled: true), @@ -214,7 +212,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -223,6 +220,7 @@ void main() { '--target-platform=darwin', ]); }, overrides: { + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), FileSystem: fsFactory, ProcessManager: () => FakeProcessManager.any(), FeatureFlags: () => TestFeatureFlags(isMacOSEnabled: true), @@ -234,7 +232,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -267,7 +264,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -301,7 +297,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -334,7 +329,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -368,7 +362,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -402,7 +395,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -436,7 +428,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -478,7 +469,6 @@ void main() { globals.fs.file('.packages').createSync(); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ @@ -518,7 +508,7 @@ void main() { globals.fs.file(globals.fs.path.join('lib', 'main.dart')).createSync(recursive: true); globals.fs.file('pubspec.yaml').createSync(); globals.fs.file('.packages').createSync(); - await globals.fs.file('config.json').writeAsString( + await globals.fs.file('config1.json').writeAsString( ''' { "kInt": 1, @@ -528,24 +518,89 @@ void main() { } ''' ); + await globals.fs.file('config2.json').writeAsString( + ''' + { + "body": "this is body from config json file" + } + ''' + ); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); await runner.run([ 'bundle', '--no-pub', - '--dart-define-from-file=config.json', + '--dart-define-from-file=config1.json', + '--dart-define-from-file=config2.json', ]); }, overrides: { BuildSystem: () => TestBuildSystem.all(BuildResult(success: true), (Target target, Environment environment) { - expect(environment.defines[kDartDefines], 'a0ludD0x,a0RvdWJsZT0xLjE=,bmFtZT1kZW5naGFpemh1,dGl0bGU9dGhpcyBpcyB0aXRsZSBmcm9tIGNvbmZpZyBqc29uIGZpbGU='); + expect(environment.defines[kDartDefines], 'a0ludD0x,a0RvdWJsZT0xLjE=,bmFtZT1kZW5naGFpemh1,dGl0bGU9dGhpcyBpcyB0aXRsZSBmcm9tIGNvbmZpZyBqc29uIGZpbGU=,Ym9keT10aGlzIGlzIGJvZHkgZnJvbSBjb25maWcganNvbiBmaWxl'); }), FileSystem: fsFactory, ProcessManager: () => FakeProcessManager.any(), }); + testUsingContext('test --dart-define-from-file option if conflict', () 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(); + await globals.fs.file('config1.json').writeAsString( + ''' + { + "kInt": 1, + "kDouble": 1.1, + "name": "denghaizhu", + "title": "this is title from config json file" + } + ''' + ); + await globals.fs.file('config2.json').writeAsString( + ''' + { + "kInt": "2" + } + ''' + ); + final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( + logger: BufferLogger.test(), + )); + + await runner.run([ + 'bundle', + '--no-pub', + '--dart-define-from-file=config1.json', + '--dart-define-from-file=config2.json', + ]); + }, overrides: { + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true), (Target target, Environment environment) { + expect(environment.defines[kDartDefines], 'a0ludD0y,a0RvdWJsZT0xLjE=,bmFtZT1kZW5naGFpemh1,dGl0bGU9dGhpcyBpcyB0aXRsZSBmcm9tIGNvbmZpZyBqc29uIGZpbGU='); + }), + FileSystem: fsFactory, + ProcessManager: () => FakeProcessManager.any(), + }); + + testUsingContext('test --dart-define-from-file option by invalid file type', () { + 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.directory('config').createSync(); + final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( + logger: BufferLogger.test(), + )); + + expect(() => runner.run([ + 'bundle', + '--no-pub', + '--dart-define-from-file=config', + ]), throwsToolExit(message: 'Json config define file "--dart-define-from-file=config" is not a file, please fix first!')); + }, overrides: { + FileSystem: fsFactory, + BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), + ProcessManager: () => FakeProcessManager.any(), + }); testUsingContext('test --dart-define-from-file option by corrupted json', () async { globals.fs.file(globals.fs.path.join('lib', 'main.dart')).createSync(recursive: true); @@ -563,20 +618,18 @@ void main() { ); final CommandRunner runner = createTestCommandRunner(BuildBundleCommand( logger: BufferLogger.test(), - bundleBuilder: fakeBundleBuilder, )); expect(() => runner.run([ 'bundle', '--no-pub', '--dart-define-from-file=config.json', - ]), throwsA(predicate((Exception e) => e is ToolExit && e.message!.startsWith('Json config define file "--dart-define-from-file=config.json" format err')))); + ]), throwsToolExit(message: 'Json config define file "--dart-define-from-file=config.json" format err')); }, overrides: { FileSystem: fsFactory, BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)), ProcessManager: () => FakeProcessManager.any(), }); - } class FakeBundleBuilder extends Fake implements BundleBuilder {