Skip to content

Commit 6919432

Browse files
[flutter_plugin_tool] Move branch-switching logic from tool_runner.sh to tool (#4268)
Eliminates the remaining logic from tool_runner.sh, completing the goal of migrating repository tooling off of bash (both to make maintenance easier, and to better support Windows both locally and in CI). Its branch-based logic is now part of the tool itself, via a new `--packages-for-branch` flag (which is hidden in help since it's only useful for CI). Part of flutter/flutter#86113
1 parent c7e7f65 commit 6919432

File tree

4 files changed

+260
-83
lines changed

4 files changed

+260
-83
lines changed

script/tool/CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
## NEXT
1+
## 0.6.0
22

33
- Added Android native integration test support to `native-test`.
44
- Added a new `android-lint` command to lint Android plugin native code.
@@ -10,6 +10,8 @@
1010
`--no-push-flags`. Releases now always tag and push.
1111
- **Breaking change**: `publish`'s `--package` flag has been replaced with the
1212
`--packages` flag used by most other packages.
13+
- **Breaking change** Passing both `--run-on-changed-packages` and `--packages`
14+
is now an error; previously it the former would be ignored.
1315

1416
## 0.5.0
1517

script/tool/lib/src/common/plugin_command.dart

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'dart:io' as io;
56
import 'dart:math';
67

78
import 'package:args/command_runner.dart';
@@ -72,11 +73,18 @@ abstract class PluginCommand extends Command<void> {
7273
);
7374
argParser.addFlag(_runOnChangedPackagesArg,
7475
help: 'Run the command on changed packages/plugins.\n'
75-
'If the $_packagesArg is specified, this flag is ignored.\n'
7676
'If no packages have changed, or if there have been changes that may\n'
7777
'affect all packages, the command runs on all packages.\n'
7878
'The packages excluded with $_excludeArg is also excluded even if changed.\n'
79-
'See $_kBaseSha if a custom base is needed to determine the diff.');
79+
'See $_kBaseSha if a custom base is needed to determine the diff.\n\n'
80+
'Cannot be combined with $_packagesArg.\n');
81+
argParser.addFlag(_packagesForBranchArg,
82+
help:
83+
'This runs on all packages (equivalent to no package selection flag)\n'
84+
'on master, and behaves like --run-on-changed-packages on any other branch.\n\n'
85+
'Cannot be combined with $_packagesArg.\n\n'
86+
'This is intended for use in CI.\n',
87+
hide: true);
8088
argParser.addOption(_kBaseSha,
8189
help: 'The base sha used to determine git diff. \n'
8290
'This is useful when $_runOnChangedPackagesArg is specified.\n'
@@ -89,6 +97,7 @@ abstract class PluginCommand extends Command<void> {
8997
static const String _shardCountArg = 'shardCount';
9098
static const String _excludeArg = 'exclude';
9199
static const String _runOnChangedPackagesArg = 'run-on-changed-packages';
100+
static const String _packagesForBranchArg = 'packages-for-branch';
92101
static const String _kBaseSha = 'base-sha';
93102

94103
/// The directory containing the plugin packages.
@@ -266,15 +275,50 @@ abstract class PluginCommand extends Command<void> {
266275
/// is a sibling of the packages directory. This is used for a small number
267276
/// of packages in the flutter/packages repository.
268277
Stream<PackageEnumerationEntry> _getAllPackages() async* {
278+
final Set<String> packageSelectionFlags = <String>{
279+
_packagesArg,
280+
_runOnChangedPackagesArg,
281+
_packagesForBranchArg,
282+
};
283+
if (packageSelectionFlags
284+
.where((String flag) => argResults!.wasParsed(flag))
285+
.length >
286+
1) {
287+
printError('Only one of --$_packagesArg, --$_runOnChangedPackagesArg, or '
288+
'--$_packagesForBranchArg can be provided.');
289+
throw ToolExit(exitInvalidArguments);
290+
}
291+
269292
Set<String> plugins = Set<String>.from(getStringListArg(_packagesArg));
270293

294+
final bool runOnChangedPackages;
295+
if (getBoolArg(_runOnChangedPackagesArg)) {
296+
runOnChangedPackages = true;
297+
} else if (getBoolArg(_packagesForBranchArg)) {
298+
final String? branch = await _getBranch();
299+
if (branch == null) {
300+
printError('Unabled to determine branch; --$_packagesForBranchArg can '
301+
'only be used in a git repository.');
302+
throw ToolExit(exitInvalidArguments);
303+
} else {
304+
runOnChangedPackages = branch != 'master';
305+
// Log the mode for auditing what was intended to run.
306+
print('--$_packagesForBranchArg: running on '
307+
'${runOnChangedPackages ? 'changed' : 'all'} packages');
308+
}
309+
} else {
310+
runOnChangedPackages = false;
311+
}
312+
271313
final Set<String> excludedPluginNames = getExcludedPackageNames();
272314

273-
final bool runOnChangedPackages = getBoolArg(_runOnChangedPackagesArg);
274-
if (plugins.isEmpty &&
275-
runOnChangedPackages &&
276-
!(await _changesRequireFullTest())) {
277-
plugins = await _getChangedPackages();
315+
if (runOnChangedPackages) {
316+
final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
317+
final List<String> changedFiles =
318+
await gitVersionFinder.getChangedFiles();
319+
if (!_changesRequireFullTest(changedFiles)) {
320+
plugins = _getChangedPackages(changedFiles);
321+
}
278322
}
279323

280324
final Directory thirdPartyPackagesDirectory = packagesDir.parent
@@ -374,15 +418,13 @@ abstract class PluginCommand extends Command<void> {
374418
return gitVersionFinder;
375419
}
376420

377-
// Returns packages that have been changed relative to the git base.
378-
Future<Set<String>> _getChangedPackages() async {
379-
final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
380-
381-
final List<String> allChangedFiles =
382-
await gitVersionFinder.getChangedFiles();
421+
// Returns packages that have been changed given a list of changed files.
422+
//
423+
// The paths must use POSIX separators (e.g., as provided by git output).
424+
Set<String> _getChangedPackages(List<String> changedFiles) {
383425
final Set<String> packages = <String>{};
384-
for (final String path in allChangedFiles) {
385-
final List<String> pathComponents = path.split('/');
426+
for (final String path in changedFiles) {
427+
final List<String> pathComponents = p.posix.split(path);
386428
final int packagesIndex =
387429
pathComponents.indexWhere((String element) => element == 'packages');
388430
if (packagesIndex != -1) {
@@ -398,11 +440,19 @@ abstract class PluginCommand extends Command<void> {
398440
return packages;
399441
}
400442

443+
Future<String?> _getBranch() async {
444+
final io.ProcessResult branchResult = await (await gitDir).runCommand(
445+
<String>['rev-parse', '--abbrev-ref', 'HEAD'],
446+
throwOnError: false);
447+
if (branchResult.exitCode != 0) {
448+
return null;
449+
}
450+
return (branchResult.stdout as String).trim();
451+
}
452+
401453
// Returns true if one or more files changed that have the potential to affect
402454
// any plugin (e.g., CI script changes).
403-
Future<bool> _changesRequireFullTest() async {
404-
final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
405-
455+
bool _changesRequireFullTest(List<String> changedFiles) {
406456
const List<String> specialFiles = <String>[
407457
'.ci.yaml', // LUCI config.
408458
'.cirrus.yml', // Cirrus config.
@@ -417,9 +467,7 @@ abstract class PluginCommand extends Command<void> {
417467
// check below is done via string prefixing.
418468
assert(specialDirectories.every((String dir) => dir.endsWith('/')));
419469

420-
final List<String> allChangedFiles =
421-
await gitVersionFinder.getChangedFiles();
422-
return allChangedFiles.any((String path) =>
470+
return changedFiles.any((String path) =>
423471
specialFiles.contains(path) ||
424472
specialDirectories.any((String dir) => path.startsWith(dir)));
425473
}

script/tool/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: flutter_plugin_tools
22
description: Productivity utils for flutter/plugins and flutter/packages
33
repository: https://github.com/flutter/plugins/tree/master/script/tool
4-
version: 0.5.0
4+
version: 0.6.0
55

66
dependencies:
77
args: ^2.1.0

0 commit comments

Comments
 (0)