From 031e6ec37b86c7bf3cdab17ba834b4898f3254a7 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 21 Apr 2026 16:54:33 -0400 Subject: [PATCH] [tool] Remove --against-pub flag We haven't used `--against-pub` in CI in a very long time, and we don't expect to need it locally. We report pub versions as part of the publish step, but the source of truth for validation is the repo, not pub. This removes the flag to simplify the tool. --- .../tool/lib/src/version_check_command.dart | 72 +--------- .../tool/test/version_check_command_test.dart | 133 +----------------- 2 files changed, 10 insertions(+), 195 deletions(-) diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 3454c7e08890..55d3ba13a597 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'package:file/file.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'; @@ -12,7 +11,6 @@ import 'common/git_version_finder.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/package_state_utils.dart'; -import 'common/pub_version_finder.dart'; import 'common/repository_package.dart'; /// Categories of version change types. @@ -98,17 +96,7 @@ class VersionCheckCommand extends PackageLoopingCommand { super.processRunner, super.platform, super.gitDir, - http.Client? httpClient, - }) : _pubVersionFinder = PubVersionFinder( - httpClient: httpClient ?? http.Client(), - ) { - argParser.addFlag( - _againstPubFlag, - help: - 'Whether the version check should run against the version on pub.\n' - 'Defaults to false, which means the version check only run against ' - 'the previous version in code.', - ); + }) { argParser.addOption( _prLabelsArg, help: @@ -140,7 +128,6 @@ class VersionCheckCommand extends PackageLoopingCommand { ); } - static const String _againstPubFlag = 'against-pub'; static const String _prLabelsArg = 'pr-labels'; static const String _checkForMissingChanges = 'check-for-missing-changes'; static const String _ignorePlatformInterfaceBreaks = @@ -161,8 +148,6 @@ class VersionCheckCommand extends PackageLoopingCommand { static const String _missingChangelogChangeOverrideLabel = 'override: no changelog needed'; - final PubVersionFinder _pubVersionFinder; - late final GitVersionFinder _gitVersionFinder; late final Set _prLabels = _getPRLabels(); @@ -184,7 +169,7 @@ class VersionCheckCommand extends PackageLoopingCommand { final String description = 'Checks if the versions of packages have been incremented per pub specification.\n' '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.'; + 'This command requires "flutter" to be in your path.'; @override bool get hasLongOutput => false; @@ -303,33 +288,6 @@ class VersionCheckCommand extends PackageLoopingCommand { : PackageResult.fail(errors); } - @override - Future 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 _fetchPreviousVersionFromPub(String packageName) async { - final PubVersionFinderResponse pubVersionFinderResponse = - await _pubVersionFinder.getPackageVersion(packageName: 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} -'''); - return null; - case PubVersionFinderResult.noPackageFound: - return Version.none; - } - } - /// Returns the version of [package] from git at the base comparison hash. Future _getPreviousVersionFromGit(RepositoryPackage package) async { final File pubspecFile = package.pubspecFile; @@ -352,28 +310,12 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} }) async { // This method isn't called unless `version` is non-null. final Version currentVersion = pubspec.version!; - Version? previousVersion; - String previousVersionSource; - if (getBoolArg(_againstPubFlag)) { - previousVersionSource = 'pub'; - previousVersion = await _fetchPreviousVersionFromPub(pubspec.name); - if (previousVersion == null) { - return _CurrentVersionState.unknown; - } - if (previousVersion != Version.none) { - print( - '$indentation${pubspec.name}: Current largest version on pub: $previousVersion', - ); - } - } else { - previousVersionSource = baseSha; - previousVersion = - await _getPreviousVersionFromGit(package) ?? Version.none; - } + final Version previousVersion = + await _getPreviousVersionFromGit(package) ?? Version.none; if (previousVersion == Version.none) { print( '${indentation}Unable to find previous version ' - '${getBoolArg(_againstPubFlag) ? 'on pub server' : 'at git base'}.', + 'at git base.', ); logWarning( '${indentation}If this package is not new, something has gone wrong.', @@ -387,7 +329,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} } // Check for reverts when doing local validation. - if (!getBoolArg(_againstPubFlag) && currentVersion < previousVersion) { + if (currentVersion < 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. @@ -414,7 +356,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} } else { printError( '${indentation}Incorrectly updated version.\n' - '${indentation}HEAD: $currentVersion, $previousVersionSource: $previousVersion.\n' + '${indentation}HEAD: $currentVersion, $baseSha: $previousVersion.\n' '${indentation}Allowed versions: $allowedNextVersions', ); return _CurrentVersionState.invalidChange; diff --git a/script/tool/test/version_check_command_test.dart b/script/tool/test/version_check_command_test.dart index ee37a55313ca..75bf007eaec1 100644 --- a/script/tool/test/version_check_command_test.dart +++ b/script/tool/test/version_check_command_test.dart @@ -2,15 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:convert'; - import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; import 'package:flutter_plugin_tools/src/version_check_command.dart'; import 'package:git/git.dart'; -import 'package:http/http.dart' as http; -import 'package:http/testing.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; @@ -40,15 +36,11 @@ void testAllowedVersion( } void main() { - const indentation = ' '; group('VersionCheckCommand', () { late MockPlatform mockPlatform; late Directory packagesDir; late CommandRunner runner; late RecordingProcessRunner gitProcessRunner; - // Ignored if mockHttpResponse is set. - int mockHttpStatus; - Map? mockHttpResponse; setUp(() { mockPlatform = MockPlatform(); @@ -57,22 +49,11 @@ void main() { (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); - // Default to simulating the plugin never having been published. - mockHttpStatus = 404; - mockHttpResponse = null; - final mockClient = MockClient((http.Request request) async { - return http.Response( - json.encode(mockHttpResponse), - mockHttpResponse == null ? mockHttpStatus : 200, - ); - }); - final command = VersionCheckCommand( packagesDir, processRunner: processRunner, platform: mockPlatform, gitDir: gitDir, - httpClient: mockClient, ); runner = CommandRunner( @@ -448,7 +429,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main', '--against-pub'], + ['version-check', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -594,7 +575,7 @@ void main() { var hasError = false; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main', '--against-pub'], + ['version-check', '--base-sha=main'], errorHandler: (Error e) { expect(e, isA()); hasError = true; @@ -669,7 +650,7 @@ void main() { var hasError = false; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main', '--against-pub'], + ['version-check', '--base-sha=main'], errorHandler: (Error e) { expect(e, isA()); hasError = true; @@ -1652,114 +1633,6 @@ packages/plugin/lib/plugin.dart }); }); - test('allows valid against pub', () async { - mockHttpResponse = { - 'name': 'some_package', - 'versions': ['0.0.1', '0.0.2', '1.0.0'], - }; - - createFakePlugin('plugin', packagesDir, version: '2.0.0'); - final List output = await runCapturingPrint(runner, [ - 'version-check', - '--base-sha=main', - '--against-pub', - ]); - - expect( - output, - containsAllInOrder([ - contains('plugin: Current largest version on pub: 1.0.0'), - ]), - ); - }); - - test('denies invalid against pub', () async { - mockHttpResponse = { - 'name': 'some_package', - 'versions': ['0.0.1', '0.0.2'], - }; - - createFakePlugin('plugin', packagesDir, version: '2.0.0'); - - var hasError = false; - final List result = await runCapturingPrint( - runner, - ['version-check', '--base-sha=main', '--against-pub'], - errorHandler: (Error e) { - expect(e, isA()); - hasError = true; - }, - ); - expect(hasError, isTrue); - - expect( - result, - containsAllInOrder([ - contains( - ''' -${indentation}Incorrectly updated version. -${indentation}HEAD: 2.0.0, pub: 0.0.2. -${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: NextVersionType.MINOR, 0.0.3: NextVersionType.PATCH}''', - ), - ]), - ); - }); - - test( - 'throw and print error message if http request failed when checking against pub', - () async { - mockHttpStatus = 400; - - createFakePlugin('plugin', packagesDir, version: '2.0.0'); - var hasError = false; - final List result = await runCapturingPrint( - runner, - ['version-check', '--base-sha=main', '--against-pub'], - errorHandler: (Error e) { - expect(e, isA()); - hasError = true; - }, - ); - expect(hasError, isTrue); - - expect( - result, - containsAllInOrder([ - contains(''' -${indentation}Error fetching version on pub for plugin. -${indentation}HTTP Status 400 -${indentation}HTTP response: null -'''), - ]), - ); - }, - ); - - test( - 'when checking against pub, allow any version if http status is 404.', - () async { - mockHttpStatus = 404; - - createFakePlugin('plugin', packagesDir, version: '2.0.0'); - gitProcessRunner.mockProcessesForExecutable['git-show'] = - [ - FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), - ]; - final List result = await runCapturingPrint(runner, [ - 'version-check', - '--base-sha=main', - '--against-pub', - ]); - - expect( - result, - containsAllInOrder([ - contains('Unable to find previous version on pub server.'), - ]), - ); - }, - ); - group('prelease versions', () { test( 'allow an otherwise-valid transition that also adds a pre-release component',