[flutter_plugin_tools] Migrate publish and version checks to NNBD (#4037)

Migrates publish-check and version-check commands to NNBD.

Reworks the version-check flow so that it's more consistent with the other commands: instead of immediately exiting on failure, it checks all plugins, gathers failures, and summarizes all failures at the end. This ensures that we don't have failures in one package temporarily masked by failures in another, so PRs don't need to go through as many check cycles.

Part of https://github.com/flutter/flutter/issues/81912
This commit is contained in:
stuartmorgan
2021-06-11 16:10:41 -07:00
committed by GitHub
parent cb92e5d416
commit 038c1796b0
7 changed files with 135 additions and 119 deletions

View File

@ -165,11 +165,10 @@ bool isLinuxPlugin(FileSystemEntity entity) {
return pluginSupportsPlatform(kPlatformFlagLinux, entity);
}
/// Throws a [ToolExit] with `exitCode` and log the `errorMessage` in red.
void printErrorAndExit({required String errorMessage, int exitCode = 1}) {
/// Prints `errorMessage` in red.
void printError(String errorMessage) {
final Colorize redError = Colorize(errorMessage)..red();
print(redError);
throw ToolExit(exitCode);
}
/// Error thrown when a command needs to exit with a non-zero exit code.
@ -456,9 +455,10 @@ abstract class PluginCommand extends Command<void> {
GitDir? baseGitDir = gitDir;
if (baseGitDir == null) {
if (!await GitDir.isGitDir(rootDir)) {
printErrorAndExit(
errorMessage: '$rootDir is not a valid Git repository.',
exitCode: 2);
printError(
'$rootDir is not a valid Git repository.',
);
throw ToolExit(2);
}
baseGitDir = await GitDir.fromExisting(rootDir);
}
@ -599,7 +599,7 @@ class ProcessRunner {
/// passing [workingDir].
///
/// Returns the started [io.Process].
Future<io.Process?> start(String executable, List<String> args,
Future<io.Process> start(String executable, List<String> args,
{Directory? workingDirectory}) async {
final io.Process process = await io.Process.start(executable, args,
workingDirectory: workingDirectory?.path);
@ -641,12 +641,12 @@ class PubVersionFinder {
if (response.statusCode == 404) {
return PubVersionFinderResponse(
versions: null,
versions: <Version>[],
result: PubVersionFinderResult.noPackageFound,
httpResponse: response);
} else if (response.statusCode != 200) {
return PubVersionFinderResponse(
versions: null,
versions: <Version>[],
result: PubVersionFinderResult.fail,
httpResponse: response);
}
@ -666,9 +666,12 @@ class PubVersionFinder {
/// Represents a response for [PubVersionFinder].
class PubVersionFinderResponse {
/// Constructor.
PubVersionFinderResponse({this.versions, this.result, this.httpResponse}) {
if (versions != null && versions!.isNotEmpty) {
versions!.sort((Version a, Version b) {
PubVersionFinderResponse(
{required this.versions,
required this.result,
required this.httpResponse}) {
if (versions.isNotEmpty) {
versions.sort((Version a, Version b) {
// TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize].
// https://github.com/flutter/flutter/issues/82222
return b.compareTo(a);
@ -680,13 +683,13 @@ class PubVersionFinderResponse {
///
/// This is sorted by largest to smallest, so the first element in the list is the largest version.
/// Might be `null` if the [result] is not [PubVersionFinderResult.success].
final List<Version>? versions;
final List<Version> versions;
/// The result of the version finder.
final PubVersionFinderResult? result;
final PubVersionFinderResult result;
/// The response object of the http request.
final http.Response? httpResponse;
final http.Response httpResponse;
}
/// An enum representing the result of [PubVersionFinder].

View File

@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// @dart=2.9
import 'dart:async';
import 'dart:convert';
import 'dart:io' as io;
@ -11,7 +9,6 @@ import 'dart:io' as io;
import 'package:colorize/colorize.dart';
import 'package:file/file.dart';
import 'package:http/http.dart' as http;
import 'package:meta/meta.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:pubspec_parse/pubspec_parse.dart';
@ -23,7 +20,7 @@ class PublishCheckCommand extends PluginCommand {
PublishCheckCommand(
Directory packagesDir, {
ProcessRunner processRunner = const ProcessRunner(),
this.httpClient,
http.Client? httpClient,
}) : _pubVersionFinder =
PubVersionFinder(httpClient: httpClient ?? http.Client()),
super(packagesDir, processRunner: processRunner) {
@ -65,9 +62,6 @@ class PublishCheckCommand extends PluginCommand {
final String description =
'Checks to make sure that a plugin *could* be published.';
/// The custom http client used to query versions on pub.
final http.Client httpClient;
final PubVersionFinder _pubVersionFinder;
// The output JSON when the _machineFlag is on.
@ -135,7 +129,7 @@ class PublishCheckCommand extends PluginCommand {
}
}
Pubspec _tryParsePubspec(Directory package) {
Pubspec? _tryParsePubspec(Directory package) {
final File pubspecFile = package.childFile('pubspec.yaml');
try {
@ -205,7 +199,7 @@ class PublishCheckCommand extends PluginCommand {
final String packageName = package.basename;
print('Checking that $packageName can be published.');
final Pubspec pubspec = _tryParsePubspec(package);
final Pubspec? pubspec = _tryParsePubspec(package);
if (pubspec == null) {
print('no pubspec');
return _PublishCheckResult._error;
@ -214,7 +208,7 @@ class PublishCheckCommand extends PluginCommand {
return _PublishCheckResult._published;
}
final Version version = pubspec.version;
final Version? version = pubspec.version;
final _PublishCheckResult alreadyPublishedResult =
await _checkIfAlreadyPublished(
packageName: packageName, version: version);
@ -238,29 +232,24 @@ class PublishCheckCommand extends PluginCommand {
// Check if `packageName` already has `version` published on pub.
Future<_PublishCheckResult> _checkIfAlreadyPublished(
{String packageName, Version version}) async {
{required String packageName, required Version? version}) async {
final PubVersionFinderResponse pubVersionFinderResponse =
await _pubVersionFinder.getPackageVersion(package: packageName);
_PublishCheckResult result;
switch (pubVersionFinderResponse.result) {
case PubVersionFinderResult.success:
result = pubVersionFinderResponse.versions.contains(version)
return pubVersionFinderResponse.versions.contains(version)
? _PublishCheckResult._published
: _PublishCheckResult._notPublished;
break;
case PubVersionFinderResult.fail:
print('''
Error fetching version on pub for $packageName.
HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode}
HTTP response: ${pubVersionFinderResponse.httpResponse.body}
''');
result = _PublishCheckResult._error;
break;
return _PublishCheckResult._error;
case PubVersionFinderResult.noPackageFound:
result = _PublishCheckResult._notPublished;
break;
return _PublishCheckResult._notPublished;
}
return result;
}
void _setStatus(String status) {
@ -272,7 +261,7 @@ HTTP response: ${pubVersionFinderResponse.httpResponse.body}
return const JsonEncoder.withIndent(' ').convert(_machineOutput);
}
void _printImportantStatusMessage(String message, {@required bool isError}) {
void _printImportantStatusMessage(String message, {required bool isError}) {
final String statusMessage = '${isError ? 'ERROR' : 'SUCCESS'}: $message';
if (getBoolArg(_machineFlag)) {
print(statusMessage);

View File

@ -507,10 +507,11 @@ Safe to ignore if the package is deleted in this commit.
}
final String credential = io.Platform.environment[_pubCredentialName];
if (credential == null) {
printErrorAndExit(errorMessage: '''
printError('''
No pub credential available. Please check if `~/.pub-cache/credentials.json` is valid.
If running this command on CI, you can set the pub credential content in the $_pubCredentialName environment variable.
''');
throw ToolExit(1);
}
credentialFile.openSync(mode: FileMode.writeOnlyAppend)
..writeStringSync(credential)

View File

@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// @dart=2.9
import 'dart:async';
import 'package:file/file.dart';
@ -37,7 +35,7 @@ enum NextVersionType {
/// those have different semver rules.
@visibleForTesting
Map<Version, NextVersionType> getAllowedNextVersions(
Version masterVersion, Version headVersion) {
{required Version masterVersion, required Version headVersion}) {
final Map<Version, NextVersionType> allowedNextVersions =
<Version, NextVersionType>{
masterVersion.nextMajor: NextVersionType.BREAKING_MAJOR,
@ -75,8 +73,8 @@ class VersionCheckCommand extends PluginCommand {
VersionCheckCommand(
Directory packagesDir, {
ProcessRunner processRunner = const ProcessRunner(),
GitDir gitDir,
this.httpClient,
GitDir? gitDir,
http.Client? httpClient,
}) : _pubVersionFinder =
PubVersionFinder(httpClient: httpClient ?? http.Client()),
super(packagesDir, processRunner: processRunner, gitDir: gitDir) {
@ -100,9 +98,6 @@ class VersionCheckCommand extends PluginCommand {
'Also checks if the latest version in CHANGELOG matches the version in pubspec.\n\n'
'This command requires "pub" and "flutter" to be in your path.';
/// The http client used to query pub server.
final http.Client httpClient;
final PubVersionFinder _pubVersionFinder;
@override
@ -112,6 +107,8 @@ class VersionCheckCommand extends PluginCommand {
final List<String> changedPubspecs =
await gitVersionFinder.getChangedPubSpecs();
final List<String> badVersionChangePubspecs = <String>[];
const String indentation = ' ';
for (final String pubspecPath in changedPubspecs) {
print('Checking versions for $pubspecPath...');
@ -126,15 +123,16 @@ class VersionCheckCommand extends PluginCommand {
continue;
}
final Version headVersion =
final Version? headVersion =
await gitVersionFinder.getPackageVersion(pubspecPath, gitRef: 'HEAD');
if (headVersion == null) {
printErrorAndExit(
errorMessage: '${indentation}No version found. A package that '
'intentionally has no version should be marked '
'"publish_to: none".');
printError('${indentation}No version found. A package that '
'intentionally has no version should be marked '
'"publish_to: none".');
badVersionChangePubspecs.add(pubspecPath);
continue;
}
Version sourceVersion;
Version? sourceVersion;
if (getBoolArg(_againstPubFlag)) {
final String packageName = pubspecFile.parent.basename;
final PubVersionFinderResponse pubVersionFinderResponse =
@ -146,12 +144,13 @@ class VersionCheckCommand extends PluginCommand {
'$indentation$packageName: Current largest version on pub: $sourceVersion');
break;
case PubVersionFinderResult.fail:
printErrorAndExit(errorMessage: '''
printError('''
${indentation}Error fetching version on pub for $packageName.
${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode}
${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
''');
break;
badVersionChangePubspecs.add(pubspecPath);
continue;
case PubVersionFinderResult.noPackageFound:
sourceVersion = null;
break;
@ -180,7 +179,8 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
// Check for reverts when doing local validation.
if (!getBoolArg(_againstPubFlag) && headVersion < sourceVersion) {
final Map<Version, NextVersionType> possibleVersionsFromNewVersion =
getAllowedNextVersions(headVersion, sourceVersion);
getAllowedNextVersions(
masterVersion: headVersion, headVersion: sourceVersion);
// Since this skips validation, try to ensure that it really is likely
// to be a revert rather than a typo by checking that the transition
// from the lower version to the new version would have been valid.
@ -192,14 +192,16 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
}
final Map<Version, NextVersionType> allowedNextVersions =
getAllowedNextVersions(sourceVersion, headVersion);
getAllowedNextVersions(
masterVersion: sourceVersion, headVersion: headVersion);
if (!allowedNextVersions.containsKey(headVersion)) {
final String source = (getBoolArg(_againstPubFlag)) ? 'pub' : 'master';
final String error = '${indentation}Incorrectly updated version.\n'
printError('${indentation}Incorrectly updated version.\n'
'${indentation}HEAD: $headVersion, $source: $sourceVersion.\n'
'${indentation}Allowed versions: $allowedNextVersions';
printErrorAndExit(errorMessage: error);
'${indentation}Allowed versions: $allowedNextVersions');
badVersionChangePubspecs.add(pubspecPath);
continue;
} else {
print('$indentation$headVersion -> $sourceVersion');
}
@ -208,38 +210,65 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
pubspec.name.endsWith('_platform_interface');
if (isPlatformInterface &&
allowedNextVersions[headVersion] == NextVersionType.BREAKING_MAJOR) {
final String error = '$pubspecPath breaking change detected.\n'
'Breaking changes to platform interfaces are strongly discouraged.\n';
printErrorAndExit(errorMessage: error);
printError('$pubspecPath breaking change detected.\n'
'Breaking changes to platform interfaces are strongly discouraged.\n');
badVersionChangePubspecs.add(pubspecPath);
continue;
}
}
_pubVersionFinder.httpClient.close();
// TODO(stuartmorgan): Unify the way iteration works for these checks; the
// two checks shouldn't be operating independently on different lists.
final List<String> mismatchedVersionPlugins = <String>[];
await for (final Directory plugin in getPlugins()) {
if (!(await _checkVersionsMatch(plugin))) {
mismatchedVersionPlugins.add(plugin.basename);
}
}
await for (final Directory plugin in getPlugins()) {
await _checkVersionsMatch(plugin);
bool passed = true;
if (badVersionChangePubspecs.isNotEmpty) {
passed = false;
printError('''
The following pubspecs failed validaton:
$indentation${badVersionChangePubspecs.join('\n$indentation')}
''');
}
if (mismatchedVersionPlugins.isNotEmpty) {
passed = false;
printError('''
The following pubspecs have different versions in pubspec.yaml and CHANGELOG.md:
$indentation${mismatchedVersionPlugins.join('\n$indentation')}
''');
}
if (!passed) {
throw ToolExit(1);
}
_pubVersionFinder.httpClient.close();
print('No version check errors found!');
}
Future<void> _checkVersionsMatch(Directory plugin) async {
/// Returns whether or not the pubspec version and CHANGELOG version for
/// [plugin] match.
Future<bool> _checkVersionsMatch(Directory plugin) async {
// get version from pubspec
final String packageName = plugin.basename;
print('-----------------------------------------');
print(
'Checking the first version listed in CHANGELOG.md matches the version in pubspec.yaml for $packageName.');
final Pubspec pubspec = _tryParsePubspec(plugin);
final Pubspec? pubspec = _tryParsePubspec(plugin);
if (pubspec == null) {
const String error = 'Cannot parse version from pubspec.yaml';
printErrorAndExit(errorMessage: error);
printError('Cannot parse version from pubspec.yaml');
return false;
}
final Version fromPubspec = pubspec.version;
final Version? fromPubspec = pubspec.version;
// get first version from CHANGELOG
final File changelog = plugin.childFile('CHANGELOG.md');
final List<String> lines = changelog.readAsLinesSync();
String firstLineWithText;
String? firstLineWithText;
final Iterator<String> iterator = lines.iterator;
while (iterator.moveNext()) {
if (iterator.current.trim().isNotEmpty) {
@ -248,7 +277,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
}
}
// Remove all leading mark down syntax from the version line.
String versionString = firstLineWithText.split(' ').last;
String? versionString = firstLineWithText?.split(' ').last;
// Skip validation for the special NEXT version that's used to accumulate
// changes that don't warrant publishing on their own.
@ -266,51 +295,48 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
}
}
final Version fromChangeLog = Version.parse(versionString);
final Version? fromChangeLog =
versionString == null ? null : Version.parse(versionString);
if (fromChangeLog == null) {
final String error =
'Cannot find version on the first line of ${plugin.path}/CHANGELOG.md';
printErrorAndExit(errorMessage: error);
printError(
'Cannot find version on the first line of ${plugin.path}/CHANGELOG.md');
return false;
}
if (fromPubspec != fromChangeLog) {
final String error = '''
printError('''
versions for $packageName in CHANGELOG.md and pubspec.yaml do not match.
The version in pubspec.yaml is $fromPubspec.
The first version listed in CHANGELOG.md is $fromChangeLog.
''';
printErrorAndExit(errorMessage: error);
''');
return false;
}
// If NEXT wasn't the first section, it should not exist at all.
if (!hasNextSection) {
final RegExp nextRegex = RegExp(r'^#+\s*NEXT\s*$');
if (lines.any((String line) => nextRegex.hasMatch(line))) {
printErrorAndExit(errorMessage: '''
printError('''
When bumping the version for release, the NEXT section should be incorporated
into the new version's release notes.
''');
return false;
}
}
print('$packageName passed version check');
return true;
}
Pubspec _tryParsePubspec(Directory package) {
Pubspec? _tryParsePubspec(Directory package) {
final File pubspecFile = package.childFile('pubspec.yaml');
try {
final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
if (pubspec == null) {
final String error =
'Failed to parse `pubspec.yaml` at ${pubspecFile.path}';
printErrorAndExit(errorMessage: error);
}
return pubspec;
} on Exception catch (exception) {
final String error =
'Failed to parse `pubspec.yaml` at ${pubspecFile.path}: $exception}';
printErrorAndExit(errorMessage: error);
printError(
'Failed to parse `pubspec.yaml` at ${pubspecFile.path}: $exception}');
}
return null;
}