From f2d9f51dd1028bbbe79981d1496f28ee22768483 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 27 Jul 2022 13:13:05 -0400 Subject: [PATCH] [path_provider] Restore 2.8 compatibility on Android and iOS (#6039) Adds a new repo tooling command that removes dev_dependencies, which aren't needed to consume a package, only for development. Also adds a --lib-only flag to analyze to analyze only the client-facing code. This is intended for use in the legacy analyze CI steps, primarily to solve the problem that currently plugins that use Pigeon can't support a version of Flutter older than the version supported by Pigeon, because otherwise the legacy analysis CI steps fail. Adds this new command to the legacy analysis CI step, and restores the recently-removed 2.8/2.10 compatibility to path_provider. --- script/tool/CHANGELOG.md | 7 ++ script/tool/lib/src/analyze_command.dart | 26 +++-- script/tool/lib/src/main.dart | 2 + .../tool/lib/src/remove_dev_dependencies.dart | 58 ++++++++++ script/tool/pubspec.yaml | 2 +- script/tool/test/analyze_command_test.dart | 53 +++++++++ .../test/remove_dev_dependencies_test.dart | 102 ++++++++++++++++++ 7 files changed, 241 insertions(+), 9 deletions(-) create mode 100644 script/tool/lib/src/remove_dev_dependencies.dart create mode 100644 script/tool/test/remove_dev_dependencies_test.dart diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index f0534c23a2..ad3f35a8e9 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.8.10 + +- Adds a new `remove-dev-dependencies` command to remove `dev_dependencies` + entries to make legacy version analysis possible in more cases. +- Adds a `--lib-only` option to `analyze` to allow only analyzing the client + parts of a library for legacy verison compatibility. + ## 0.8.9 - Includes `dev_dependencies` when overridding dependencies using diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 8778b3de9d..54b4f33a23 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -32,10 +32,13 @@ class AnalyzeCommand extends PackageLoopingCommand { valueHelp: 'dart-sdk', help: 'An optional path to a Dart SDK; this is used to override the ' 'SDK used to provide analysis.'); + argParser.addFlag(_libOnlyFlag, + help: 'Only analyze the lib/ directory of the main package, not the ' + 'entire package.'); } static const String _customAnalysisFlag = 'custom-analysis'; - + static const String _libOnlyFlag = 'lib-only'; static const String _analysisSdk = 'analysis-sdk'; late String _dartBinaryPath; @@ -104,13 +107,20 @@ class AnalyzeCommand extends PackageLoopingCommand { @override Future runForPackage(RepositoryPackage package) async { - // Analysis runs over the package and all subpackages, so all of them need - // `flutter pub 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 bool libOnly = getBoolArg(_libOnlyFlag); + + if (libOnly && !package.libDirectory.existsSync()) { + return PackageResult.skip('No lib/ directory.'); + } + + // Analysis runs over the package and all subpackages (unless only lib/ is + // being analyzed), so all of them need `flutter pub 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(), + if (!libOnly) ...await getSubpackages(package).toList(), ]; for (final RepositoryPackage packageToGet in packagesToGet) { if (packageToGet.directory.basename != 'example' || @@ -129,8 +139,8 @@ class AnalyzeCommand extends PackageLoopingCommand { if (_hasUnexpecetdAnalysisOptions(package)) { return PackageResult.fail(['Unexpected local analysis options']); } - final int exitCode = await processRunner.runAndStream( - _dartBinaryPath, ['analyze', '--fatal-infos'], + final int exitCode = await processRunner.runAndStream(_dartBinaryPath, + ['analyze', '--fatal-infos', if (libOnly) 'lib'], workingDir: package.directory); if (exitCode != 0) { return PackageResult.fail(); diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 966e7b6be5..ea1ec067c8 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -28,6 +28,7 @@ import 'publish_check_command.dart'; import 'publish_plugin_command.dart'; import 'pubspec_check_command.dart'; import 'readme_check_command.dart'; +import 'remove_dev_dependencies.dart'; import 'test_command.dart'; import 'update_excerpts_command.dart'; import 'update_release_info_command.dart'; @@ -71,6 +72,7 @@ void main(List args) { ..addCommand(PublishPluginCommand(packagesDir)) ..addCommand(PubspecCheckCommand(packagesDir)) ..addCommand(ReadmeCheckCommand(packagesDir)) + ..addCommand(RemoveDevDependenciesCommand(packagesDir)) ..addCommand(TestCommand(packagesDir)) ..addCommand(UpdateExcerptsCommand(packagesDir)) ..addCommand(UpdateReleaseInfoCommand(packagesDir)) diff --git a/script/tool/lib/src/remove_dev_dependencies.dart b/script/tool/lib/src/remove_dev_dependencies.dart new file mode 100644 index 0000000000..3085e0df85 --- /dev/null +++ b/script/tool/lib/src/remove_dev_dependencies.dart @@ -0,0 +1,58 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:yaml/yaml.dart'; +import 'package:yaml_edit/yaml_edit.dart'; + +import 'common/package_looping_command.dart'; +import 'common/repository_package.dart'; + +/// A command to remove dev_dependencies, which are not used by package clients. +/// +/// This is intended for use with legacy Flutter version testing, to allow +/// running analysis (with --lib-only) with versions that are supported for +/// clients of the library, but not for development of the library. +class RemoveDevDependenciesCommand extends PackageLoopingCommand { + /// Creates a publish metadata updater command instance. + RemoveDevDependenciesCommand(Directory packagesDir) : super(packagesDir); + + @override + final String name = 'remove-dev-dependencies'; + + @override + final String description = 'Removes any dev_dependencies section from a ' + 'package, to allow more legacy testing.'; + + @override + bool get hasLongOutput => false; + + @override + PackageLoopingType get packageLoopingType => + PackageLoopingType.includeAllSubpackages; + + @override + Future runForPackage(RepositoryPackage package) async { + bool changed = false; + final YamlEditor editablePubspec = + YamlEditor(package.pubspecFile.readAsStringSync()); + const String devDependenciesKey = 'dev_dependencies'; + final YamlNode root = editablePubspec.parseAt([]); + final YamlMap? devDependencies = + (root as YamlMap)[devDependenciesKey] as YamlMap?; + if (devDependencies != null) { + changed = true; + print('${indentation}Removed dev_dependencies'); + editablePubspec.remove([devDependenciesKey]); + } + + if (changed) { + package.pubspecFile.writeAsStringSync(editablePubspec.toString()); + } + + return changed + ? PackageResult.success() + : PackageResult.skip('Nothing to remove.'); + } +} diff --git a/script/tool/pubspec.yaml b/script/tool/pubspec.yaml index a4ecbfb75e..2eee1e3c7b 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.9 +version: 0.8.10 dependencies: args: ^2.1.0 diff --git a/script/tool/test/analyze_command_test.dart b/script/tool/test/analyze_command_test.dart index a4a47a2211..d3abf0b6b8 100644 --- a/script/tool/test/analyze_command_test.dart +++ b/script/tool/test/analyze_command_test.dart @@ -93,6 +93,59 @@ void main() { ])); }); + test('passes lib/ directory with --lib-only', () async { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + + await runCapturingPrint(runner, ['analyze', '--lib-only']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('flutter', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['analyze', '--fatal-infos', 'lib'], + package.path), + ])); + }); + + test('skips when missing lib/ directory with --lib-only', () async { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + package.libDirectory.deleteSync(); + + final List output = + await runCapturingPrint(runner, ['analyze', '--lib-only']); + + expect(processRunner.recordedCalls, isEmpty); + expect( + output, + containsAllInOrder([ + contains('SKIPPING: No lib/ directory'), + ]), + ); + }); + + test( + 'does not run flutter pub get for non-example subpackages with --lib-only', + () async { + final RepositoryPackage mainPackage = createFakePackage('a', packagesDir); + final Directory otherPackagesDir = + mainPackage.directory.childDirectory('other_packages'); + createFakePackage('subpackage1', otherPackagesDir); + createFakePackage('subpackage2', otherPackagesDir); + + await runCapturingPrint(runner, ['analyze', '--lib-only']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + 'flutter', const ['pub', 'get'], mainPackage.path), + ProcessCall('dart', const ['analyze', '--fatal-infos', 'lib'], + mainPackage.path), + ])); + }); + test("don't elide a non-contained example package", () async { final RepositoryPackage plugin1 = createFakePlugin('a', packagesDir); final RepositoryPackage plugin2 = createFakePlugin('example', packagesDir); diff --git a/script/tool/test/remove_dev_dependencies_test.dart b/script/tool/test/remove_dev_dependencies_test.dart new file mode 100644 index 0000000000..6b212870c5 --- /dev/null +++ b/script/tool/test/remove_dev_dependencies_test.dart @@ -0,0 +1,102 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:args/command_runner.dart'; +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_plugin_tools/src/remove_dev_dependencies.dart'; +import 'package:test/test.dart'; + +import 'util.dart'; + +void main() { + late FileSystem fileSystem; + late Directory packagesDir; + late CommandRunner runner; + + setUp(() { + fileSystem = MemoryFileSystem(); + packagesDir = createPackagesDirectory(fileSystem: fileSystem); + + final RemoveDevDependenciesCommand command = RemoveDevDependenciesCommand( + packagesDir, + ); + runner = CommandRunner('trim_dev_dependencies_command', + 'Test for trim_dev_dependencies_command'); + runner.addCommand(command); + }); + + void _addToPubspec(RepositoryPackage package, String addition) { + final String originalContent = package.pubspecFile.readAsStringSync(); + package.pubspecFile.writeAsStringSync(''' +$originalContent +$addition +'''); + } + + test('skips if nothing is removed', () async { + createFakePackage('a_package', packagesDir, version: '1.0.0'); + + final List output = + await runCapturingPrint(runner, ['remove-dev-dependencies']); + + expect( + output, + containsAllInOrder([ + contains('SKIPPING: Nothing to remove.'), + ]), + ); + }); + + test('removes dev_dependencies', () async { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir, version: '1.0.0'); + + _addToPubspec(package, ''' +dev_dependencies: + some_dependency: ^2.1.8 + another_dependency: ^1.0.0 +'''); + + final List output = + await runCapturingPrint(runner, ['remove-dev-dependencies']); + + expect( + output, + containsAllInOrder([ + contains('Removed dev_dependencies'), + ]), + ); + expect(package.pubspecFile.readAsStringSync(), + isNot(contains('some_dependency:'))); + expect(package.pubspecFile.readAsStringSync(), + isNot(contains('another_dependency:'))); + }); + + test('removes from examples', () async { + final RepositoryPackage package = + createFakePackage('a_package', packagesDir, version: '1.0.0'); + + final RepositoryPackage example = package.getExamples().first; + _addToPubspec(example, ''' +dev_dependencies: + some_dependency: ^2.1.8 + another_dependency: ^1.0.0 +'''); + + final List output = + await runCapturingPrint(runner, ['remove-dev-dependencies']); + + expect( + output, + containsAllInOrder([ + contains('Removed dev_dependencies'), + ]), + ); + expect(package.pubspecFile.readAsStringSync(), + isNot(contains('some_dependency:'))); + expect(package.pubspecFile.readAsStringSync(), + isNot(contains('another_dependency:'))); + }); +}