forked from firka/flutter
[tool] Don't write the .flutter-plugins-dependencies file if it is unchanged (#166164)
Currently, `refreshPlugins` always updates the `.flutter-plugins-dependencies` file. In a future change, `flutter assemble` will use the `.flutter-plugins-dependencies` file as an input. Unnecessary writes to this file will invalidate build targets. This updates the logic to only write .flutter-plugins-dependencies if it has "significant" changes. Part of https://github.com/flutter/flutter/issues/163874 Next pull request: https://github.com/flutter/flutter/pull/165916 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Victoria Ashworth <15619084+vashworth@users.noreply.github.com>
This commit is contained in:
Loïc Sharma
@@ -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<String, Object?>) {
|
||||
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<String, Object> newJson,
|
||||
) {
|
||||
final String? oldPluginsFileStringContent = _readFileContent(pluginsFile);
|
||||
if (oldPluginsFileStringContent == null) {
|
||||
return (pluginsChanged: true, contentsChanged: true);
|
||||
}
|
||||
|
||||
try {
|
||||
final Object? oldJson = jsonDecode(oldPluginsFileStringContent);
|
||||
if (oldJson is! Map<String, Object?>) {
|
||||
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<String, Object?> newJsonCopy = <String, Object?>{...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<String> ignoredKeys = <String>[
|
||||
'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<Map<String, Object>> _createPluginMapOfPlatform(List<Plugin> plugins, String platformKey) {
|
||||
|
||||
@@ -409,7 +409,7 @@ dependencies:
|
||||
() async {
|
||||
await refreshPluginsList(flutterProject);
|
||||
|
||||
expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), false);
|
||||
expect(flutterProject.flutterPluginsDependenciesFile, isNot(exists));
|
||||
},
|
||||
overrides: <Type, Generator>{
|
||||
FileSystem: () => fs,
|
||||
@@ -426,7 +426,7 @@ dependencies:
|
||||
|
||||
await refreshPluginsList(flutterProject);
|
||||
|
||||
expect(flutterProject.flutterPluginsDependenciesFile.existsSync(), false);
|
||||
expect(flutterProject.flutterPluginsDependenciesFile, isNot(exists));
|
||||
},
|
||||
overrides: <Type, Generator>{
|
||||
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: <String>[]);
|
||||
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: <String>[]);
|
||||
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: <Type, Generator>{
|
||||
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<String, dynamic> 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<String, dynamic> 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<String, dynamic> 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<String, dynamic> 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: <Type, Generator>{
|
||||
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: <Type, Generator>{
|
||||
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: <Type, Generator>{
|
||||
@@ -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: <Type, Generator>{
|
||||
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: <Type, Generator>{
|
||||
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: <Type, Generator>{
|
||||
@@ -2096,7 +2140,7 @@ flutter:
|
||||
await refreshPluginsList(flutterProject);
|
||||
|
||||
for (final File file in dummyFiles) {
|
||||
expect(file.existsSync(), false);
|
||||
expect(file, isNot(exists));
|
||||
}
|
||||
},
|
||||
overrides: <Type, Generator>{
|
||||
@@ -2124,7 +2168,7 @@ flutter:
|
||||
createPluginSymlinks(flutterProject);
|
||||
|
||||
for (final File file in dummyFiles) {
|
||||
expect(file.existsSync(), true);
|
||||
expect(file, exists);
|
||||
}
|
||||
},
|
||||
overrides: <Type, Generator>{
|
||||
@@ -2152,7 +2196,7 @@ flutter:
|
||||
createPluginSymlinks(flutterProject);
|
||||
|
||||
for (final Link link in links) {
|
||||
expect(link.existsSync(), true);
|
||||
expect(link, exists);
|
||||
}
|
||||
},
|
||||
overrides: <Type, Generator>{
|
||||
|
||||
Reference in New Issue
Block a user