-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[tool] version-check publish-check commands can check against pub #3840
Changes from 12 commits
b6a1cdd
cf26cd4
46ca9ae
7262ef3
3f2d76d
497292e
c70da8f
e5d149e
453f8bf
8a29e36
3880f2e
4cd1f65
3259241
6a96692
3242c8b
eb5ee5c
fc26cbb
f083fe4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,13 @@ | |
// found in the LICENSE file. | ||
|
||
import 'dart:async'; | ||
import 'dart:convert'; | ||
import 'dart:io' as io; | ||
|
||
import 'package:colorize/colorize.dart'; | ||
import 'package:file/file.dart'; | ||
import 'package:http/http.dart' as http; | ||
import 'package:pub_semver/pub_semver.dart'; | ||
import 'package:pubspec_parse/pubspec_parse.dart'; | ||
|
||
import 'common.dart'; | ||
|
@@ -18,17 +21,38 @@ class PublishCheckCommand extends PluginCommand { | |
Directory packagesDir, | ||
FileSystem fileSystem, { | ||
ProcessRunner processRunner = const ProcessRunner(), | ||
}) : super(packagesDir, fileSystem, processRunner: processRunner) { | ||
this.httpClient, | ||
}) : _pubVersionFinder = | ||
PubVersionFinder(httpClient: httpClient ?? http.Client()), | ||
super(packagesDir, fileSystem, processRunner: processRunner) { | ||
argParser.addFlag( | ||
_allowPrereleaseFlag, | ||
help: 'Allows the pre-release SDK warning to pass.\n' | ||
'When enabled, a pub warning, which asks to publish the package as a pre-release version when ' | ||
'the SDK constraint is a pre-release version, is ignored.', | ||
defaultsTo: false, | ||
); | ||
argParser.addFlag(_machineFlag, | ||
help: 'Only prints a final status as a defined string.\n' | ||
'The possible values are:\n' | ||
' $_machineMessageNeedsPublish: There is at least one package need to be published. They also passed all publish checks.\n' | ||
' $_machineMessageNoPublish: There are no packages needs to be published. Either no pubspec change detected or all versions have already been published.\n' | ||
' $_machineMessageError: Some error has occurred.', | ||
defaultsTo: false, | ||
negatable: true); | ||
} | ||
|
||
static const String _allowPrereleaseFlag = 'allow-pre-release'; | ||
static const String _machineFlag = 'machine'; | ||
static const String _machineMessageNeedsPublish = 'needs-publish'; | ||
static const String _machineMessageNoPublish = 'no-publish'; | ||
static const String _machineMessageError = 'error'; | ||
|
||
final List<String> _validStatus = <String>[ | ||
_machineMessageNeedsPublish, | ||
_machineMessageNoPublish, | ||
_machineMessageError | ||
]; | ||
|
||
@override | ||
final String name = 'publish-check'; | ||
|
@@ -37,15 +61,46 @@ 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; | ||
|
||
@override | ||
Future<void> run() async { | ||
final ZoneSpecification logSwitchSpecification = ZoneSpecification( | ||
print: (Zone self, ZoneDelegate parent, Zone zone, String message) { | ||
final bool logMachineMessage = argResults[_machineFlag] as bool; | ||
final bool isMachineMessage = _validStatus.contains(message); | ||
if (logMachineMessage == isMachineMessage) { | ||
parent.print(zone, message); | ||
} | ||
}); | ||
|
||
await runZoned(_runCommand, zoneSpecification: logSwitchSpecification); | ||
} | ||
|
||
Future<void> _runCommand() async { | ||
final List<Directory> failedPackages = <Directory>[]; | ||
|
||
String resultToLog = _machineMessageNoPublish; | ||
await for (final Directory plugin in getPlugins()) { | ||
if (!(await _passesPublishCheck(plugin))) { | ||
failedPackages.add(plugin); | ||
final _PublishCheckResult result = await _passesPublishCheck(plugin); | ||
switch (result) { | ||
case _PublishCheckResult._needsPublish: | ||
if (failedPackages.isEmpty) { | ||
resultToLog = _machineMessageNeedsPublish; | ||
} | ||
break; | ||
case _PublishCheckResult._noPublish: | ||
break; | ||
case _PublishCheckResult._error: | ||
failedPackages.add(plugin); | ||
resultToLog = _machineMessageError; | ||
break; | ||
} | ||
} | ||
_pubVersionFinder.httpClient.close(); | ||
|
||
if (failedPackages.isNotEmpty) { | ||
final String error = | ||
|
@@ -56,12 +111,18 @@ class PublishCheckCommand extends PluginCommand { | |
final Colorize colorizedError = Colorize('$error\n$joinedFailedPackages') | ||
..red(); | ||
print(colorizedError); | ||
throw ToolExit(1); | ||
} else { | ||
final Colorize passedMessage = | ||
Colorize('All packages passed publish check!')..green(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the new tests, it's very strange to have terminal color commands in the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated so that both machine mesasge and human message include the |
||
print(passedMessage); | ||
} | ||
|
||
final Colorize passedMessage = | ||
Colorize('All packages passed publish check!')..green(); | ||
print(passedMessage); | ||
if (argResults[_machineFlag] as bool) { | ||
print(resultToLog); | ||
} | ||
if (failedPackages.isNotEmpty) { | ||
throw ToolExit(1); | ||
} | ||
} | ||
|
||
Pubspec _tryParsePubspec(Directory package) { | ||
|
@@ -89,17 +150,23 @@ class PublishCheckCommand extends PluginCommand { | |
final Completer<void> stdOutCompleter = Completer<void>(); | ||
process.stdout.listen( | ||
(List<int> event) { | ||
io.stdout.add(event); | ||
outputBuffer.write(String.fromCharCodes(event)); | ||
final String output = String.fromCharCodes(event); | ||
if (output.isNotEmpty) { | ||
print(output); | ||
outputBuffer.write(output); | ||
} | ||
}, | ||
onDone: () => stdOutCompleter.complete(), | ||
); | ||
|
||
final Completer<void> stdInCompleter = Completer<void>(); | ||
process.stderr.listen( | ||
(List<int> event) { | ||
io.stderr.add(event); | ||
outputBuffer.write(String.fromCharCodes(event)); | ||
final String output = String.fromCharCodes(event); | ||
if (output.isNotEmpty) { | ||
print(Colorize(output)..red()); | ||
outputBuffer.write(output); | ||
} | ||
}, | ||
onDone: () => stdInCompleter.complete(), | ||
); | ||
|
@@ -121,24 +188,67 @@ class PublishCheckCommand extends PluginCommand { | |
'Packages with an SDK constraint on a pre-release of the Dart SDK should themselves be published as a pre-release version.'); | ||
} | ||
|
||
Future<bool> _passesPublishCheck(Directory package) async { | ||
Future<_PublishCheckResult> _passesPublishCheck(Directory package) async { | ||
final String packageName = package.basename; | ||
print('Checking that $packageName can be published.'); | ||
|
||
final Pubspec pubspec = _tryParsePubspec(package); | ||
if (pubspec == null) { | ||
return false; | ||
print('no pubspec'); | ||
return _PublishCheckResult._error; | ||
} else if (pubspec.publishTo == 'none') { | ||
print('Package $packageName is marked as unpublishable. Skipping.'); | ||
return true; | ||
return _PublishCheckResult._noPublish; | ||
} | ||
|
||
final Version version = pubspec.version; | ||
final bool alreadyPublished = await _checkIfAlreadyPublished( | ||
packageName: packageName, version: version); | ||
if (alreadyPublished) { | ||
print( | ||
'Package $packageName version: $version has already be published on pub.'); | ||
return _PublishCheckResult._noPublish; | ||
} | ||
|
||
if (await _hasValidPublishCheckRun(package)) { | ||
print('Package $packageName is able to be published.'); | ||
return true; | ||
return _PublishCheckResult._needsPublish; | ||
} else { | ||
print('Unable to publish $packageName'); | ||
return false; | ||
return _PublishCheckResult._error; | ||
} | ||
} | ||
|
||
// Check if `packageName` already has `version` published on pub. | ||
Future<bool> _checkIfAlreadyPublished( | ||
{String packageName, Version version}) async { | ||
final PubVersionFinderResponse pubVersionFinderResponse = | ||
await _pubVersionFinder.getPackageVersion(package: packageName); | ||
bool published; | ||
switch (pubVersionFinderResponse.result) { | ||
case PubVersionFinderResult.success: | ||
published = pubVersionFinderResponse.versions.contains(version); | ||
break; | ||
case PubVersionFinderResult.fail: | ||
print(_machineMessageError); | ||
printErrorAndExit(errorMessage: ''' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't this break the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking this should return something, rather than bailing completely. Is the reason for this that we assume that all the other checks will fail the same way because it's likely a network issue? If we do want to keep this construction, this line (and the line above) need comments explaining why they are doing what they are doing, since it's non-obvious. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah ok, I updated to print the message and handled the error later |
||
Error fetching version on pub for $packageName. | ||
HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} | ||
HTTP response: ${pubVersionFinderResponse.httpResponse.body} | ||
'''); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm realizing that failures here are going to be almost impossible to debug, because we won't have any information in the log. Perhaps we should consider doing something like a JSON dictionary of output, with status and detailed log messages as separate keys? That would mean we'd need to consume the result with something like a Dart script that can parse the JSON, rather than just a one-liner in the GitHub Action. Thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good Idea, updated the code to do exactly that |
||
break; | ||
case PubVersionFinderResult.noPackageFound: | ||
published = false; | ||
break; | ||
} | ||
return published; | ||
} | ||
} | ||
|
||
enum _PublishCheckResult { | ||
_needsPublish, | ||
|
||
_noPublish, | ||
|
||
_error, | ||
} |
Uh oh!
There was an error while loading. Please reload this page.