mirror of
https://github.com/flutter/packages.git
synced 2025-06-09 05:59:45 +08:00
[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 https://github.com/flutter/flutter/issues/83413
This commit is contained in:
stuartmorgan
@ -6,12 +6,13 @@ import 'package:file/file.dart';
|
||||
import 'package:git/git.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:meta/meta.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:pub_semver/pub_semver.dart';
|
||||
import 'package:pubspec_parse/pubspec_parse.dart';
|
||||
|
||||
import 'common/core.dart';
|
||||
import 'common/git_version_finder.dart';
|
||||
import 'common/plugin_command.dart';
|
||||
import 'common/package_looping_command.dart';
|
||||
import 'common/process_runner.dart';
|
||||
import 'common/pub_version_finder.dart';
|
||||
|
||||
@ -31,46 +32,47 @@ enum NextVersionType {
|
||||
}
|
||||
|
||||
/// Returns the set of allowed next versions, with their change type, for
|
||||
/// [masterVersion].
|
||||
/// [version].
|
||||
///
|
||||
/// [headVerison] is used to check whether this is a pre-1.0 version bump, as
|
||||
/// [newVersion] is used to check whether this is a pre-1.0 version bump, as
|
||||
/// those have different semver rules.
|
||||
@visibleForTesting
|
||||
Map<Version, NextVersionType> getAllowedNextVersions(
|
||||
{required Version masterVersion, required Version headVersion}) {
|
||||
Version version, {
|
||||
required Version newVersion,
|
||||
}) {
|
||||
final Map<Version, NextVersionType> allowedNextVersions =
|
||||
<Version, NextVersionType>{
|
||||
masterVersion.nextMajor: NextVersionType.BREAKING_MAJOR,
|
||||
masterVersion.nextMinor: NextVersionType.MINOR,
|
||||
masterVersion.nextPatch: NextVersionType.PATCH,
|
||||
version.nextMajor: NextVersionType.BREAKING_MAJOR,
|
||||
version.nextMinor: NextVersionType.MINOR,
|
||||
version.nextPatch: NextVersionType.PATCH,
|
||||
};
|
||||
|
||||
if (masterVersion.major < 1 && headVersion.major < 1) {
|
||||
if (version.major < 1 && newVersion.major < 1) {
|
||||
int nextBuildNumber = -1;
|
||||
if (masterVersion.build.isEmpty) {
|
||||
if (version.build.isEmpty) {
|
||||
nextBuildNumber = 1;
|
||||
} else {
|
||||
final int currentBuildNumber = masterVersion.build.first as int;
|
||||
final int currentBuildNumber = version.build.first as int;
|
||||
nextBuildNumber = currentBuildNumber + 1;
|
||||
}
|
||||
final Version preReleaseVersion = Version(
|
||||
masterVersion.major,
|
||||
masterVersion.minor,
|
||||
masterVersion.patch,
|
||||
version.major,
|
||||
version.minor,
|
||||
version.patch,
|
||||
build: nextBuildNumber.toString(),
|
||||
);
|
||||
allowedNextVersions.clear();
|
||||
allowedNextVersions[masterVersion.nextMajor] = NextVersionType.RELEASE;
|
||||
allowedNextVersions[masterVersion.nextMinor] =
|
||||
NextVersionType.BREAKING_MAJOR;
|
||||
allowedNextVersions[masterVersion.nextPatch] = NextVersionType.MINOR;
|
||||
allowedNextVersions[version.nextMajor] = NextVersionType.RELEASE;
|
||||
allowedNextVersions[version.nextMinor] = NextVersionType.BREAKING_MAJOR;
|
||||
allowedNextVersions[version.nextPatch] = NextVersionType.MINOR;
|
||||
allowedNextVersions[preReleaseVersion] = NextVersionType.PATCH;
|
||||
}
|
||||
return allowedNextVersions;
|
||||
}
|
||||
|
||||
/// A command to validate version changes to packages.
|
||||
class VersionCheckCommand extends PluginCommand {
|
||||
class VersionCheckCommand extends PackageLoopingCommand {
|
||||
/// Creates an instance of the version check command.
|
||||
VersionCheckCommand(
|
||||
Directory packagesDir, {
|
||||
@ -91,6 +93,8 @@ class VersionCheckCommand extends PluginCommand {
|
||||
|
||||
static const String _againstPubFlag = 'against-pub';
|
||||
|
||||
final PubVersionFinder _pubVersionFinder;
|
||||
|
||||
@override
|
||||
final String name = 'version-check';
|
||||
|
||||
@ -100,175 +104,173 @@ 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.';
|
||||
|
||||
final PubVersionFinder _pubVersionFinder;
|
||||
@override
|
||||
bool get hasLongOutput => false;
|
||||
|
||||
@override
|
||||
Future<void> run() async {
|
||||
final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
|
||||
Future<void> initializeRun() async {}
|
||||
|
||||
final List<String> changedPubspecs =
|
||||
await gitVersionFinder.getChangedPubSpecs();
|
||||
@override
|
||||
Future<List<String>> runForPackage(Directory package) async {
|
||||
final List<String> errors = <String>[];
|
||||
|
||||
final List<String> badVersionChangePubspecs = <String>[];
|
||||
final Pubspec? pubspec = _tryParsePubspec(package);
|
||||
if (pubspec == null) {
|
||||
errors.add('Invalid pubspec.yaml.');
|
||||
return errors; // No remaining checks make sense.
|
||||
}
|
||||
|
||||
const String indentation = ' ';
|
||||
for (final String pubspecPath in changedPubspecs) {
|
||||
print('Checking versions for $pubspecPath...');
|
||||
final File pubspecFile = packagesDir.fileSystem.file(pubspecPath);
|
||||
if (!pubspecFile.existsSync()) {
|
||||
print('${indentation}Deleted; skipping.');
|
||||
continue;
|
||||
}
|
||||
final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
|
||||
if (pubspec.publishTo == 'none') {
|
||||
print('${indentation}Found "publish_to: none"; skipping.');
|
||||
continue;
|
||||
}
|
||||
if (pubspec.publishTo == 'none') {
|
||||
printSkip('${indentation}Found "publish_to: none".');
|
||||
return PackageLoopingCommand.success;
|
||||
}
|
||||
|
||||
final Version? headVersion =
|
||||
await gitVersionFinder.getPackageVersion(pubspecPath, gitRef: 'HEAD');
|
||||
if (headVersion == null) {
|
||||
printError('${indentation}No version found. A package that '
|
||||
'intentionally has no version should be marked '
|
||||
'"publish_to: none".');
|
||||
badVersionChangePubspecs.add(pubspecPath);
|
||||
continue;
|
||||
}
|
||||
Version? sourceVersion;
|
||||
if (getBoolArg(_againstPubFlag)) {
|
||||
final String packageName = pubspecFile.parent.basename;
|
||||
final PubVersionFinderResponse pubVersionFinderResponse =
|
||||
await _pubVersionFinder.getPackageVersion(package: packageName);
|
||||
switch (pubVersionFinderResponse.result) {
|
||||
case PubVersionFinderResult.success:
|
||||
sourceVersion = pubVersionFinderResponse.versions.first;
|
||||
print(
|
||||
'$indentation$packageName: Current largest version on pub: $sourceVersion');
|
||||
break;
|
||||
case PubVersionFinderResult.fail:
|
||||
printError('''
|
||||
final Version? currentPubspecVersion = pubspec.version;
|
||||
if (currentPubspecVersion == null) {
|
||||
printError('${indentation}No version found in pubspec.yaml. A package '
|
||||
'that intentionally has no version should be marked '
|
||||
'"publish_to: none".');
|
||||
errors.add('No pubspec.yaml version.');
|
||||
return errors; // No remaining checks make sense.
|
||||
}
|
||||
|
||||
if (!await _hasValidVersionChange(package, pubspec: pubspec)) {
|
||||
errors.add('Disallowed version change.');
|
||||
}
|
||||
|
||||
if (!(await _hasConsistentVersion(package, pubspec: pubspec))) {
|
||||
errors.add('pubspec.yaml and CHANGELOG.md have different versions');
|
||||
}
|
||||
|
||||
return errors;
|
||||
}
|
||||
|
||||
@override
|
||||
Future<void> completeRun() async {
|
||||
_pubVersionFinder.httpClient.close();
|
||||
}
|
||||
|
||||
/// Returns the previous published version of [package].
|
||||
///
|
||||
/// [packageName] must be the actual name of the package as published (i.e.,
|
||||
/// the name from pubspec.yaml, not the on disk name if different.)
|
||||
Future<Version?> _fetchPreviousVersionFromPub(String packageName) async {
|
||||
final PubVersionFinderResponse pubVersionFinderResponse =
|
||||
await _pubVersionFinder.getPackageVersion(package: packageName);
|
||||
switch (pubVersionFinderResponse.result) {
|
||||
case PubVersionFinderResult.success:
|
||||
return pubVersionFinderResponse.versions.first;
|
||||
case PubVersionFinderResult.fail:
|
||||
printError('''
|
||||
${indentation}Error fetching version on pub for $packageName.
|
||||
${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode}
|
||||
${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
|
||||
''');
|
||||
badVersionChangePubspecs.add(pubspecPath);
|
||||
continue;
|
||||
case PubVersionFinderResult.noPackageFound:
|
||||
sourceVersion = null;
|
||||
break;
|
||||
}
|
||||
} else {
|
||||
sourceVersion = await gitVersionFinder.getPackageVersion(pubspecPath);
|
||||
}
|
||||
if (sourceVersion == null) {
|
||||
String safeToIgnoreMessage;
|
||||
if (getBoolArg(_againstPubFlag)) {
|
||||
safeToIgnoreMessage =
|
||||
'${indentation}Unable to find package on pub server.';
|
||||
} else {
|
||||
safeToIgnoreMessage =
|
||||
'${indentation}Unable to find pubspec in master.';
|
||||
}
|
||||
print('$safeToIgnoreMessage Safe to ignore if the project is new.');
|
||||
continue;
|
||||
}
|
||||
|
||||
if (sourceVersion == headVersion) {
|
||||
print('${indentation}No version change.');
|
||||
continue;
|
||||
}
|
||||
|
||||
// Check for reverts when doing local validation.
|
||||
if (!getBoolArg(_againstPubFlag) && headVersion < sourceVersion) {
|
||||
final Map<Version, NextVersionType> possibleVersionsFromNewVersion =
|
||||
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.
|
||||
if (possibleVersionsFromNewVersion.containsKey(sourceVersion)) {
|
||||
print('${indentation}New version is lower than previous version. '
|
||||
'This is assumed to be a revert.');
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
final Map<Version, NextVersionType> allowedNextVersions =
|
||||
getAllowedNextVersions(
|
||||
masterVersion: sourceVersion, headVersion: headVersion);
|
||||
|
||||
if (!allowedNextVersions.containsKey(headVersion)) {
|
||||
final String source = (getBoolArg(_againstPubFlag)) ? 'pub' : 'master';
|
||||
printError('${indentation}Incorrectly updated version.\n'
|
||||
'${indentation}HEAD: $headVersion, $source: $sourceVersion.\n'
|
||||
'${indentation}Allowed versions: $allowedNextVersions');
|
||||
badVersionChangePubspecs.add(pubspecPath);
|
||||
continue;
|
||||
} else {
|
||||
print('$indentation$headVersion -> $sourceVersion');
|
||||
}
|
||||
|
||||
final bool isPlatformInterface =
|
||||
pubspec.name.endsWith('_platform_interface');
|
||||
if (isPlatformInterface &&
|
||||
allowedNextVersions[headVersion] == NextVersionType.BREAKING_MAJOR) {
|
||||
printError('$pubspecPath breaking change detected.\n'
|
||||
'Breaking changes to platform interfaces are strongly discouraged.\n');
|
||||
badVersionChangePubspecs.add(pubspecPath);
|
||||
continue;
|
||||
}
|
||||
return null;
|
||||
case PubVersionFinderResult.noPackageFound:
|
||||
return Version.none;
|
||||
}
|
||||
_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);
|
||||
/// Returns the version of [package] from git at the base comparison hash.
|
||||
Future<Version?> _getPreviousVersionFromGit(
|
||||
Directory package, {
|
||||
required GitVersionFinder gitVersionFinder,
|
||||
}) async {
|
||||
final File pubspecFile = package.childFile('pubspec.yaml');
|
||||
return await gitVersionFinder.getPackageVersion(
|
||||
p.relative(pubspecFile.absolute.path, from: (await gitDir).path));
|
||||
}
|
||||
|
||||
/// Returns true if the version of [package] is either unchanged relative to
|
||||
/// the comparison base (git or pub, depending on flags), or is a valid
|
||||
/// version transition.
|
||||
Future<bool> _hasValidVersionChange(
|
||||
Directory package, {
|
||||
required Pubspec pubspec,
|
||||
}) async {
|
||||
// This method isn't called unless `version` is non-null.
|
||||
final Version currentVersion = pubspec.version!;
|
||||
Version? previousVersion;
|
||||
if (getBoolArg(_againstPubFlag)) {
|
||||
previousVersion = await _fetchPreviousVersionFromPub(pubspec.name);
|
||||
if (previousVersion == null) {
|
||||
return false;
|
||||
}
|
||||
if (previousVersion != Version.none) {
|
||||
print(
|
||||
'$indentation${pubspec.name}: Current largest version on pub: $previousVersion');
|
||||
}
|
||||
} else {
|
||||
final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
|
||||
previousVersion = await _getPreviousVersionFromGit(package,
|
||||
gitVersionFinder: gitVersionFinder) ??
|
||||
Version.none;
|
||||
}
|
||||
if (previousVersion == Version.none) {
|
||||
print('${indentation}Unable to find previous version '
|
||||
'${getBoolArg(_againstPubFlag) ? 'on pub server' : 'at git base'}.');
|
||||
printWarning(
|
||||
'${indentation}If this plugin is not new, something has gone wrong.');
|
||||
return true;
|
||||
}
|
||||
|
||||
if (previousVersion == currentVersion) {
|
||||
print('${indentation}No version change.');
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check for reverts when doing local validation.
|
||||
if (!getBoolArg(_againstPubFlag) && currentVersion < previousVersion) {
|
||||
final Map<Version, NextVersionType> possibleVersionsFromNewVersion =
|
||||
getAllowedNextVersions(currentVersion, newVersion: previousVersion);
|
||||
// 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.
|
||||
if (possibleVersionsFromNewVersion.containsKey(previousVersion)) {
|
||||
print('${indentation}New version is lower than previous version. '
|
||||
'This is assumed to be a revert.');
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
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);
|
||||
final Map<Version, NextVersionType> allowedNextVersions =
|
||||
getAllowedNextVersions(previousVersion, newVersion: currentVersion);
|
||||
|
||||
if (allowedNextVersions.containsKey(currentVersion)) {
|
||||
print('$indentation$previousVersion -> $currentVersion');
|
||||
} else {
|
||||
final String source = (getBoolArg(_againstPubFlag)) ? 'pub' : 'master';
|
||||
printError('${indentation}Incorrectly updated version.\n'
|
||||
'${indentation}HEAD: $currentVersion, $source: $previousVersion.\n'
|
||||
'${indentation}Allowed versions: $allowedNextVersions');
|
||||
return false;
|
||||
}
|
||||
|
||||
print('No version check errors found!');
|
||||
final bool isPlatformInterface =
|
||||
pubspec.name.endsWith('_platform_interface');
|
||||
// TODO(stuartmorgan): Relax this check. See
|
||||
// https://github.com/flutter/flutter/issues/85391
|
||||
if (isPlatformInterface &&
|
||||
allowedNextVersions[currentVersion] == NextVersionType.BREAKING_MAJOR) {
|
||||
printError('${indentation}Breaking change detected.\n'
|
||||
'${indentation}Breaking changes to platform interfaces are strongly discouraged.\n');
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/// 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);
|
||||
if (pubspec == null) {
|
||||
printError('Cannot parse version from pubspec.yaml');
|
||||
return false;
|
||||
}
|
||||
final Version? fromPubspec = pubspec.version;
|
||||
Future<bool> _hasConsistentVersion(
|
||||
Directory package, {
|
||||
required Pubspec pubspec,
|
||||
}) async {
|
||||
// This method isn't called unless `version` is non-null.
|
||||
final Version fromPubspec = pubspec.version!;
|
||||
|
||||
// get first version from CHANGELOG
|
||||
final File changelog = plugin.childFile('CHANGELOG.md');
|
||||
final File changelog = package.childFile('CHANGELOG.md');
|
||||
final List<String> lines = changelog.readAsLinesSync();
|
||||
String? firstLineWithText;
|
||||
final Iterator<String> iterator = lines.iterator;
|
||||
@ -285,7 +287,8 @@ $indentation${mismatchedVersionPlugins.join('\n$indentation')}
|
||||
// changes that don't warrant publishing on their own.
|
||||
final bool hasNextSection = versionString == 'NEXT';
|
||||
if (hasNextSection) {
|
||||
print('Found NEXT; validating next version in the CHANGELOG.');
|
||||
print(
|
||||
'${indentation}Found NEXT; validating next version in the CHANGELOG.');
|
||||
// Ensure that the version in pubspec hasn't changed without updating
|
||||
// CHANGELOG. That means the next version entry in the CHANGELOG pass the
|
||||
// normal validation.
|
||||
@ -301,15 +304,15 @@ $indentation${mismatchedVersionPlugins.join('\n$indentation')}
|
||||
versionString == null ? null : Version.parse(versionString);
|
||||
if (fromChangeLog == null) {
|
||||
printError(
|
||||
'Cannot find version on the first line of ${plugin.path}/CHANGELOG.md');
|
||||
'${indentation}Cannot find version on the first line CHANGELOG.md');
|
||||
return false;
|
||||
}
|
||||
|
||||
if (fromPubspec != fromChangeLog) {
|
||||
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.
|
||||
${indentation}Versions in CHANGELOG.md and pubspec.yaml do not match.
|
||||
${indentation}The version in pubspec.yaml is $fromPubspec.
|
||||
${indentation}The first version listed in CHANGELOG.md is $fromChangeLog.
|
||||
''');
|
||||
return false;
|
||||
}
|
||||
@ -318,15 +321,13 @@ The first version listed in CHANGELOG.md is $fromChangeLog.
|
||||
if (!hasNextSection) {
|
||||
final RegExp nextRegex = RegExp(r'^#+\s*NEXT\s*$');
|
||||
if (lines.any((String line) => nextRegex.hasMatch(line))) {
|
||||
printError('''
|
||||
When bumping the version for release, the NEXT section should be incorporated
|
||||
into the new version's release notes.
|
||||
''');
|
||||
printError('${indentation}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;
|
||||
}
|
||||
|
||||
@ -337,9 +338,8 @@ into the new version's release notes.
|
||||
final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
|
||||
return pubspec;
|
||||
} on Exception catch (exception) {
|
||||
printError(
|
||||
'Failed to parse `pubspec.yaml` at ${pubspecFile.path}: $exception}');
|
||||
printError('${indentation}Failed to parse `pubspec.yaml`: $exception}');
|
||||
return null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user