diff --git a/packages/flutter_tools/lib/src/flutter_plugins.dart b/packages/flutter_tools/lib/src/flutter_plugins.dart index ad5b9554d2..6d4c0eb057 100644 --- a/packages/flutter_tools/lib/src/flutter_plugins.dart +++ b/packages/flutter_tools/lib/src/flutter_plugins.dart @@ -234,26 +234,79 @@ bool _writeFlutterPluginsList( 'macos': swiftPackageManagerEnabledMacos, }; - // Only notify if the plugins list has changed. [date_created] will always be different, - // [version] is not relevant for this check. - final String? oldPluginsFileStringContent = _readFileContent(pluginsFile); - bool pluginsChanged = true; - if (oldPluginsFileStringContent != null) { - try { - final Object? decodedJson = jsonDecode(oldPluginsFileStringContent); - if (decodedJson is Map) { - final String jsonOfNewPluginsMap = jsonEncode(pluginsMap); - final String jsonOfOldPluginsMap = jsonEncode(decodedJson[_kFlutterPluginsPluginListKey]); - pluginsChanged = jsonOfNewPluginsMap != jsonOfOldPluginsMap; - } - } on FormatException catch (_) {} + // Only write the plugins file if its content have changed. This ensures + // that flutter assemble targets that depend on this file aren't invalidated + // unnecessarily. + final (:bool pluginsChanged, :bool contentsChanged) = _detectFlutterPluginsListChanges( + pluginsFile, + result, + ); + if (pluginsChanged || contentsChanged) { + final String pluginFileContent = json.encode(result); + pluginsFile.writeAsStringSync(pluginFileContent, flush: true); } - final String pluginFileContent = json.encode(result); - pluginsFile.writeAsStringSync(pluginFileContent, flush: true); return pluginsChanged; } +/// Find what has changed in the .flutter-plugins-dependencies JSON file. +/// +/// [pluginsChanged] is [true] if anything changed in the 'plugins' property. +/// This indicates that platform-specific tooling like 'pod install' should be +/// re-run. +/// +/// [contentsChanged] is [true] if [newJson] has changes that should be saved to +/// disk. +({bool pluginsChanged, bool contentsChanged}) _detectFlutterPluginsListChanges( + File pluginsFile, + Map newJson, +) { + final String? oldPluginsFileStringContent = _readFileContent(pluginsFile); + if (oldPluginsFileStringContent == null) { + return (pluginsChanged: true, contentsChanged: true); + } + + try { + final Object? oldJson = jsonDecode(oldPluginsFileStringContent); + if (oldJson is! Map) { + return (pluginsChanged: true, contentsChanged: true); + } + + // Check if plugins changed. + final String jsonOfNewPluginsMap = jsonEncode(newJson[_kFlutterPluginsPluginListKey]); + final String jsonOfOldPluginsMap = jsonEncode(oldJson[_kFlutterPluginsPluginListKey]); + if (jsonOfNewPluginsMap != jsonOfOldPluginsMap) { + return (pluginsChanged: true, contentsChanged: true); + } + + // Create a copy of the new JSON so that the input isn't mutated when we + // drop properties that should be ignored. + final Map newJsonCopy = {...newJson}; + + // The 'info' property is a comment that doesn't affect functionality. + // The 'dependencyGraph' property is deprecated and shouldn't be used. + // The 'date_created' property is always updated. + // The 'plugins' property has already been checked by the logic above. + const List ignoredKeys = [ + 'info', + 'dependencyGraph', + 'date_created', + _kFlutterPluginsPluginListKey, + ]; + for (final String key in ignoredKeys) { + oldJson.remove(key); + newJsonCopy.remove(key); + } + + final String old = jsonEncode(oldJson); + final String updated = jsonEncode(newJsonCopy); + + return (pluginsChanged: false, contentsChanged: old != updated); + } on FormatException catch (_) { + return (pluginsChanged: true, contentsChanged: true); + } +} + /// Creates a map representation of the [plugins] for those supported by [platformKey]. /// All given [plugins] must provide an implementation for the [platformKey]. List> _createPluginMapOfPlatform(List plugins, String platformKey) { diff --git a/packages/flutter_tools/test/general.shard/plugins_test.dart b/packages/flutter_tools/test/general.shard/plugins_test.dart index b3669ec394..f46164129a 100644 --- a/packages/flutter_tools/test/general.shard/plugins_test.dart +++ b/packages/flutter_tools/test/general.shard/plugins_test.dart @@ -409,7 +409,7 @@ dependencies: () async { await refreshPluginsList(flutterProject); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), false); + expect(flutterProject.flutterPluginsDependenciesFile, isNot(exists)); }, overrides: { FileSystem: () => fs, @@ -426,7 +426,7 @@ dependencies: await refreshPluginsList(flutterProject); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), false); + expect(flutterProject.flutterPluginsDependenciesFile, isNot(exists)); }, overrides: { FileSystem: () => fs, @@ -450,12 +450,8 @@ dependencies: await refreshPluginsList(flutterProject); - expect( - flutterProject.flutterPluginsFile.existsSync(), - false, - reason: 'No longer emitted', - ); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), true); + expect(flutterProject.flutterPluginsFile, isNot(exists), reason: 'No longer emitted'); + expect(flutterProject.flutterPluginsDependenciesFile, exists); final String pluginsFileContents = flutterProject.flutterPluginsDependenciesFile.readAsStringSync(); @@ -527,8 +523,8 @@ dependencies: await refreshPluginsList(flutterProject); // Verify .flutter-plugins-dependencies is configured correctly. - expect(flutterProject.flutterPluginsFile.existsSync(), true); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), true); + expect(flutterProject.flutterPluginsFile, exists); + expect(flutterProject.flutterPluginsDependenciesFile, exists); expect( flutterProject.flutterPluginsFile.readAsStringSync(), '# This is a generated file; do not edit or check into version control.\n' @@ -619,6 +615,54 @@ dependencies: }, ); + testUsingContext( + 'Refreshing the plugin list updates .flutter-plugins-dependencies if the plugins changed', + () async { + // Refresh the plugin list (we have no plugins). + await refreshPluginsList(flutterProject); + expect(flutterProject.flutterPluginsDependenciesFile, isNot(exists)); + + // Create an initial plugin (we previously had none). + createLegacyPluginWithDependencies(name: 'plugin-a', dependencies: []); + await refreshPluginsList(flutterProject, iosPlatform: true, macOSPlatform: true); + expect(flutterProject.flutterPluginsDependenciesFile, exists); + final FileStat stat1 = flutterProject.flutterPluginsDependenciesFile.statSync(); + + // Add a new plugin. + createLegacyPluginWithDependencies(name: 'plugin-b', dependencies: []); + await refreshPluginsList(flutterProject, iosPlatform: true, macOSPlatform: true); + expect(flutterProject.flutterPluginsDependenciesFile, exists); + final FileStat stat2 = flutterProject.flutterPluginsDependenciesFile.statSync(); + expect( + stat2.modified.isAfter(stat1.modified), + isTrue, + reason: 'A new plugin was added, .flutter-plugins-dependencies file should be updated.', + ); + + // Do not add new plugins. + await refreshPluginsList(flutterProject, iosPlatform: true, macOSPlatform: true); + expect(flutterProject.flutterPluginsDependenciesFile, exists); + final FileStat stat3 = flutterProject.flutterPluginsDependenciesFile.statSync(); + expect( + stat3.modified, + stat2.modified, + reason: 'No plugins changed, .flutter-plugins-dependencies should not be changed', + ); + }, + overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + SystemClock: () => systemClock, + FlutterVersion: () => flutterVersion, + Pub: FakePubWithPrimedDeps.new, + // TODO(matanlurey): Remove as part of https://github.com/flutter/flutter/issues/160257. + // Not necessary, you can observe this bug by calling `generateLegacyPlugins: false`, + // but since this flag is about to be enabled, and enabling it implicitly sets that + // argument to false, this is a more "honest" test. + FeatureFlags: enableExplicitPackageDependencies, + }, + ); + testUsingContext( 'Refreshing the plugin list for iOS/macOS projects invokes invalidatePodInstallOutput if the plugins changed', () async { @@ -718,7 +762,7 @@ dependencies: await refreshPluginsList(flutterProject); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), true); + expect(flutterProject.flutterPluginsDependenciesFile, exists); final String pluginsString = flutterProject.flutterPluginsDependenciesFile.readAsStringSync(); final Map jsonContent = @@ -808,7 +852,7 @@ dependencies: await refreshPluginsList(flutterProject, iosPlatform: true, macOSPlatform: true); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), true); + expect(flutterProject.flutterPluginsDependenciesFile, exists); final String pluginsString = flutterProject.flutterPluginsDependenciesFile.readAsStringSync(); final Map jsonContent = @@ -860,7 +904,7 @@ dependencies: await refreshPluginsList(flutterProject, forceCocoaPodsOnly: true); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), true); + expect(flutterProject.flutterPluginsDependenciesFile, exists); final String pluginsString = flutterProject.flutterPluginsDependenciesFile.readAsStringSync(); final Map jsonContent = @@ -912,7 +956,7 @@ dependencies: await refreshPluginsList(flutterProject, iosPlatform: true, macOSPlatform: true); - expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), true); + expect(flutterProject.flutterPluginsDependenciesFile, exists); final String pluginsString = flutterProject.flutterPluginsDependenciesFile.readAsStringSync(); final Map jsonContent = @@ -944,8 +988,8 @@ dependencies: macosProject.exists = true; await refreshPluginsList(flutterProject, iosPlatform: true, macOSPlatform: true); - expect(iosProject.podManifestLock.existsSync(), false); - expect(macosProject.podManifestLock.existsSync(), false); + expect(iosProject.podManifestLock, isNot(exists)); + expect(macosProject.podManifestLock, isNot(exists)); }, overrides: { FileSystem: () => fs, @@ -973,8 +1017,8 @@ dependencies: simulatePodInstallRun(macosProject); await refreshPluginsList(flutterProject); - expect(iosProject.podManifestLock.existsSync(), true); - expect(macosProject.podManifestLock.existsSync(), true); + expect(iosProject.podManifestLock, exists); + expect(macosProject.podManifestLock, exists); }, overrides: { FileSystem: () => fs, @@ -1007,7 +1051,7 @@ dependencies: ) .childFile('GeneratedPluginRegistrant.java'); - expect(registrant.existsSync(), isTrue); + expect(registrant, exists); expect(registrant.readAsStringSync(), contains('package io.flutter.plugins')); expect(registrant.readAsStringSync(), contains('class GeneratedPluginRegistrant')); expect( @@ -1070,7 +1114,7 @@ dependencies: ) .childFile('GeneratedPluginRegistrant.java'); - expect(registrant.existsSync(), isTrue); + expect(registrant, exists); expect(registrant.readAsStringSync(), contains('package io.flutter.plugins')); expect(registrant.readAsStringSync(), contains('class GeneratedPluginRegistrant')); expect( @@ -1101,7 +1145,7 @@ dependencies: ) .childFile('GeneratedPluginRegistrant.java'); - expect(registrant.existsSync(), isTrue); + expect(registrant, exists); expect(registrant.readAsStringSync(), contains('package io.flutter.plugins')); expect(registrant.readAsStringSync(), contains('class GeneratedPluginRegistrant')); expect( @@ -1259,7 +1303,7 @@ dependencies: .childDirectory('lib') .childFile('web_plugin_registrant.dart'); - expect(registrant.existsSync(), isTrue); + expect(registrant, exists); expect( registrant.readAsStringSync(), contains("import 'package:web_plugin_with_nested/src/web_plugin.dart';"), @@ -1325,7 +1369,7 @@ dependencies: .childDirectory('lib') .childFile('web_plugin_registrant.dart'); - expect(registrant.existsSync(), isTrue); + expect(registrant, exists); expect( registrant.readAsStringSync(), contains( @@ -1533,8 +1577,8 @@ flutter: 'generated_plugin_registrant.cc', ); - expect(registrantHeader.existsSync(), isTrue); - expect(registrantImpl.existsSync(), isTrue); + expect(registrantHeader, exists); + expect(registrantImpl, exists); expect( registrantImpl.readAsStringSync(), contains('some_plugin_register_with_registrar'), @@ -1593,7 +1637,7 @@ dependencies: 'generated_plugin_registrant.cc', ); - expect(registrantImpl.existsSync(), isTrue); + expect(registrantImpl, exists); expect( registrantImpl.readAsStringSync(), contains('user_selected_url_launcher_linux_register_with_registrar'), @@ -1666,7 +1710,7 @@ dependencies: 'generated_plugin_registrant.cc', ); - expect(registrantImpl.existsSync(), isTrue); + expect(registrantImpl, exists); expect( registrantImpl.readAsStringSync(), contains('user_selected_url_launcher_linux_register_with_registrar'), @@ -1756,7 +1800,7 @@ flutter: final File pluginMakefile = linuxProject.generatedPluginCmakeFile; - expect(pluginMakefile.existsSync(), isTrue); + expect(pluginMakefile, exists); final String contents = pluginMakefile.readAsStringSync(); expect(contents, contains('some_plugin')); expect( @@ -1825,8 +1869,8 @@ flutter: 'generated_plugin_registrant.cc', ); - expect(registrantHeader.existsSync(), isTrue); - expect(registrantImpl.existsSync(), isTrue); + expect(registrantHeader, exists); + expect(registrantImpl, exists); expect(registrantImpl.readAsStringSync(), contains('SomePluginRegisterWithRegistrar')); }, overrides: { @@ -1950,7 +1994,7 @@ flutter: ]) { final File pluginCmakefile = project!.generatedPluginCmakeFile; - expect(pluginCmakefile.existsSync(), isTrue); + expect(pluginCmakefile, exists); final String contents = pluginCmakefile.readAsStringSync(); expect(contents, contains('add_subdirectory(flutter/ephemeral/.plugin_symlinks')); } @@ -2024,7 +2068,7 @@ flutter: // refreshPluginsList should call createPluginSymlinks. await refreshPluginsList(flutterProject); - expect(linuxProject.pluginSymlinkDirectory.childLink('some_plugin').existsSync(), true); + expect(linuxProject.pluginSymlinkDirectory.childLink('some_plugin'), exists); }, overrides: { FileSystem: () => fs, @@ -2041,7 +2085,7 @@ flutter: // refreshPluginsList should call createPluginSymlinks. await refreshPluginsList(flutterProject); - expect(windowsProject.pluginSymlinkDirectory.childLink('some_plugin').existsSync(), true); + expect(windowsProject.pluginSymlinkDirectory.childLink('some_plugin'), exists); }, overrides: { FileSystem: () => fs, @@ -2067,7 +2111,7 @@ flutter: createPluginSymlinks(flutterProject, force: true); for (final File file in dummyFiles) { - expect(file.existsSync(), false); + expect(file, isNot(exists)); } }, overrides: { @@ -2096,7 +2140,7 @@ flutter: await refreshPluginsList(flutterProject); for (final File file in dummyFiles) { - expect(file.existsSync(), false); + expect(file, isNot(exists)); } }, overrides: { @@ -2124,7 +2168,7 @@ flutter: createPluginSymlinks(flutterProject); for (final File file in dummyFiles) { - expect(file.existsSync(), true); + expect(file, exists); } }, overrides: { @@ -2152,7 +2196,7 @@ flutter: createPluginSymlinks(flutterProject); for (final Link link in links) { - expect(link.existsSync(), true); + expect(link, exists); } }, overrides: {