diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index bfc0cafa88..82a31154e1 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,5 +1,8 @@ -## NEXT +## 0.8.3 +- Adds a new `update-excerpts` command to maintain README files using the + `code-excerpter` package from flutter/site-shared. +- `license-check` now ignores submodules. - Allows `make-deps-path-based` to skip packages it has alredy rewritten, so that running multiple times won't fail after the first time. diff --git a/script/tool/README.md b/script/tool/README.md index 05be917014..d52ee08fdc 100644 --- a/script/tool/README.md +++ b/script/tool/README.md @@ -107,6 +107,17 @@ dart run ./script/tool/bin/flutter_plugin_tools.dart native-test --ios --android dart run ./script/tool/bin/flutter_plugin_tools.dart native-test --macos --packages plugin_name ``` +### Update README.md from Example Sources + +`update-excerpts` requires sources that are in a submodule. If you didn't clone +with submodules, you will need to `git submodule update --init --recursive` +before running this command. + +```sh +cd +dart run ./script/tool/bin/flutter_plugin_tools.dart update-excerpts --packages plugin_name +``` + ### Publish a Release **Releases are automated for `flutter/plugins` and `flutter/packages`.** diff --git a/script/tool/lib/src/license_check_command.dart b/script/tool/lib/src/license_check_command.dart index d2c129ff7b..87e4c8b148 100644 --- a/script/tool/lib/src/license_check_command.dart +++ b/script/tool/lib/src/license_check_command.dart @@ -3,7 +3,9 @@ // found in the LICENSE file. import 'package:file/file.dart'; +import 'package:git/git.dart'; import 'package:path/path.dart' as p; +import 'package:platform/platform.dart'; import 'common/core.dart'; import 'common/plugin_command.dart'; @@ -105,7 +107,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. /// Validates that code files have copyright and license blocks. class LicenseCheckCommand extends PluginCommand { /// Creates a new license check command for [packagesDir]. - LicenseCheckCommand(Directory packagesDir) : super(packagesDir); + LicenseCheckCommand(Directory packagesDir, + {Platform platform = const LocalPlatform(), GitDir? gitDir}) + : super(packagesDir, platform: platform, gitDir: gitDir); @override final String name = 'license-check'; @@ -116,7 +120,14 @@ class LicenseCheckCommand extends PluginCommand { @override Future run() async { - final Iterable allFiles = await _getAllFiles(); + // Create a set of absolute paths to submodule directories, with trailing + // separator, to do prefix matching with to test directory inclusion. + final Iterable submodulePaths = (await _getSubmoduleDirectories()) + .map( + (Directory dir) => '${dir.absolute.path}${platform.pathSeparator}'); + + final Iterable allFiles = (await _getAllFiles()).where( + (File file) => !submodulePaths.any(file.absolute.path.startsWith)); final Iterable codeFiles = allFiles.where((File file) => _codeFileExtensions.contains(p.extension(file.path)) && @@ -275,6 +286,24 @@ class LicenseCheckCommand extends PluginCommand { .where((FileSystemEntity entity) => entity is File) .map((FileSystemEntity file) => file as File) .toList(); + + // Returns the directories containing mapped submodules, if any. + Future> _getSubmoduleDirectories() async { + final List submodulePaths = []; + final Directory repoRoot = + packagesDir.fileSystem.directory((await gitDir).path); + final File submoduleSpec = repoRoot.childFile('.gitmodules'); + if (submoduleSpec.existsSync()) { + final RegExp pathLine = RegExp(r'path\s*=\s*(.*)'); + for (final String line in submoduleSpec.readAsLinesSync()) { + final RegExpMatch? match = pathLine.firstMatch(line); + if (match != null) { + submodulePaths.add(repoRoot.childDirectory(match.group(1)!.trim())); + } + } + } + return submodulePaths; + } } enum _LicenseFailureType { incorrectFirstParty, unknownThirdParty } diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index aa1cf300bd..9c572ee270 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -28,6 +28,7 @@ import 'publish_plugin_command.dart'; import 'pubspec_check_command.dart'; import 'readme_check_command.dart'; import 'test_command.dart'; +import 'update_excerpts_command.dart'; import 'version_check_command.dart'; import 'xcode_analyze_command.dart'; @@ -68,6 +69,7 @@ void main(List args) { ..addCommand(PubspecCheckCommand(packagesDir)) ..addCommand(ReadmeCheckCommand(packagesDir)) ..addCommand(TestCommand(packagesDir)) + ..addCommand(UpdateExcerptsCommand(packagesDir)) ..addCommand(VersionCheckCommand(packagesDir)) ..addCommand(XcodeAnalyzeCommand(packagesDir)); diff --git a/script/tool/lib/src/update_excerpts_command.dart b/script/tool/lib/src/update_excerpts_command.dart new file mode 100644 index 0000000000..320a3c5963 --- /dev/null +++ b/script/tool/lib/src/update_excerpts_command.dart @@ -0,0 +1,225 @@ +// 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 'dart:io' as io; + +import 'package:file/file.dart'; +import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:git/git.dart'; +import 'package:platform/platform.dart'; +import 'package:yaml/yaml.dart'; +import 'package:yaml_edit/yaml_edit.dart'; + +import 'common/package_looping_command.dart'; +import 'common/process_runner.dart'; +import 'common/repository_package.dart'; + +/// A command to update README code excerpts from code files. +class UpdateExcerptsCommand extends PackageLoopingCommand { + /// Creates a excerpt updater command instance. + UpdateExcerptsCommand( + Directory packagesDir, { + ProcessRunner processRunner = const ProcessRunner(), + Platform platform = const LocalPlatform(), + GitDir? gitDir, + }) : super( + packagesDir, + processRunner: processRunner, + platform: platform, + gitDir: gitDir, + ) { + argParser.addFlag(_failOnChangeFlag, hide: true); + } + + static const String _failOnChangeFlag = 'fail-on-change'; + + static const String _buildRunnerConfigName = 'excerpt'; + // The name of the build_runner configuration file that will be in an example + // directory if the package is set up to use `code-excerpt`. + static const String _buildRunnerConfigFile = + 'build.$_buildRunnerConfigName.yaml'; + + // The relative directory path to put the extracted excerpt yaml files. + static const String _excerptOutputDir = 'excerpts'; + + // The filename to store the pre-modification copy of the pubspec. + static const String _originalPubspecFilename = + 'pubspec.plugin_tools_original.yaml'; + + @override + final String name = 'update-excerpts'; + + @override + final String description = 'Updates code excerpts in README.md files, based ' + 'on code from code files, via code-excerpt'; + + @override + Future runForPackage(RepositoryPackage package) async { + final Iterable configuredExamples = package + .getExamples() + .where((RepositoryPackage example) => + example.directory.childFile(_buildRunnerConfigFile).existsSync()); + + if (configuredExamples.isEmpty) { + return PackageResult.skip( + 'No $_buildRunnerConfigFile found in example(s).'); + } + + final Directory repoRoot = + packagesDir.fileSystem.directory((await gitDir).path); + + for (final RepositoryPackage example in configuredExamples) { + _addSubmoduleDependencies(example, repoRoot: repoRoot); + + try { + // Ensure that dependencies are available. + final int pubGetExitCode = await processRunner.runAndStream( + 'dart', ['pub', 'get'], + workingDir: example.directory); + if (pubGetExitCode != 0) { + return PackageResult.fail( + ['Unable to get script dependencies']); + } + + // Update the excerpts. + if (!await _extractSnippets(example)) { + return PackageResult.fail(['Unable to extract excerpts']); + } + if (!await _injectSnippets(example, targetPackage: package)) { + return PackageResult.fail(['Unable to inject excerpts']); + } + } finally { + // Clean up the pubspec changes and extracted excerpts directory. + _undoPubspecChanges(example); + final Directory excerptDirectory = + example.directory.childDirectory(_excerptOutputDir); + if (excerptDirectory.existsSync()) { + excerptDirectory.deleteSync(recursive: true); + } + } + } + + if (getBoolArg(_failOnChangeFlag)) { + final String? stateError = await _validateRepositoryState(); + if (stateError != null) { + printError('README.md is out of sync with its source excerpts.\n\n' + 'If you edited code in README.md directly, you should instead edit ' + 'the example source files. If you edited source files, run the ' + 'repository tooling\'s "$name" command on this package, and update ' + 'your PR with the resulting changes.'); + return PackageResult.fail([stateError]); + } + } + + return PackageResult.success(); + } + + /// Runs the extraction step to create the excerpt files for the given + /// example, returning true on success. + Future _extractSnippets(RepositoryPackage example) async { + final int exitCode = await processRunner.runAndStream( + 'dart', + [ + 'run', + 'build_runner', + 'build', + '--config', + _buildRunnerConfigName, + '--output', + _excerptOutputDir, + '--delete-conflicting-outputs', + ], + workingDir: example.directory); + return exitCode == 0; + } + + /// Runs the injection step to update [targetPackage]'s README with the latest + /// excerpts from [example], returning true on success. + Future _injectSnippets( + RepositoryPackage example, { + required RepositoryPackage targetPackage, + }) async { + final String relativeReadmePath = + getRelativePosixPath(targetPackage.readmeFile, from: example.directory); + final int exitCode = await processRunner.runAndStream( + 'dart', + [ + 'run', + 'code_excerpt_updater', + '--write-in-place', + '--yaml', + '--no-escape-ng-interpolation', + relativeReadmePath, + ], + workingDir: example.directory); + return exitCode == 0; + } + + /// Adds `code_excerpter` and `code_excerpt_updater` to [package]'s + /// `dev_dependencies` using path-based references to the submodule copies. + /// + /// This is done on the fly rather than being checked in so that: + /// - Just building examples don't require everyone to check out submodules. + /// - Examples can be analyzed/built even on versions of Flutter that these + /// submodules do not support. + void _addSubmoduleDependencies(RepositoryPackage package, + {required Directory repoRoot}) { + final String pubspecContents = package.pubspecFile.readAsStringSync(); + // Save aside a copy of the current pubspec state. This allows restoration + // to the previous state regardless of its git status at the time the script + // ran. + package.directory + .childFile(_originalPubspecFilename) + .writeAsStringSync(pubspecContents); + + // Update the actual pubspec. + final YamlEditor editablePubspec = YamlEditor(pubspecContents); + const String devDependenciesKey = 'dev_dependencies'; + final YamlNode root = editablePubspec.parseAt([]); + // Ensure that there's a `dev_dependencies` entry to update. + if ((root as YamlMap)[devDependenciesKey] == null) { + editablePubspec.update(['dev_dependencies'], YamlMap()); + } + final Set submoduleDependencies = { + 'code_excerpter', + 'code_excerpt_updater', + }; + final String relativeRootPath = + getRelativePosixPath(repoRoot, from: package.directory); + for (final String dependency in submoduleDependencies) { + editablePubspec.update([ + devDependenciesKey, + dependency + ], { + 'path': '$relativeRootPath/site-shared/packages/$dependency' + }); + } + package.pubspecFile.writeAsStringSync(editablePubspec.toString()); + } + + /// Restores the version of the pubspec that was present before running + /// [_addSubmoduleDependencies]. + void _undoPubspecChanges(RepositoryPackage package) { + package.directory + .childFile(_originalPubspecFilename) + .renameSync(package.pubspecFile.path); + } + + /// Checks the git state, returning an error string unless nothing has + /// changed. + Future _validateRepositoryState() async { + final io.ProcessResult modifiedFiles = await processRunner.run( + 'git', + ['ls-files', '--modified'], + workingDir: packagesDir, + logOnError: true, + ); + if (modifiedFiles.exitCode != 0) { + return 'Unable to determine local file state'; + } + + final String stdout = modifiedFiles.stdout as String; + return stdout.trim().isEmpty ? null : 'Snippets are out of sync'; + } +} diff --git a/script/tool/pubspec.yaml b/script/tool/pubspec.yaml index 14b22a16e3..9f9910f934 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.2+1 +version: 0.8.3 dependencies: args: ^2.1.0 @@ -21,6 +21,7 @@ dependencies: test: ^1.17.3 uuid: ^3.0.4 yaml: ^3.1.0 + yaml_edit: ^2.0.2 dev_dependencies: build_runner: ^2.0.3 diff --git a/script/tool/test/format_command_test.dart b/script/tool/test/format_command_test.dart index 2890c528e4..6c10a7dc32 100644 --- a/script/tool/test/format_command_test.dart +++ b/script/tool/test/format_command_test.dart @@ -448,7 +448,7 @@ void main() { ])); }); - test('fails if files are changed with --file-on-change', () async { + test('fails if files are changed with --fail-on-change', () async { const List files = [ 'linux/foo_plugin.cc', 'macos/Classes/Foo.h', diff --git a/script/tool/test/license_check_command_test.dart b/script/tool/test/license_check_command_test.dart index e97274afd0..efaf969c83 100644 --- a/script/tool/test/license_check_command_test.dart +++ b/script/tool/test/license_check_command_test.dart @@ -7,24 +7,35 @@ import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; import 'package:flutter_plugin_tools/src/license_check_command.dart'; +import 'package:mockito/mockito.dart'; +import 'package:platform/platform.dart'; import 'package:test/test.dart'; +import 'common/plugin_command_test.mocks.dart'; +import 'mocks.dart'; import 'util.dart'; void main() { - group('$LicenseCheckCommand', () { + group('LicenseCheckCommand', () { late CommandRunner runner; late FileSystem fileSystem; + late Platform platform; late Directory root; setUp(() { fileSystem = MemoryFileSystem(); + platform = MockPlatformWithSeparator(); final Directory packagesDir = fileSystem.currentDirectory.childDirectory('packages'); root = packagesDir.parent; + final MockGitDir gitDir = MockGitDir(); + when(gitDir.path).thenReturn(packagesDir.parent.path); + final LicenseCheckCommand command = LicenseCheckCommand( packagesDir, + platform: platform, + gitDir: gitDir, ); runner = CommandRunner('license_test', 'Test for $LicenseCheckCommand'); @@ -123,6 +134,33 @@ void main() { } }); + test('ignores submodules', () async { + const String submoduleName = 'a_submodule'; + + final File submoduleSpec = root.childFile('.gitmodules'); + submoduleSpec.writeAsStringSync(''' +[submodule "$submoduleName"] + path = $submoduleName + url = https://github.com/foo/$submoduleName +'''); + + const List submoduleFiles = [ + '$submoduleName/foo.dart', + '$submoduleName/a/b/bar.dart', + '$submoduleName/LICENSE', + ]; + for (final String filePath in submoduleFiles) { + root.childFile(filePath).createSync(recursive: true); + } + + final List output = + await runCapturingPrint(runner, ['license-check']); + + for (final String filePath in submoduleFiles) { + expect(output, isNot(contains('Checking $filePath'))); + } + }); + test('passes if all checked files have license blocks', () async { final File checked = root.childFile('checked.cc'); checked.createSync(); @@ -509,6 +547,11 @@ void main() { }); } +class MockPlatformWithSeparator extends MockPlatform { + @override + String get pathSeparator => isWindows ? r'\' : '/'; +} + const String _correctLicenseFileText = ''' Copyright 2013 The Flutter Authors. All rights reserved. diff --git a/script/tool/test/update_excerpts_command_test.dart b/script/tool/test/update_excerpts_command_test.dart new file mode 100644 index 0000000000..30189cf23a --- /dev/null +++ b/script/tool/test/update_excerpts_command_test.dart @@ -0,0 +1,284 @@ +// 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 'dart:io' as io; + +import 'package:args/command_runner.dart'; +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/common/repository_package.dart'; +import 'package:flutter_plugin_tools/src/update_excerpts_command.dart'; +import 'package:mockito/mockito.dart'; +import 'package:test/test.dart'; + +import 'common/plugin_command_test.mocks.dart'; +import 'mocks.dart'; +import 'util.dart'; + +void main() { + late FileSystem fileSystem; + late Directory packagesDir; + late RecordingProcessRunner processRunner; + late CommandRunner runner; + + setUp(() { + fileSystem = MemoryFileSystem(); + packagesDir = createPackagesDirectory(fileSystem: fileSystem); + final MockGitDir gitDir = MockGitDir(); + when(gitDir.path).thenReturn(packagesDir.parent.path); + processRunner = RecordingProcessRunner(); + final UpdateExcerptsCommand command = UpdateExcerptsCommand( + packagesDir, + processRunner: processRunner, + platform: MockPlatform(), + gitDir: gitDir, + ); + + runner = CommandRunner( + 'update_excerpts_command', 'Test for update_excerpts_command'); + runner.addCommand(command); + }); + + test('runs pub get before running scripts', () async { + final Directory package = createFakePlugin('a_package', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + final Directory example = package.childDirectory('example'); + + await runCapturingPrint(runner, ['update-excerpts']); + + expect( + processRunner.recordedCalls, + containsAll([ + ProcessCall('dart', const ['pub', 'get'], example.path), + ProcessCall( + 'dart', + const [ + 'run', + 'build_runner', + 'build', + '--config', + 'excerpt', + '--output', + 'excerpts', + '--delete-conflicting-outputs', + ], + example.path), + ])); + }); + + test('runs when config is present', () async { + final Directory package = createFakePlugin('a_package', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + final Directory example = package.childDirectory('example'); + + final List output = + await runCapturingPrint(runner, ['update-excerpts']); + + expect( + processRunner.recordedCalls, + containsAll([ + ProcessCall( + 'dart', + const [ + 'run', + 'build_runner', + 'build', + '--config', + 'excerpt', + '--output', + 'excerpts', + '--delete-conflicting-outputs', + ], + example.path), + ProcessCall( + 'dart', + const [ + 'run', + 'code_excerpt_updater', + '--write-in-place', + '--yaml', + '--no-escape-ng-interpolation', + '../README.md', + ], + example.path), + ])); + + expect( + output, + containsAllInOrder([ + contains('Ran for 1 package(s)'), + ])); + }); + + test('skips when no config is present', () async { + createFakePlugin('a_package', packagesDir); + + final List output = + await runCapturingPrint(runner, ['update-excerpts']); + + expect(processRunner.recordedCalls, isEmpty); + + expect( + output, + containsAllInOrder([ + contains('Skipped 1 package(s)'), + ])); + }); + + test('restores pubspec even if running the script fails', () async { + final Directory package = createFakePlugin('a_package', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + + processRunner.mockProcessesForExecutable['dart'] = [ + MockProcess(exitCode: 1), // dart pub get + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['update-excerpts'], errorHandler: (Error e) { + commandError = e; + }); + + // Check that it's definitely a failure in a step between making the changes + // and restoring the original. + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('The following packages had errors:'), + contains('a_package:\n' + ' Unable to get script dependencies') + ])); + + final String examplePubspecContent = RepositoryPackage(package) + .getExamples() + .first + .pubspecFile + .readAsStringSync(); + expect(examplePubspecContent, isNot(contains('code_excerpter'))); + expect(examplePubspecContent, isNot(contains('code_excerpt_updater'))); + }); + + test('fails if pub get fails', () async { + createFakePlugin('a_package', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + + processRunner.mockProcessesForExecutable['dart'] = [ + MockProcess(exitCode: 1), // dart pub get + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['update-excerpts'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('The following packages had errors:'), + contains('a_package:\n' + ' Unable to get script dependencies') + ])); + }); + + test('fails if extraction fails', () async { + createFakePlugin('a_package', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + + processRunner.mockProcessesForExecutable['dart'] = [ + MockProcess(exitCode: 0), // dart pub get + MockProcess(exitCode: 1), // dart run build_runner ... + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['update-excerpts'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('The following packages had errors:'), + contains('a_package:\n' + ' Unable to extract excerpts') + ])); + }); + + test('fails if injection fails', () async { + createFakePlugin('a_package', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + + processRunner.mockProcessesForExecutable['dart'] = [ + MockProcess(exitCode: 0), // dart pub get + MockProcess(exitCode: 0), // dart run build_runner ... + MockProcess(exitCode: 1), // dart run code_excerpt_updater ... + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['update-excerpts'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('The following packages had errors:'), + contains('a_package:\n' + ' Unable to inject excerpts') + ])); + }); + + test('fails if files are changed with --fail-on-change', () async { + createFakePlugin('a_plugin', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + + const String changedFilePath = 'packages/a_plugin/linux/foo_plugin.cc'; + processRunner.mockProcessesForExecutable['git'] = [ + MockProcess(stdout: changedFilePath), + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['update-excerpts', '--fail-on-change'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('README.md is out of sync with its source excerpts'), + ])); + }); + + test('fails if git ls-files fails', () async { + createFakePlugin('a_plugin', packagesDir, + extraFiles: ['example/build.excerpt.yaml']); + + processRunner.mockProcessesForExecutable['git'] = [ + MockProcess(exitCode: 1) + ]; + Error? commandError; + final List output = await runCapturingPrint( + runner, ['update-excerpts', '--fail-on-change'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Unable to determine local file state'), + ])); + }); +}