Skip to content

Commit aac5e95

Browse files
[flutter_tools] refactor packages_autoroller.dart script (#106580)
1 parent 28d271e commit aac5e95

File tree

5 files changed

+443
-29
lines changed

5 files changed

+443
-29
lines changed

dev/bots/analyze.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -1778,7 +1778,7 @@ const Set<String> kExecutableAllowlist = <String>{
17781778
'dev/bots/docs.sh',
17791779

17801780
'dev/conductor/bin/conductor',
1781-
'dev/conductor/bin/roll-packages',
1781+
'dev/conductor/bin/packages_autoroller',
17821782
'dev/conductor/core/lib/src/proto/compile_proto.sh',
17831783

17841784
'dev/customer_testing/ci.sh',

dev/conductor/bin/roll-packages dev/conductor/bin/packages_autoroller

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,4 @@ DART_BIN="$REPO_DIR/bin/dart"
4343
# Ensure pub get has been run in the repo before running the conductor
4444
(cd "$REPO_DIR/dev/conductor/core"; "$DART_BIN" pub get 1>&2)
4545

46-
exec "$DART_BIN" --enable-asserts "$REPO_DIR/dev/conductor/core/bin/roll_packages.dart" "$@"
46+
exec "$DART_BIN" --enable-asserts "$REPO_DIR/dev/conductor/core/bin/packages_autoroller.dart" "$@"

dev/conductor/core/bin/packages_autoroller.dart

+30-10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:conductor_core/conductor_core.dart';
99
import 'package:conductor_core/packages_autoroller.dart';
1010
import 'package:file/file.dart';
1111
import 'package:file/local.dart';
12+
import 'package:meta/meta.dart' show visibleForTesting;
1213
import 'package:platform/platform.dart';
1314
import 'package:process/process.dart';
1415

@@ -17,12 +18,21 @@ const String kGithubClient = 'github-client';
1718
const String kMirrorRemote = 'mirror-remote';
1819
const String kUpstreamRemote = 'upstream-remote';
1920

20-
Future<void> main(List<String> args) async {
21+
Future<void> main(List<String> args) {
22+
return run(args);
23+
}
24+
25+
@visibleForTesting
26+
Future<void> run(
27+
List<String> args, {
28+
FileSystem fs = const LocalFileSystem(),
29+
ProcessManager processManager = const LocalProcessManager(),
30+
}) async {
2131
final ArgParser parser = ArgParser();
2232
parser.addOption(
2333
kTokenOption,
24-
help: 'GitHub access token env variable name.',
25-
defaultsTo: 'GITHUB_TOKEN',
34+
help: 'Path to GitHub access token file.',
35+
mandatory: true,
2636
);
2737
parser.addOption(
2838
kGithubClient,
@@ -56,12 +66,17 @@ ${parser.usage}
5666

5767
final String mirrorUrl = results[kMirrorRemote]! as String;
5868
final String upstreamUrl = results[kUpstreamRemote]! as String;
59-
const Platform platform = LocalPlatform();
60-
final String tokenName = results[kTokenOption]! as String;
61-
final String? token = platform.environment[tokenName];
62-
if (token == null || token.isEmpty) {
63-
throw FormatException(
64-
'Tried to read a GitHub access token from env variable \$$tokenName but it was undefined or empty',
69+
final String tokenPath = results[kTokenOption]! as String;
70+
final File tokenFile = fs.file(tokenPath);
71+
if (!tokenFile.existsSync()) {
72+
throw ArgumentError(
73+
'Provided token path $tokenPath but no file exists at ${tokenFile.absolute.path}',
74+
);
75+
}
76+
final String token = tokenFile.readAsStringSync().trim();
77+
if (token.isEmpty) {
78+
throw ArgumentError(
79+
'Tried to read a GitHub access token from file ${tokenFile.path} but it was empty',
6580
);
6681
}
6782

@@ -76,7 +91,7 @@ ${parser.usage}
7691
githubClient: results[kGithubClient] as String? ?? 'gh',
7792
orgName: _parseOrgName(mirrorUrl),
7893
token: token,
79-
processManager: const LocalProcessManager(),
94+
processManager: processManager,
8095
).roll();
8196
}
8297

@@ -126,3 +141,8 @@ Directory get _localFlutterRoot {
126141
);
127142
return fileSystem.directory(checkoutsDirname);
128143
}
144+
145+
@visibleForTesting
146+
void validateTokenFile(String filePath, [FileSystem fs = const LocalFileSystem()]) {
147+
148+
}

dev/conductor/core/lib/src/packages_autoroller.dart

+88-14
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:process/process.dart';
1010
import 'git.dart';
1111
import 'globals.dart';
1212
import 'repository.dart';
13+
import 'stdio.dart';
1314

1415
/// A service for rolling the SDK's pub packages to latest and open a PR upstream.
1516
class PackageAutoroller {
@@ -19,7 +20,10 @@ class PackageAutoroller {
1920
required this.framework,
2021
required this.orgName,
2122
required this.processManager,
23+
this.githubUsername = 'fluttergithubbot',
24+
Stdio? stdio,
2225
}) {
26+
this.stdio = stdio ?? VerboseStdio.local();
2327
if (token.trim().isEmpty) {
2428
throw Exception('empty token!');
2529
}
@@ -31,12 +35,16 @@ class PackageAutoroller {
3135
}
3236
}
3337

38+
late final Stdio stdio;
39+
3440
final FrameworkRepository framework;
3541
final ProcessManager processManager;
3642

3743
/// Path to GitHub CLI client.
3844
final String githubClient;
3945

46+
final String githubUsername;
47+
4048
/// GitHub API access token.
4149
final String token;
4250

@@ -63,23 +71,46 @@ This PR was generated by `flutter update-packages --force-upgrade`.
6371
return name(x);
6472
})();
6573

74+
void log(String message) {
75+
stdio.printStatus(_redactToken(message));
76+
}
77+
6678
/// Name of the GitHub organization to push the feature branch to.
6779
final String orgName;
6880

6981
Future<void> roll() async {
70-
await authLogin();
71-
await updatePackages();
72-
await pushBranch();
73-
await createPr(
74-
repository: await framework.checkoutDirectory,
75-
);
76-
await authLogout();
82+
try {
83+
await authLogin();
84+
final bool openPrAlready = await hasOpenPrs();
85+
if (openPrAlready) {
86+
// Don't open multiple roll PRs.
87+
return;
88+
}
89+
final bool didUpdate = await updatePackages();
90+
if (!didUpdate) {
91+
log('Packages are already at latest.');
92+
return;
93+
}
94+
await pushBranch();
95+
await createPr(repository: await framework.checkoutDirectory);
96+
await authLogout();
97+
} on Exception catch (exception) {
98+
final String message = _redactToken(exception.toString());
99+
throw Exception('${exception.runtimeType}: $message');
100+
}
77101
}
78102

79-
Future<void> updatePackages({
103+
// Ensure we don't leak the GitHub token in exception messages
104+
String _redactToken(String message) => message.replaceAll(token, '[GitHub TOKEN]');
105+
106+
/// Attempt to update all pub packages.
107+
///
108+
/// Will return whether or not any changes were made.
109+
Future<bool> updatePackages({
80110
bool verbose = true,
81-
String author = 'flutter-packages-autoroller <[email protected]>'
82111
}) async {
112+
final String author = '$githubUsername <$githubUsername@gmail.com>';
113+
83114
await framework.newBranch(await featureBranchName);
84115
final io.Process flutterProcess = await framework.streamFlutter(<String>[
85116
if (verbose) '--verbose',
@@ -90,18 +121,26 @@ This PR was generated by `flutter update-packages --force-upgrade`.
90121
if (exitCode != 0) {
91122
throw ConductorException('Failed to update packages with exit code $exitCode');
92123
}
124+
// If the git checkout is clean, then pub packages are already at latest that cleanly resolve.
125+
if (await framework.gitCheckoutClean()) {
126+
return false;
127+
}
93128
await framework.commit(
94129
'roll packages',
95130
addFirst: true,
96131
author: author,
97132
);
133+
return true;
98134
}
99135

100136
Future<void> pushBranch() async {
137+
final String projectName = framework.mirrorRemote!.url.split(r'/').last;
138+
// Encode the token into the remote URL for authentication to work
139+
final String remote = 'https://$token@$hostname/$orgName/$projectName';
101140
await framework.pushRef(
102141
fromRef: await featureBranchName,
103142
toRef: await featureBranchName,
104-
remote: framework.mirrorRemote!.url,
143+
remote: remote,
105144
);
106145
}
107146

@@ -123,7 +162,7 @@ This PR was generated by `flutter update-packages --force-upgrade`.
123162
'https',
124163
'--with-token',
125164
],
126-
stdin: token,
165+
stdin: '$token\n',
127166
);
128167
}
129168

@@ -151,6 +190,8 @@ This PR was generated by `flutter update-packages --force-upgrade`.
151190
'$orgName:${await featureBranchName}',
152191
'--base',
153192
base,
193+
'--label',
194+
'tool',
154195
if (draft)
155196
'--draft',
156197
],
@@ -165,13 +206,16 @@ This PR was generated by `flutter update-packages --force-upgrade`.
165206
]);
166207
}
167208

168-
Future<void> cli(
209+
/// Run a sub-process with the GitHub CLI client.
210+
///
211+
/// Will return STDOUT of the sub-process.
212+
Future<String> cli(
169213
List<String> args, {
170214
bool allowFailure = false,
171215
String? stdin,
172216
String? workingDirectory,
173217
}) async {
174-
print('Executing "$githubClient ${args.join(' ')}" in $workingDirectory');
218+
log('Executing "$githubClient ${args.join(' ')}" in $workingDirectory');
175219
final io.Process process = await processManager.start(
176220
<String>[githubClient, ...args],
177221
workingDirectory: workingDirectory,
@@ -203,6 +247,36 @@ This PR was generated by `flutter update-packages --force-upgrade`.
203247
args,
204248
);
205249
}
206-
print(stdout);
250+
log(stdout);
251+
return stdout;
252+
}
253+
254+
Future<bool> hasOpenPrs() async {
255+
// gh pr list --author christopherfujino --repo flutter/flutter --state open --json number
256+
final String openPrString = await cli(<String>[
257+
'pr',
258+
'list',
259+
'--author',
260+
githubUsername,
261+
'--repo',
262+
'flutter/flutter',
263+
'--state',
264+
'open',
265+
// We are only interested in pub rolls, not devicelab flaky PRs
266+
'--label',
267+
'tool',
268+
// Return structured JSON with the PR numbers of open PRs
269+
'--json',
270+
'number',
271+
]);
272+
273+
// This will be an array of objects, one for each open PR.
274+
final List<Object?> openPrs = json.decode(openPrString) as List<Object?>;
275+
276+
if (openPrs.isNotEmpty) {
277+
log('$githubUsername already has open tool PRs:\n$openPrs');
278+
return true;
279+
}
280+
return false;
207281
}
208282
}

0 commit comments

Comments
 (0)