Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit 54082e0

Browse files
[flutter_plugin_tools] Restructure version-check (#4111)
Combines the two different aspects of version-checking into a single looping structure based on plugins, using the new base command, rather than one operating on plugins as controlled by the usual flags and the other operating on a git list of changed files. Also simplifies the determination of the new version by simply checking the file, rather than querying git for the HEAD state of the file. Tests setup is simplified since we no longer need to set up nearly as much fake `git` output. Minor changes to base commands: - Move indentation up to PackageLoopingCommand so that it is consistent across commands - Add a new post-loop command for any cleanup, which is needed by version-check - Change the way the GitDir instance is managed by the base PluginCommand, so that it's always cached even when not overridden, to reduce duplicate work and code. Part of flutter/flutter#83413
1 parent 62e8380 commit 54082e0

7 files changed

+282
-380
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ void printSuccess(String successMessage) {
5858
print(Colorize(successMessage)..green());
5959
}
6060

61+
/// Prints `warningMessage` in yellow.
62+
///
63+
/// Warnings are not surfaced in CI summaries, so this is only useful for
64+
/// highlighting something when someone is already looking though the log
65+
/// messages. DO NOT RELY on someone noticing a warning; instead, use it for
66+
/// things that might be useful to someone debugging an unexpected result.
67+
void printWarning(String warningMessage) {
68+
print(Colorize(warningMessage)..yellow());
69+
}
70+
6171
/// Prints `errorMessage` in red.
6272
void printError(String errorMessage) {
6373
print(Colorize(errorMessage)..red());

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ abstract class PackageLoopingCommand extends PluginCommand {
3535
/// in the final summary. An empty list indicates success.
3636
Future<List<String>> runForPackage(Directory package);
3737

38+
/// Called during [run] after all calls to [runForPackage]. This provides an
39+
/// opportunity to do any cleanup of run-level state.
40+
Future<void> completeRun() async {}
41+
3842
/// Whether or not the output (if any) of [runForPackage] is long, or short.
3943
///
4044
/// This changes the logging that happens at the start of each package's
@@ -99,6 +103,9 @@ abstract class PackageLoopingCommand extends PluginCommand {
99103
return packageName;
100104
}
101105

106+
/// The suggested indentation for printed output.
107+
String get indentation => hasLongOutput ? '' : ' ';
108+
102109
// ----------------------------------------
103110

104111
@override
@@ -115,6 +122,8 @@ abstract class PackageLoopingCommand extends PluginCommand {
115122
results[package] = await runForPackage(package);
116123
}
117124

125+
completeRun();
126+
118127
// If there were any errors reported, summarize them and exit.
119128
if (results.values.any((List<String> failures) => failures.isNotEmpty)) {
120129
const String indentation = ' ';

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

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ abstract class PluginCommand extends Command<void> {
2020
PluginCommand(
2121
this.packagesDir, {
2222
this.processRunner = const ProcessRunner(),
23-
this.gitDir,
24-
}) {
23+
GitDir? gitDir,
24+
}) : _gitDir = gitDir {
2525
argParser.addMultiOption(
2626
_pluginsArg,
2727
splitCommas: true,
@@ -76,10 +76,11 @@ abstract class PluginCommand extends Command<void> {
7676
/// This can be overridden for testing.
7777
final ProcessRunner processRunner;
7878

79-
/// The git directory to use. By default it uses the parent directory.
79+
/// The git directory to use. If unset, [gitDir] populates it from the
80+
/// packages directory's enclosing repository.
8081
///
8182
/// This can be mocked for testing.
82-
final GitDir? gitDir;
83+
GitDir? _gitDir;
8384

8485
int? _shardIndex;
8586
int? _shardCount;
@@ -100,6 +101,26 @@ abstract class PluginCommand extends Command<void> {
100101
return _shardCount!;
101102
}
102103

104+
/// Returns the [GitDir] containing [packagesDir].
105+
Future<GitDir> get gitDir async {
106+
GitDir? gitDir = _gitDir;
107+
if (gitDir != null) {
108+
return gitDir;
109+
}
110+
111+
// Ensure there are no symlinks in the path, as it can break
112+
// GitDir's allowSubdirectory:true.
113+
final String packagesPath = packagesDir.resolveSymbolicLinksSync();
114+
if (!await GitDir.isGitDir(packagesPath)) {
115+
printError('$packagesPath is not a valid Git repository.');
116+
throw ToolExit(2);
117+
}
118+
gitDir =
119+
await GitDir.fromExisting(packagesDir.path, allowSubdirectory: true);
120+
_gitDir = gitDir;
121+
return gitDir;
122+
}
123+
103124
/// Convenience accessor for boolean arguments.
104125
bool getBoolArg(String key) {
105126
return (argResults![key] as bool?) ?? false;
@@ -291,22 +312,10 @@ abstract class PluginCommand extends Command<void> {
291312
///
292313
/// Throws tool exit if [gitDir] nor root directory is a git directory.
293314
Future<GitVersionFinder> retrieveVersionFinder() async {
294-
final String rootDir = packagesDir.parent.absolute.path;
295315
final String baseSha = getStringArg(_kBaseSha);
296316

297-
GitDir? baseGitDir = gitDir;
298-
if (baseGitDir == null) {
299-
if (!await GitDir.isGitDir(rootDir)) {
300-
printError(
301-
'$rootDir is not a valid Git repository.',
302-
);
303-
throw ToolExit(2);
304-
}
305-
baseGitDir = await GitDir.fromExisting(rootDir);
306-
}
307-
308317
final GitVersionFinder gitVersionFinder =
309-
GitVersionFinder(baseGitDir, baseSha);
318+
GitVersionFinder(await gitDir, baseSha);
310319
return gitVersionFinder;
311320
}
312321

script/tool/lib/src/publish_plugin_command.dart

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,7 @@ class PublishPluginCommand extends PluginCommand {
149149
}
150150

151151
_print('Checking local repo...');
152-
// Ensure there are no symlinks in the path, as it can break
153-
// GitDir's allowSubdirectory:true.
154-
final String packagesPath = packagesDir.resolveSymbolicLinksSync();
155-
if (!await GitDir.isGitDir(packagesPath)) {
156-
_print('$packagesPath is not a valid Git repository.');
157-
throw ToolExit(1);
158-
}
159-
final GitDir baseGitDir = gitDir ??
160-
await GitDir.fromExisting(packagesPath, allowSubdirectory: true);
152+
final GitDir repository = await gitDir;
161153

162154
final bool shouldPushTag = getBoolArg(_pushTagsOption);
163155
_RemoteInfo? remote;
@@ -179,7 +171,7 @@ class PublishPluginCommand extends PluginCommand {
179171
bool successful;
180172
if (publishAllChanged) {
181173
successful = await _publishAllChangedPackages(
182-
baseGitDir: baseGitDir,
174+
baseGitDir: repository,
183175
remoteForTagPush: remote,
184176
);
185177
} else {

script/tool/lib/src/pubspec_check_command.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ class PubspecCheckCommand extends PackageLoopingCommand {
7070
File pubspecFile, {
7171
required String packageName,
7272
}) async {
73-
const String indentation = ' ';
7473
final String contents = pubspecFile.readAsStringSync();
7574
final Pubspec? pubspec = _tryParsePubspec(contents);
7675
if (pubspec == null) {

0 commit comments

Comments
 (0)