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 =