From 52f49c1aa2b783e612d82b8b070340b184fdd752 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Mon, 6 Dec 2021 13:23:14 -0500 Subject: [PATCH] Remove deprecated plugins (#4580) This removes all of the deprecated plugins from the tree, as well as all references to them. They were being left in the tree only in case they needed critical fixes before the end of this year, when they will become completely unsupported and be officially discontinued on pub.dev. The end of the year is now close enough that such a release is extremely unlikely, and they are causing some maintenance burden (e.g., the tree is currently closed on an out-of-band failure in android_alarm_manager). In the event that we do have to push an emergency fix in the next several weeks, we can either temporarily restore the plugin from git history, or use a branch. Minor related cleanup: - Scanning the repository for remaining references turned up that `image_picker_for_web` and `video_player_for_web` had copypasta'd the package name of its example application from one of the deprecated plugins, so this fixes those names. - Fixes `federation-safety-check` handling of top-level files in unfederated plugins that accidentally tripped federated plugin heuristics. - Fixes `federation-safety-check` handling of deleted plugins, as it was crashing, and tests that. - Fixes `make-deps-path-based` handling of deleted plugins, as it was crashing, and tests that. --- script/tool/CHANGELOG.md | 3 + .../src/federation_safety_check_command.dart | 7 ++- .../lib/src/make_deps_path_based_command.dart | 16 +++-- .../federation_safety_check_command_test.dart | 60 +++++++++++++++++++ .../make_deps_path_based_command_test.dart | 23 +++++++ 5 files changed, 104 insertions(+), 5 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 234700ab5a..dec218e2ba 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -5,6 +5,9 @@ dependencies to path-based dependencies. - Adds a (hidden) `--run-on-dirty-packages` flag for use with `make-deps-path-based` in CI. +- Fix `federation-safety-check` handling of plugin deletion, and of top-level + files in unfederated plugins whose names match federated plugin heuristics + (e.g., `packages/foo/foo_android.iml`). ## 0.7.3 diff --git a/script/tool/lib/src/federation_safety_check_command.dart b/script/tool/lib/src/federation_safety_check_command.dart index 200f9c3f48..df9d86892e 100644 --- a/script/tool/lib/src/federation_safety_check_command.dart +++ b/script/tool/lib/src/federation_safety_check_command.dart @@ -81,7 +81,8 @@ class FederationSafetyCheckCommand extends PackageLoopingCommand { // Count the top-level plugin as changed. _changedPlugins.add(packageName); if (relativeComponents[0] == packageName || - relativeComponents[0].startsWith('${packageName}_')) { + (relativeComponents.length > 1 && + relativeComponents[0].startsWith('${packageName}_'))) { packageName = relativeComponents.removeAt(0); } @@ -178,6 +179,10 @@ class FederationSafetyCheckCommand extends PackageLoopingCommand { String pubspecRepoRelativePosixPath) async { final File pubspecFile = childFileWithSubcomponents( packagesDir.parent, p.posix.split(pubspecRepoRelativePosixPath)); + if (!pubspecFile.existsSync()) { + // If the package was deleted, nothing will be published. + return false; + } final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync()); if (pubspec.publishTo == 'none') { return false; diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart index 04869639cf..5ee42848a8 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -196,19 +196,27 @@ dependency_overrides: Future> _getNonBreakingUpdatePackages() async { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); final String baseSha = await gitVersionFinder.getBaseSha(); - print('Finding changed packages relative to "$baseSha"\n'); + print('Finding changed packages relative to "$baseSha"...'); final Set changedPackages = {}; - for (final String path in await gitVersionFinder.getChangedFiles()) { + for (final String changedPath in await gitVersionFinder.getChangedFiles()) { // Git output always uses Posix paths. - final List allComponents = p.posix.split(path); + final List allComponents = p.posix.split(changedPath); // Only pubspec changes are potential publishing events. if (allComponents.last != 'pubspec.yaml' || allComponents.contains('example')) { continue; } final RepositoryPackage package = - RepositoryPackage(packagesDir.fileSystem.file(path).parent); + RepositoryPackage(packagesDir.fileSystem.file(changedPath).parent); + // Ignored deleted packages, as they won't be published. + if (!package.pubspecFile.existsSync()) { + final String directoryName = p.posix.joinAll(path.split(path.relative( + package.directory.absolute.path, + from: packagesDir.path))); + print(' Skipping $directoryName; deleted.'); + continue; + } final String packageName = package.parsePubspec().name; if (!await _hasNonBreakingVersionChange(package)) { // Log packages that had pubspec changes but weren't included for ease diff --git a/script/tool/test/federation_safety_check_command_test.dart b/script/tool/test/federation_safety_check_command_test.dart index e23485fbc8..126aa8b41c 100644 --- a/script/tool/test/federation_safety_check_command_test.dart +++ b/script/tool/test/federation_safety_check_command_test.dart @@ -352,4 +352,64 @@ void main() { ]), ); }); + + test('handles top-level files that match federated package heuristics', + () async { + final Directory plugin = createFakePlugin('foo', packagesDir); + + final String changedFileOutput = [ + // This should be picked up as a change to 'foo', and not crash. + plugin.childFile('foo_bar.baz'), + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: changedFileOutput), + ]; + + final List output = + await runCapturingPrint(runner, ['federation-safety-check']); + + expect( + output, + containsAllInOrder([ + contains('Running for foo...'), + ]), + ); + }); + + test('handles deletion of an entire plugin', () async { + // Simulate deletion, in the form of diffs for packages that don't exist in + // the filesystem. + final String changedFileOutput = [ + packagesDir.childDirectory('foo').childFile('pubspec.yaml'), + packagesDir + .childDirectory('foo') + .childDirectory('lib') + .childFile('foo.dart'), + packagesDir + .childDirectory('foo_platform_interface') + .childFile('pubspec.yaml'), + packagesDir + .childDirectory('foo_platform_interface') + .childDirectory('lib') + .childFile('foo.dart'), + packagesDir.childDirectory('foo_web').childFile('pubspec.yaml'), + packagesDir + .childDirectory('foo_web') + .childDirectory('lib') + .childFile('foo.dart'), + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: changedFileOutput), + ]; + + final List output = + await runCapturingPrint(runner, ['federation-safety-check']); + + expect( + output, + containsAllInOrder([ + contains('Ran for 0 package(s)'), + ]), + ); + }); } diff --git a/script/tool/test/make_deps_path_based_command_test.dart b/script/tool/test/make_deps_path_based_command_test.dart index 29e4f724b3..bdd2139b23 100644 --- a/script/tool/test/make_deps_path_based_command_test.dart +++ b/script/tool/test/make_deps_path_based_command_test.dart @@ -173,6 +173,29 @@ void main() { ); }); + test('no-ops for no deleted packages', () async { + final String changedFileOutput = [ + // A change for a file that's not on disk simulates a deletion. + packagesDir.childDirectory('foo').childFile('pubspec.yaml'), + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: changedFileOutput), + ]; + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies-with-non-breaking-updates' + ]); + + expect( + output, + containsAllInOrder([ + contains('Skipping foo; deleted.'), + contains('No target dependencies'), + ]), + ); + }); + test('includes bugfix version changes as targets', () async { const String newVersion = '1.0.1'; final Directory package =