From 97fb8c05609ea6f146e29e3d59979b4f7b93f614 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Thu, 30 Sep 2021 23:25:13 -0400 Subject: [PATCH] Fix Dart plugin registrant interaction with 'flutter test' (#90288) Building an application for a desktop platform that transitively included any Dart-based plugins (such as path_provider) broke `flutter test`, because its compilation was overriding the provided main (in this case, the test main) with `generated_main.dart` if it was present. This PR: - Changes the `flutter test` compilation path to update `generated_main.dart`, so that the tests will work, and will include any registered Dart plugins. - Makes using `generated_main.dart` during recompile opt-in, to try to reduce the chance of a similar bug happening with other codepaths in the future. Fixes https://github.com/flutter/flutter/issues/88794 --- .../targets/dart_plugin_registrant.dart | 24 ++++----- packages/flutter_tools/lib/src/compile.dart | 8 ++- packages/flutter_tools/lib/src/devfs.dart | 1 + .../lib/src/flutter_plugins.dart | 2 +- packages/flutter_tools/lib/src/project.dart | 5 ++ packages/flutter_tools/lib/src/run_hot.dart | 1 + .../lib/src/test/test_compiler.dart | 20 ++++++- .../test/general.shard/dart_plugin_test.dart | 25 ++++----- .../test/general.shard/devfs_test.dart | 2 +- .../general.shard/resident_runner_test.dart | 1 + .../resident_web_runner_test.dart | 1 + .../test/test_compiler_test.dart | 54 +++++++++++++++++++ .../general.shard/web/devfs_web_test.dart | 1 + 13 files changed, 111 insertions(+), 34 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart b/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart index 4561c6374a..67d0507ce0 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart @@ -33,27 +33,23 @@ class DartPluginRegistrantTarget extends Target { @override Future build(Environment environment) async { assert(environment.generateDartPluginRegistry); - final File packagesFile = environment.projectDir - .childDirectory('.dart_tool') - .childFile('package_config.json'); + final FlutterProject project = _project + ?? FlutterProject.fromDirectory(environment.projectDir); final PackageConfig packageConfig = await loadPackageConfigWithLogging( - packagesFile, + project.packageConfigFile, logger: environment.logger, ); - final String targetFile = environment.defines[kTargetFile] ?? + final String targetFilePath = environment.defines[kTargetFile] ?? environment.fileSystem.path.join('lib', 'main.dart'); - final File mainFile = environment.fileSystem.file(targetFile); + final File mainFile = environment.fileSystem.file(targetFilePath); final Uri mainFileUri = mainFile.absolute.uri; - final String mainUri = packageConfig.toPackageUri(mainFileUri)?.toString() ?? mainFileUri.toString(); - final File newMainDart = environment.projectDir - .childDirectory('.dart_tool') - .childDirectory('flutter_build') - .childFile('generated_main.dart'); + final String mainFileUriString = packageConfig.toPackageUri(mainFileUri)?.toString() + ?? mainFileUri.toString(); + await generateMainDartWithPluginRegistrant( - _project ?? FlutterProject.fromDirectory(environment.projectDir), + project, packageConfig, - mainUri, - newMainDart, + mainFileUriString, mainFile, throwOnPluginPubspecError: false, ); diff --git a/packages/flutter_tools/lib/src/compile.dart b/packages/flutter_tools/lib/src/compile.dart index a35bdaacfd..97176dfaa2 100644 --- a/packages/flutter_tools/lib/src/compile.dart +++ b/packages/flutter_tools/lib/src/compile.dart @@ -462,6 +462,10 @@ abstract class ResidentCompiler { /// point that is used for recompilation. /// Binary file name is returned if compilation was successful, otherwise /// null is returned. + /// + /// If [checkDartPluginRegistry] is true, it is the caller's responsibility + /// to ensure that the generated registrant file has been updated such that + /// it is wrapping [mainUri]. Future recompile( Uri mainUri, List? invalidatedFiles, { @@ -470,6 +474,7 @@ abstract class ResidentCompiler { required String projectRootPath, required FileSystem fs, bool suppressErrors = false, + bool checkDartPluginRegistry = false, }); Future compileExpression( @@ -610,6 +615,7 @@ class DefaultResidentCompiler implements ResidentCompiler { required String outputPath, required PackageConfig packageConfig, bool suppressErrors = false, + bool checkDartPluginRegistry = false, String? projectRootPath, FileSystem? fs, }) async { @@ -618,7 +624,7 @@ class DefaultResidentCompiler implements ResidentCompiler { _controller.stream.listen(_handleCompilationRequest); } // `generated_main.dart` contains the Dart plugin registry. - if (projectRootPath != null && fs != null) { + if (checkDartPluginRegistry && projectRootPath != null && fs != null) { final File generatedMainDart = fs.file( fs.path.join( projectRootPath, diff --git a/packages/flutter_tools/lib/src/devfs.dart b/packages/flutter_tools/lib/src/devfs.dart index e005aaa2a2..c254f5c350 100644 --- a/packages/flutter_tools/lib/src/devfs.dart +++ b/packages/flutter_tools/lib/src/devfs.dart @@ -613,6 +613,7 @@ class DevFS { fs: _fileSystem, projectRootPath: projectRootPath, packageConfig: packageConfig, + checkDartPluginRegistry: true, // The entry point is assumed not to have changed. ).then((CompilerOutput result) { compileTimer.stop(); return result; diff --git a/packages/flutter_tools/lib/src/flutter_plugins.dart b/packages/flutter_tools/lib/src/flutter_plugins.dart index 32936bbd9f..aca5827bf7 100644 --- a/packages/flutter_tools/lib/src/flutter_plugins.dart +++ b/packages/flutter_tools/lib/src/flutter_plugins.dart @@ -1233,7 +1233,6 @@ Future generateMainDartWithPluginRegistrant( FlutterProject rootProject, PackageConfig packageConfig, String currentMainUri, - File newMainDart, File mainFile, { bool throwOnPluginPubspecError = false, }) async { @@ -1254,6 +1253,7 @@ Future generateMainDartWithPluginRegistrant( MacOSPlugin.kConfigKey: [], WindowsPlugin.kConfigKey: [], }; + final File newMainDart = rootProject.dartPluginRegistrant; if (resolutions.isEmpty) { try { if (newMainDart.existsSync()) { diff --git a/packages/flutter_tools/lib/src/project.dart b/packages/flutter_tools/lib/src/project.dart index 1239f60007..7b4b926c11 100644 --- a/packages/flutter_tools/lib/src/project.dart +++ b/packages/flutter_tools/lib/src/project.dart @@ -219,6 +219,11 @@ class FlutterProject { .childDirectory('generated') .childDirectory(manifest.appName); + /// The generated Dart plugin registrant for non-web platforms. + File get dartPluginRegistrant => dartTool + .childDirectory('flutter_build') + .childFile('generated_main.dart'); + /// The example sub-project of this project. FlutterProject get example => FlutterProject( _exampleDirectory(directory), diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index 840b69ecfc..cf4677f00c 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -381,6 +381,7 @@ class HotRunner extends ResidentRunner { // the native build step. If there is a Dart compilation error, it // should only be displayed once. suppressErrors: applicationBinary == null, + checkDartPluginRegistry: true, outputPath: dillOutputPath ?? getDefaultApplicationKernelPath( trackWidgetCreation: debuggingOptions.buildInfo.trackWidgetCreation, diff --git a/packages/flutter_tools/lib/src/test/test_compiler.dart b/packages/flutter_tools/lib/src/test/test_compiler.dart index 41e95ad610..107d0baad4 100644 --- a/packages/flutter_tools/lib/src/test/test_compiler.dart +++ b/packages/flutter_tools/lib/src/test/test_compiler.dart @@ -13,6 +13,7 @@ import '../base/file_system.dart'; import '../build_info.dart'; import '../bundle.dart'; import '../compile.dart'; +import '../flutter_plugins.dart'; import '../globals_null_migrated.dart' as globals; import '../project.dart'; @@ -143,12 +144,29 @@ class TestCompiler { compiler = await createCompiler(); firstCompile = true; } + + final List invalidatedRegistrantFiles = []; + if (flutterProject != null) { + // Update the generated registrant to use the test target's main. + final String mainUriString = buildInfo.packageConfig.toPackageUri(request.mainUri)?.toString() + ?? request.mainUri.toString(); + await generateMainDartWithPluginRegistrant( + flutterProject, + buildInfo.packageConfig, + mainUriString, + globals.fs.file(request.mainUri), + throwOnPluginPubspecError: false, + ); + invalidatedRegistrantFiles.add(flutterProject.dartPluginRegistrant.absolute.uri); + } + final CompilerOutput compilerOutput = await compiler.recompile( request.mainUri, - [request.mainUri], + [request.mainUri, ...invalidatedRegistrantFiles], outputPath: outputDill.path, packageConfig: buildInfo.packageConfig, projectRootPath: flutterProject?.directory?.absolute?.path, + checkDartPluginRegistry: true, fs: globals.fs, ); final String outputPath = compilerOutput?.outputFilename; diff --git a/packages/flutter_tools/test/general.shard/dart_plugin_test.dart b/packages/flutter_tools/test/general.shard/dart_plugin_test.dart index f2c38b7dea..cafabc5022 100644 --- a/packages/flutter_tools/test/general.shard/dart_plugin_test.dart +++ b/packages/flutter_tools/test/general.shard/dart_plugin_test.dart @@ -33,7 +33,8 @@ void main() { ..manifest = flutterManifest ..directory = directory ..flutterPluginsFile = directory.childFile('.flutter-plugins') - ..flutterPluginsDependenciesFile = directory.childFile('.flutter-plugins-dependencies'); + ..flutterPluginsDependenciesFile = directory.childFile('.flutter-plugins-dependencies') + ..dartPluginRegistrant = directory.childFile('generated_main.dart'); flutterProject.directory.childFile('.packages').createSync(recursive: true); }); @@ -610,7 +611,6 @@ void main() { void main() { } '''); - final File generatedMainFile = flutterProject.directory.childFile('generated_main.dart'); final PackageConfig packageConfig = await loadPackageConfigWithLogging( flutterProject.directory.childDirectory('.dart_tool').childFile('package_config.json'), logger: globals.logger, @@ -620,11 +620,10 @@ void main() { flutterProject, packageConfig, 'package:app/main.dart', - generatedMainFile, mainFile, throwOnPluginPubspecError: true, ); - expect(generatedMainFile.readAsStringSync(), + expect(flutterProject.dartPluginRegistrant.readAsStringSync(), '//\n' '// Generated file. Do not edit.\n' '// This file is generated from template in file `flutter_tools/lib/src/flutter_plugins.dart`.\n' @@ -730,7 +729,6 @@ void main() { libDir.createSync(recursive: true); final File mainFile = libDir.childFile('main.dart')..writeAsStringSync(''); - final File generatedMainFile = flutterProject.directory.childFile('generated_main.dart'); final PackageConfig packageConfig = await loadPackageConfigWithLogging( flutterProject.directory.childDirectory('.dart_tool').childFile('package_config.json'), logger: globals.logger, @@ -741,7 +739,6 @@ void main() { flutterProject, packageConfig, 'package:app/main.dart', - generatedMainFile, mainFile, throwOnPluginPubspecError: true, ), throwsToolExit(message: @@ -773,7 +770,6 @@ void main() { libDir.createSync(recursive: true); final File mainFile = libDir.childFile('main.dart')..writeAsStringSync(''); - final File generatedMainFile = flutterProject.directory.childFile('generated_main.dart'); final PackageConfig packageConfig = await loadPackageConfigWithLogging( flutterProject.directory.childDirectory('.dart_tool').childFile('package_config.json'), logger: globals.logger, @@ -784,7 +780,6 @@ void main() { flutterProject, packageConfig, 'package:app/main.dart', - generatedMainFile, mainFile, throwOnPluginPubspecError: true, ), throwsToolExit(message: @@ -834,7 +829,6 @@ void main() { libDir.createSync(recursive: true); final File mainFile = libDir.childFile('main.dart')..writeAsStringSync(''); - final File generatedMainFile = flutterProject.directory.childFile('generated_main.dart'); final PackageConfig packageConfig = await loadPackageConfigWithLogging( flutterProject.directory.childDirectory('.dart_tool').childFile('package_config.json'), logger: globals.logger, @@ -844,11 +838,10 @@ void main() { flutterProject, packageConfig, 'package:app/main.dart', - generatedMainFile, mainFile, throwOnPluginPubspecError: true, ); - expect(generatedMainFile.existsSync(), isFalse); + expect(flutterProject.dartPluginRegistrant.existsSync(), isFalse); }, overrides: { FileSystem: () => fs, ProcessManager: () => FakeProcessManager.any(), @@ -876,7 +869,6 @@ void main() { libDir.createSync(recursive: true); final File mainFile = libDir.childFile('main.dart')..writeAsStringSync(''); - final File generatedMainFile = flutterProject.directory.childFile('generated_main.dart'); final PackageConfig packageConfig = await loadPackageConfigWithLogging( flutterProject.directory.childDirectory('.dart_tool').childFile('package_config.json'), logger: globals.logger, @@ -886,11 +878,10 @@ void main() { flutterProject, packageConfig, 'package:app/main.dart', - generatedMainFile, mainFile, throwOnPluginPubspecError: true, ); - expect(generatedMainFile.existsSync(), isTrue); + expect(flutterProject.dartPluginRegistrant.existsSync(), isTrue); // No plugins. createFakeDartPlugins( @@ -903,11 +894,10 @@ void main() { flutterProject, packageConfig, 'package:app/main.dart', - generatedMainFile, mainFile, throwOnPluginPubspecError: true, ); - expect(generatedMainFile.existsSync(), isFalse); + expect(flutterProject.dartPluginRegistrant.existsSync(), isFalse); }, overrides: { FileSystem: () => fs, ProcessManager: () => FakeProcessManager.any(), @@ -962,6 +952,9 @@ class FakeFlutterProject extends Fake implements FlutterProject { @override File flutterPluginsDependenciesFile; + @override + File dartPluginRegistrant; + @override IosProject ios; diff --git a/packages/flutter_tools/test/general.shard/devfs_test.dart b/packages/flutter_tools/test/general.shard/devfs_test.dart index 22024a236c..ddc38be05d 100644 --- a/packages/flutter_tools/test/general.shard/devfs_test.dart +++ b/packages/flutter_tools/test/general.shard/devfs_test.dart @@ -476,7 +476,7 @@ class FakeResidentCompiler extends Fake implements ResidentCompiler { Future Function(Uri mainUri, List invalidatedFiles) onRecompile; @override - Future recompile(Uri mainUri, List invalidatedFiles, {String outputPath, PackageConfig packageConfig, String projectRootPath, FileSystem fs, bool suppressErrors = false}) { + Future recompile(Uri mainUri, List invalidatedFiles, {String outputPath, PackageConfig packageConfig, String projectRootPath, FileSystem fs, bool suppressErrors = false, bool checkDartPluginRegistry = false}) { return onRecompile?.call(mainUri, invalidatedFiles) ?? Future.value(const CompilerOutput('', 1, [])); } diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart index aec0aab159..dda956265b 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_test.dart @@ -2191,6 +2191,7 @@ class FakeResidentCompiler extends Fake implements ResidentCompiler { @required String projectRootPath, @required FileSystem fs, bool suppressErrors = false, + bool checkDartPluginRegistry = false, }) async { didSuppressErrors = suppressErrors; return nextOutput ?? const CompilerOutput('foo.dill', 0, []); diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart index 21cf19dd40..b732d607d3 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart @@ -1115,6 +1115,7 @@ class FakeResidentCompiler extends Fake implements ResidentCompiler { @required String projectRootPath, @required FileSystem fs, bool suppressErrors = false, + bool checkDartPluginRegistry = false, }) async { return const CompilerOutput('foo.dill', 0, []); } diff --git a/packages/flutter_tools/test/general.shard/test/test_compiler_test.dart b/packages/flutter_tools/test/general.shard/test/test_compiler_test.dart index 715b367a56..5eebf9611f 100644 --- a/packages/flutter_tools/test/general.shard/test/test_compiler_test.dart +++ b/packages/flutter_tools/test/general.shard/test/test_compiler_test.dart @@ -5,6 +5,7 @@ // @dart = 2.8 import 'package:file/memory.dart'; +import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; @@ -40,6 +41,7 @@ void main() { fileSystem = MemoryFileSystem.test(); fileSystem.file('pubspec.yaml').createSync(); fileSystem.file('test/foo.dart').createSync(recursive: true); + fileSystem.file('.packages').createSync(); residentCompiler = FakeResidentCompiler(fileSystem); }); @@ -113,6 +115,57 @@ void main() { ProcessManager: () => FakeProcessManager.any(), Logger: () => BufferLogger.test(), }); + + testUsingContext('TestCompiler updates generated_main.dart', () async { + final Directory fakeDartPlugin = fileSystem.directory('a_plugin'); + fileSystem.file('pubspec.yaml').writeAsStringSync(''' +name: foo +dependencies: + flutter: + sdk: flutter + a_plugin: 1.0.0 +'''); + fileSystem.file('.packages').writeAsStringSync('a_plugin:/a_plugin/lib/'); + fakeDartPlugin.childFile('pubspec.yaml') + ..createSync(recursive: true) + ..writeAsStringSync(''' +name: a_plugin +flutter: + plugin: + implements: a + platforms: + linux: + dartPluginClass: APlugin +environment: + sdk: ">=2.14.0 <3.0.0" + flutter: ">=2.5.0" +'''); + + residentCompiler.compilerOutput = const CompilerOutput('abc.dill', 0, []); + final FakeTestCompiler testCompiler = FakeTestCompiler( + debugBuild, + FlutterProject.fromDirectoryTest(fileSystem.currentDirectory), + residentCompiler, + ); + + await testCompiler.compile(Uri.parse('test/foo.dart')); + + final File generatedMain = fileSystem + .directory('.dart_tool') + .childDirectory('flutter_build') + .childFile('generated_main.dart'); + + expect(generatedMain, exists); + expect( + generatedMain.readAsLinesSync(), + contains("import 'test/foo.dart' as entrypoint;") + ); + }, overrides: { + FileSystem: () => fileSystem, + Platform: () => linuxPlatform, + ProcessManager: () => FakeProcessManager.any(), + Logger: () => BufferLogger.test(), + }); } /// Override the creation of the Resident Compiler to simplify testing. @@ -150,6 +203,7 @@ class FakeResidentCompiler extends Fake implements ResidentCompiler { String projectRootPath, FileSystem fs, bool suppressErrors = false, + bool checkDartPluginRegistry = false, }) async { if (compilerOutput != null) { fileSystem.file(compilerOutput.outputFilename).createSync(recursive: true); diff --git a/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart b/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart index 52d58029b9..6e5475e230 100644 --- a/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart +++ b/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart @@ -1077,6 +1077,7 @@ class FakeResidentCompiler extends Fake implements ResidentCompiler { String projectRootPath, FileSystem fs, bool suppressErrors = false, + bool checkDartPluginRegistry = false, }) async { return output; }