From 1bbfb608161595d229a98bf6eded4bf8d4534f8f Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Fri, 11 Mar 2022 15:15:26 -0500 Subject: [PATCH] [flutter_plugin_tools] Fix subpackage analysis (#5027) --- script/tool/CHANGELOG.md | 5 +++ script/tool/lib/src/analyze_command.dart | 29 +++++++++------ .../tool/lib/src/common/plugin_command.dart | 20 +++++++---- .../lib/src/make_deps_path_based_command.dart | 4 +++ script/tool/pubspec.yaml | 2 +- script/tool/test/analyze_command_test.dart | 25 +++++++++++++ .../make_deps_path_based_command_test.dart | 36 +++++++++++++++++++ 7 files changed, 103 insertions(+), 18 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 8e5ae210ce..35786f4a8b 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.8.1 + +- Fixes an `analyze` regression in 0.8.0 with packages that have non-`example` + sub-packages. + ## 0.8.0 - Ensures that `firebase-test-lab` runs include an `integration_test` runner. diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index d8b17bfd56..824d766f4e 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -102,16 +102,25 @@ class AnalyzeCommand extends PackageLoopingCommand { @override Future runForPackage(RepositoryPackage package) async { - // For non-example packages, fetch dependencies. 'flutter packages get' - // automatically runs 'pub get' in examples as part of handling the parent - // directory, which is guaranteed to come first in the package enumeration. - if (package.directory.basename != 'example' || - !RepositoryPackage(package.directory.parent).pubspecFile.existsSync()) { - final int exitCode = await processRunner.runAndStream( - flutterCommand, ['packages', 'get'], - workingDir: package.directory); - if (exitCode != 0) { - return PackageResult.fail(['Unable to get dependencies']); + // Analysis runs over the package and all subpackages, so all of them need + // `flutter packages get` run before analyzing. `example` packages can be + // skipped since 'flutter packages get' automatically runs `pub get` in + // examples as part of handling the parent directory. + final List packagesToGet = [ + package, + ...await getSubpackages(package).toList(), + ]; + for (final RepositoryPackage packageToGet in packagesToGet) { + if (packageToGet.directory.basename != 'example' || + !RepositoryPackage(packageToGet.directory.parent) + .pubspecFile + .existsSync()) { + final int exitCode = await processRunner.runAndStream( + flutterCommand, ['packages', 'get'], + workingDir: packageToGet.directory); + if (exitCode != 0) { + return PackageResult.fail(['Unable to get dependencies']); + } } } diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart index fcc87c94ef..d210028996 100644 --- a/script/tool/lib/src/common/plugin_command.dart +++ b/script/tool/lib/src/common/plugin_command.dart @@ -417,16 +417,22 @@ abstract class PluginCommand extends Command { await for (final PackageEnumerationEntry plugin in getTargetPackages(filterExcluded: filterExcluded)) { yield plugin; - yield* plugin.package.directory - .list(recursive: true, followLinks: false) - .where(_isDartPackage) - .map((FileSystemEntity directory) => PackageEnumerationEntry( - // _isDartPackage guarantees that this cast is valid. - RepositoryPackage(directory as Directory), - excluded: plugin.excluded)); + yield* getSubpackages(plugin.package).map((RepositoryPackage package) => + PackageEnumerationEntry(package, excluded: plugin.excluded)); } } + /// Returns all Dart package folders (e.g., examples) under the given package. + Stream getSubpackages(RepositoryPackage package, + {bool filterExcluded = true}) async* { + yield* package.directory + .list(recursive: true, followLinks: false) + .where(_isDartPackage) + .map((FileSystemEntity directory) => + // _isDartPackage guarantees that this cast is valid. + RepositoryPackage(directory as Directory)); + } + /// Returns the files contained, recursively, within the packages /// involved in this command execution. Stream getFiles() { 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 5ee42848a8..c09060310e 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -207,6 +207,10 @@ dependency_overrides: allComponents.contains('example')) { continue; } + if (!allComponents.contains(packagesDir.basename)) { + print(' Skipping $changedPath; not in packages directory.'); + continue; + } final RepositoryPackage package = RepositoryPackage(packagesDir.fileSystem.file(changedPath).parent); // Ignored deleted packages, as they won't be published. diff --git a/script/tool/pubspec.yaml b/script/tool/pubspec.yaml index 9ca5e2b775..93a1a87ca3 100644 --- a/script/tool/pubspec.yaml +++ b/script/tool/pubspec.yaml @@ -1,7 +1,7 @@ name: flutter_plugin_tools description: Productivity utils for flutter/plugins and flutter/packages repository: https://github.com/flutter/plugins/tree/main/script/tool -version: 0.8.0 +version: 0.8.1 dependencies: args: ^2.1.0 diff --git a/script/tool/test/analyze_command_test.dart b/script/tool/test/analyze_command_test.dart index 087e5ada37..56f4ab9576 100644 --- a/script/tool/test/analyze_command_test.dart +++ b/script/tool/test/analyze_command_test.dart @@ -71,6 +71,31 @@ void main() { ])); }); + test('runs flutter pub get for non-example subpackages', () async { + final Directory mainPackageDir = createFakePackage('a', packagesDir); + final Directory otherPackages = + mainPackageDir.childDirectory('other_packages'); + final Directory subpackage1 = + createFakePackage('subpackage1', otherPackages); + final Directory subpackage2 = + createFakePackage('subpackage2', otherPackages); + + await runCapturingPrint(runner, ['analyze']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('flutter', const ['packages', 'get'], + mainPackageDir.path), + ProcessCall( + 'flutter', const ['packages', 'get'], subpackage1.path), + ProcessCall( + 'flutter', const ['packages', 'get'], subpackage2.path), + ProcessCall('dart', const ['analyze', '--fatal-infos'], + mainPackageDir.path), + ])); + }); + test('don\'t elide a non-contained example package', () async { final Directory plugin1Dir = createFakePlugin('a', packagesDir); final Directory plugin2Dir = createFakePlugin('example', packagesDir); 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 bdd2139b23..2021f24079 100644 --- a/script/tool/test/make_deps_path_based_command_test.dart +++ b/script/tool/test/make_deps_path_based_command_test.dart @@ -323,5 +323,41 @@ void main() { ]), ); }); + + test('skips anything outside of the packages directory', () async { + final Directory toolDir = packagesDir.parent.childDirectory('tool'); + const String newVersion = '1.1.0'; + final Directory package = createFakePackage( + 'flutter_plugin_tools', toolDir, + version: newVersion); + + // Simulate a minor version change so it would be a target. + final File pubspecFile = RepositoryPackage(package).pubspecFile; + final String changedFileOutput = [ + pubspecFile, + ].map((File file) => file.path).join('\n'); + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: changedFileOutput), + ]; + final String gitPubspecContents = + pubspecFile.readAsStringSync().replaceAll(newVersion, '1.0.0'); + processRunner.mockProcessesForExecutable['git-show'] = [ + MockProcess(stdout: gitPubspecContents), + ]; + + final List output = await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies-with-non-breaking-updates' + ]); + + expect( + output, + containsAllInOrder([ + contains( + 'Skipping /tool/flutter_plugin_tools/pubspec.yaml; not in packages directory.'), + contains('No target dependencies'), + ]), + ); + }); }); }