From 30692ee4b787998ac5cd0babaccb8b85b7cfcdfa Mon Sep 17 00:00:00 2001
From: Andrew Kolos <andrewrkolos@gmail.com>
Date: Thu, 3 Aug 2023 21:00:12 -0700
Subject: [PATCH] make `--dart-define` override redundant values in
 `--dart-define-from-file` (#131088)

Fixes #130604
---
 .../lib/src/runner/flutter_command.dart       | 16 ++++----
 .../hermetic/assemble_test.dart               |  2 +-
 .../permeable/build_bundle_test.dart          | 38 ++++++++++++++++++-
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart
index 204782e3d40..737def1f3a0 100644
--- a/packages/flutter_tools/lib/src/runner/flutter_command.dart
+++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart
@@ -646,7 +646,8 @@ abstract class FlutterCommand extends Command<void> {
       help:
           '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'
-          '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',
       splitCommas: false,
     );
@@ -1351,14 +1352,14 @@ abstract class FlutterCommand extends Command<void> {
   List<String> extractDartDefines({required Map<String, Object?> defineConfigJsonMap}) {
     final List<String> dartDefines = <String>[];
 
-    if (argParser.options.containsKey(FlutterOptions.kDartDefinesOption)) {
-      dartDefines.addAll(stringsArg(FlutterOptions.kDartDefinesOption));
-    }
-
     defineConfigJsonMap.forEach((String key, Object? value) {
       dartDefines.add('$key=$value');
     });
 
+    if (argParser.options.containsKey(FlutterOptions.kDartDefinesOption)) {
+      dartDefines.addAll(stringsArg(FlutterOptions.kDartDefinesOption));
+    }
+
     return dartDefines;
   }
 
@@ -1372,9 +1373,8 @@ abstract class FlutterCommand extends Command<void> {
 
       for (final String path in configFilePaths) {
         if (!globals.fs.isFileSync(path)) {
-          throwToolExit('Json config define file "--${FlutterOptions
-              .kDartDefineFromFileOption}=$path" is not a file, '
-              'please fix first!');
+          throwToolExit('Did not find the file passed to "--${FlutterOptions
+              .kDartDefineFromFileOption}". Path: $path');
         }
 
         final String configRaw = globals.fs.file(path).readAsStringSync();
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 bf0434b3ca2..09f2c56444f 100644
--- a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart
@@ -302,7 +302,7 @@ void main() {
       '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!'));
+        throwsToolExit(message: 'Did not find the file passed to "--dart-define-from-file". Path: config'));
   }, overrides: <Type, Generator>{
     Cache: () => Cache.test(processManager: FakeProcessManager.any()),
     FileSystem: () => MemoryFileSystem.test(),
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 c4a501897f5..a0d34ace277 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
@@ -559,6 +559,42 @@ void main() {
     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 {
     globals.fs
         .file(globals.fs.path.join('lib', 'main.dart'))
@@ -763,7 +799,7 @@ void main() {
       '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!'));
+    ]), throwsToolExit(message: 'Did not find the file passed to "--dart-define-from-file". Path: config'));
   }, overrides: <Type, Generator>{
     FileSystem: fsFactory,
     BuildSystem: () => TestBuildSystem.all(BuildResult(success: true)),