[tool] Use 'flutter pub get' for Flutter packages (#4397)

stuartmorgan-g · web-flow · commit 12ec9fe3f0aa · 2023-07-07T20:22:19.000Z
Extracts common logic for running `pub get`, and switches commands to use it. The common logic always uses `flutter pub get` for Flutter packages, rather than `dart pub get`, since the latter will fail if someone has a non-Flutter `dart` in their path before `flutter` (e.g., Dart team members contributing PRs).
diff --git a/script/tool/lib/src/common/pub_utils.dart b/script/tool/lib/src/common/pub_utils.dart
@@ -0,0 +1,44 @@
+// Copyright 2013 The Flutter 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 'dart:io' as io;
+
+import 'package:platform/platform.dart';
+
+import 'process_runner.dart';
+import 'repository_package.dart';
+
+/// Runs either `dart pub get` or `flutter pub get` in [package], depending on
+/// the package type.
+///
+/// If [alwaysUseFlutter] is true, it will use `flutter pub get` regardless.
+/// This can be useful, for instance, to get the `flutter`-default behavior
+/// of fetching example packages as well.
+///
+/// If [streamOutput] is false, output will only be printed if the command
+/// fails.
+Future<bool> runPubGet(
+    RepositoryPackage package, ProcessRunner processRunner, Platform platform,
+    {bool alwaysUseFlutter = false, bool streamOutput = true}) async {
+  // Running `dart pub get` on a Flutter package can fail if a non-Flutter Dart
+  // is first in the path, so use `flutter pub get` for any Flutter package.
+  final bool useFlutter = alwaysUseFlutter || package.requiresFlutter();
+  final String command =
+      useFlutter ? (platform.isWindows ? 'flutter.bat' : 'flutter') : 'dart';
+  final List<String> args = <String>['pub', 'get'];
+
+  final int exitCode;
+  if (streamOutput) {
+    exitCode = await processRunner.runAndStream(command, args,
+        workingDir: package.directory);
+  } else {
+    final io.ProcessResult result =
+        await processRunner.run(command, args, workingDir: package.directory);
+    exitCode = result.exitCode;
+    if (exitCode != 0) {
+      print('${result.stdout}\n${result.stderr}\n');
+    }
+  }
+  return exitCode == 0;
+}
diff --git a/script/tool/lib/src/create_all_packages_app_command.dart b/script/tool/lib/src/create_all_packages_app_command.dart
@@ -13,6 +13,7 @@ import 'common/core.dart';
 import 'common/file_utils.dart';
 import 'common/package_command.dart';
 import 'common/process_runner.dart';
+import 'common/pub_utils.dart';
 import 'common/repository_package.dart';
 
 /// The name of the build-all-packages project, as passed to `flutter create`.
@@ -96,7 +97,7 @@ class CreateAllPackagesAppCommand extends PackageCommand {
     // further and/or implement https://github.com/flutter/flutter/issues/93407,
     // and remove the need for this conditional.
     if (!platform.isWindows) {
-      if (!await _genNativeBuildFiles()) {
+      if (!await runPubGet(app, processRunner, platform)) {
         printError(
             "Failed to generate native build files via 'flutter pub get'");
         throw ToolExit(_exitGenNativeBuildFilesFailed);
@@ -371,15 +372,6 @@ dev_dependencies:${_pubspecMapString(pubspec.devDependencies)}
     return buffer.toString();
   }
 
-  Future<bool> _genNativeBuildFiles() async {
-    final int exitCode = await processRunner.runAndStream(
-      flutterCommand,
-      <String>['pub', 'get'],
-      workingDir: _appDirectory,
-    );
-    return exitCode == 0;
-  }
-
   Future<void> _updateMacosPodfile() async {
     /// Only change the macOS deployment target if the host platform is macOS.
     /// The Podfile is not generated on other platforms.
diff --git a/script/tool/lib/src/custom_test_command.dart b/script/tool/lib/src/custom_test_command.dart
@@ -7,6 +7,7 @@ import 'package:platform/platform.dart';
 
 import 'common/package_looping_command.dart';
 import 'common/process_runner.dart';
+import 'common/pub_utils.dart';
 import 'common/repository_package.dart';
 
 const String _scriptName = 'run_tests.dart';
@@ -47,10 +48,7 @@ class CustomTestCommand extends PackageLoopingCommand {
     // Run the custom Dart script if presest.
     if (script.existsSync()) {
       // Ensure that dependencies are available.
-      final int pubGetExitCode = await processRunner.runAndStream(
-          'dart', <String>['pub', 'get'],
-          workingDir: package.directory);
-      if (pubGetExitCode != 0) {
+      if (!await runPubGet(package, processRunner, platform)) {
         return PackageResult.fail(
             <String>['Unable to get script dependencies']);
       }
diff --git a/script/tool/lib/src/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart
@@ -9,6 +9,7 @@ import 'common/core.dart';
 import 'common/package_looping_command.dart';
 import 'common/plugin_utils.dart';
 import 'common/process_runner.dart';
+import 'common/pub_utils.dart';
 import 'common/repository_package.dart';
 
 /// A command to run Dart unit tests for packages.
@@ -130,21 +131,16 @@ class DartTestCommand extends PackageLoopingCommand {
   /// Runs the Dart tests for a non-Flutter package, returning true on success.
   Future<bool> _runDartTests(RepositoryPackage package,
       {String? platform}) async {
-    // Unlike `flutter test`, `pub run test` does not automatically get
+    // Unlike `flutter test`, `dart run test` does not automatically get
     // packages
-    int exitCode = await processRunner.runAndStream(
-      'dart',
-      <String>['pub', 'get'],
-      workingDir: package.directory,
-    );
-    if (exitCode != 0) {
+    if (!await runPubGet(package, processRunner, super.platform)) {
       printError('Unable to fetch dependencies.');
       return false;
     }
 
     final String experiment = getStringArg(kEnableExperiment);
 
-    exitCode = await processRunner.runAndStream(
+    final int exitCode = await processRunner.runAndStream(
       'dart',
       <String>[
         'run',
diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart
@@ -14,6 +14,7 @@ import 'package:pub_semver/pub_semver.dart';
 import 'common/core.dart';
 import 'common/package_looping_command.dart';
 import 'common/process_runner.dart';
+import 'common/pub_utils.dart';
 import 'common/pub_version_finder.dart';
 import 'common/repository_package.dart';
 
@@ -136,11 +137,7 @@ class PublishCheckCommand extends PackageLoopingCommand {
   // Run `dart pub get` on the examples of [package].
   Future<void> _fetchExampleDeps(RepositoryPackage package) async {
     for (final RepositoryPackage example in package.getExamples()) {
-      await processRunner.runAndStream(
-        'dart',
-        <String>['pub', 'get'],
-        workingDir: example.directory,
-      );
+      await runPubGet(example, processRunner, platform);
     }
   }
 
diff --git a/script/tool/lib/src/update_dependency_command.dart b/script/tool/lib/src/update_dependency_command.dart
@@ -13,6 +13,7 @@ import 'package:yaml_edit/yaml_edit.dart';
 import 'common/core.dart';
 import 'common/package_looping_command.dart';
 import 'common/process_runner.dart';
+import 'common/pub_utils.dart';
 import 'common/pub_version_finder.dart';
 import 'common/repository_package.dart';
 
@@ -239,11 +240,9 @@ ${response.httpResponse.body}
     }
 
     print('${indentation}Running pub get...');
-    final io.ProcessResult getResult = await processRunner
-        .run('dart', <String>['pub', 'get'], workingDir: package.directory);
-    if (getResult.exitCode != 0) {
-      printError('dart pub get failed (${getResult.exitCode}):\n'
-          '${getResult.stdout}\n${getResult.stderr}\n');
+    if (!await runPubGet(package, processRunner, platform,
+        streamOutput: false)) {
+      printError('${indentation}Fetching dependencies failed');
       return false;
     }
 
@@ -273,11 +272,9 @@ ${response.httpResponse.body}
     }
 
     print('${indentation}Running pub get...');
-    final io.ProcessResult getResult = await processRunner
-        .run('dart', <String>['pub', 'get'], workingDir: package.directory);
-    if (getResult.exitCode != 0) {
-      printError('dart pub get failed (${getResult.exitCode}):\n'
-          '${getResult.stdout}\n${getResult.stderr}\n');
+    if (!await runPubGet(package, processRunner, platform,
+        streamOutput: false)) {
+      printError('${indentation}Fetching dependencies failed');
       return false;
     }
 
diff --git a/script/tool/lib/src/update_excerpts_command.dart b/script/tool/lib/src/update_excerpts_command.dart
@@ -14,6 +14,7 @@ import 'package:yaml_edit/yaml_edit.dart';
 import 'common/core.dart';
 import 'common/package_looping_command.dart';
 import 'common/process_runner.dart';
+import 'common/pub_utils.dart';
 import 'common/repository_package.dart';
 
 /// A command to update .md code excerpts from code files.
@@ -81,10 +82,7 @@ class UpdateExcerptsCommand extends PackageLoopingCommand {
 
       try {
         // Ensure that dependencies are available.
-        final int pubGetExitCode = await processRunner.runAndStream(
-            'dart', <String>['pub', 'get'],
-            workingDir: example.directory);
-        if (pubGetExitCode != 0) {
+        if (!await runPubGet(example, processRunner, platform)) {
           return PackageResult.fail(
               <String>['Unable to get script dependencies']);
         }
diff --git a/script/tool/test/common/pub_utils_test.dart b/script/tool/test/common/pub_utils_test.dart
@@ -0,0 +1,104 @@
+// Copyright 2013 The Flutter 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:file/file.dart';
+import 'package:file/memory.dart';
+import 'package:flutter_plugin_tools/src/common/pub_utils.dart';
+import 'package:test/test.dart';
+
+import '../mocks.dart';
+import '../util.dart';
+
+void main() {
+  late FileSystem fileSystem;
+  late Directory packagesDir;
+  late RecordingProcessRunner processRunner;
+
+  setUp(() {
+    fileSystem = MemoryFileSystem();
+    packagesDir = createPackagesDirectory(fileSystem: fileSystem);
+    processRunner = RecordingProcessRunner();
+  });
+
+  test('runs with Dart for a non-Flutter package by default', () async {
+    final RepositoryPackage package =
+        createFakePackage('a_package', packagesDir);
+    final MockPlatform platform = MockPlatform();
+
+    await runPubGet(package, processRunner, platform);
+
+    expect(
+        processRunner.recordedCalls,
+        orderedEquals(<ProcessCall>[
+          ProcessCall('dart', const <String>['pub', 'get'], package.path),
+        ]));
+  });
+
+  test('runs with Flutter for a Flutter package by default', () async {
+    final RepositoryPackage package =
+        createFakePackage('a_package', packagesDir, isFlutter: true);
+    final MockPlatform platform = MockPlatform();
+
+    await runPubGet(package, processRunner, platform);
+
+    expect(
+        processRunner.recordedCalls,
+        orderedEquals(<ProcessCall>[
+          ProcessCall('flutter', const <String>['pub', 'get'], package.path),
+        ]));
+  });
+
+  test('runs with Flutter for a Dart package when requested', () async {
+    final RepositoryPackage package =
+        createFakePackage('a_package', packagesDir);
+    final MockPlatform platform = MockPlatform();
+
+    await runPubGet(package, processRunner, platform, alwaysUseFlutter: true);
+
+    expect(
+        processRunner.recordedCalls,
+        orderedEquals(<ProcessCall>[
+          ProcessCall('flutter', const <String>['pub', 'get'], package.path),
+        ]));
+  });
+
+  test('uses the correct Flutter command on Windows', () async {
+    final RepositoryPackage package =
+        createFakePackage('a_package', packagesDir, isFlutter: true);
+    final MockPlatform platform = MockPlatform(isWindows: true);
+
+    await runPubGet(package, processRunner, platform);
+
+    expect(
+        processRunner.recordedCalls,
+        orderedEquals(<ProcessCall>[
+          ProcessCall(
+              'flutter.bat', const <String>['pub', 'get'], package.path),
+        ]));
+  });
+
+  test('reports success', () async {
+    final RepositoryPackage package =
+        createFakePackage('a_package', packagesDir);
+    final MockPlatform platform = MockPlatform();
+
+    final bool result = await runPubGet(package, processRunner, platform);
+
+    expect(result, true);
+  });
+
+  test('reports failure', () async {
+    final RepositoryPackage package =
+        createFakePackage('a_package', packagesDir);
+    final MockPlatform platform = MockPlatform();
+
+    processRunner.mockProcessesForExecutable['dart'] = <FakeProcessInfo>[
+      FakeProcessInfo(MockProcess(exitCode: 1), <String>['pub', 'get'])
+    ];
+
+    final bool result = await runPubGet(package, processRunner, platform);
+
+    expect(result, false);
+  });
+}
diff --git a/script/tool/test/publish_check_command_test.dart b/script/tool/test/publish_check_command_test.dart
@@ -88,7 +88,7 @@ void main() {
       final Iterable<ProcessCall> pubGetCalls =
           plugin1.getExamples().map((RepositoryPackage example) {
         return ProcessCall(
-          'dart',
+          getFlutterCommand(mockPlatform),
           const <String>['pub', 'get'],
           example.path,
         );
@@ -117,6 +117,7 @@ void main() {
       createFakePlugin('plugin_tools_test_package_a', packagesDir);
 
       processRunner.mockProcessesForExecutable['flutter'] = <FakeProcessInfo>[
+        FakeProcessInfo(MockProcess(), <String>['pub', 'get']),
         FakeProcessInfo(MockProcess(exitCode: 1, stdout: 'Some error from pub'),
             <String>['pub', 'publish'])
       ];
@@ -205,7 +206,8 @@ void main() {
               'Packages with an SDK constraint on a pre-release of the Dart '
               'SDK should themselves be published as a pre-release version.');
       processRunner.mockProcessesForExecutable['flutter'] = <FakeProcessInfo>[
-        FakeProcessInfo(process, <String>['pub', 'publish'])
+        FakeProcessInfo(MockProcess(), <String>['pub', 'get']),
+        FakeProcessInfo(process, <String>['pub', 'publish']),
       ];
 
       expect(
@@ -223,7 +225,8 @@ void main() {
               'Packages with an SDK constraint on a pre-release of the Dart '
               'SDK should themselves be published as a pre-release version.');
       processRunner.mockProcessesForExecutable['flutter'] = <FakeProcessInfo>[
-        FakeProcessInfo(process, <String>['pub', 'publish'])
+        FakeProcessInfo(MockProcess(), <String>['pub', 'get']),
+        FakeProcessInfo(process, <String>['pub', 'publish']),
       ];
 
       Error? commandError;
@@ -247,6 +250,7 @@ void main() {
       createFakePlugin('d', packagesDir);
 
       processRunner.mockProcessesForExecutable['flutter'] = <FakeProcessInfo>[
+        FakeProcessInfo(MockProcess(), <String>['pub', 'get']),
         FakeProcessInfo(MockProcess(stdout: 'Package has 0 warnings.'),
             <String>['pub', 'publish']),
       ];
diff --git a/script/tool/test/update_dependency_command_test.dart b/script/tool/test/update_dependency_command_test.dart
@@ -429,7 +429,7 @@ dev_dependencies:
       expect(
         output,
         containsAllInOrder(<Matcher>[
-          contains('dart pub get failed'),
+          contains('Fetching dependencies failed'),
           contains('Failed to update pigeon files'),
         ]),
       );
@@ -545,7 +545,7 @@ dev_dependencies:
       expect(
         output,
         containsAllInOrder(<Matcher>[
-          contains('dart pub get failed'),
+          contains('Fetching dependencies failed'),
           contains('Failed to update mocks'),
         ]),
       );
diff --git a/script/tool/test/update_excerpts_command_test.dart b/script/tool/test/update_excerpts_command_test.dart
