Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions script/tool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.14.1

* Adds `min_dart` as an alternative to `min_flutter` in tool configuration.
* Makes .ci.yaml optional if batched release is not used.

## 0.14.0

* Re-launches the published version of the tool for use in
Expand Down
13 changes: 13 additions & 0 deletions script/tool/lib/src/common/tool_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,19 @@ String? getMinFlutterVersion(Directory repoRoot) {
return yaml;
}

/// Returns the minimum Dart version allowed.
String? getMinDartVersion(Directory repoRoot) {
final Object? yaml = _getToolConfig(repoRoot)['min_dart'];
if (yaml == null) {
return null;
Comment thread
chunhtai marked this conversation as resolved.
}
if (yaml is! String) {
printError('min_dart must be a full version string (e.g., "3.10.0").');
throw ToolExit(exitInvalidArguments);
}
return yaml;
}

/// Returns the allowed dependencies, grouped by 'pinned' and 'unpinned'.
({List<String> pinned, List<String> unpinned}) getAllowedDependencies(
Directory repoRoot,
Expand Down
44 changes: 36 additions & 8 deletions script/tool/lib/src/validate_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'package:file/file.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';

import 'common/core.dart';
import 'common/git_version_finder.dart';
Expand All @@ -19,6 +20,9 @@ import 'validators/readme_validator.dart';
import 'validators/repo_info_validator.dart';
import 'validators/version_and_changelog_validator.dart';

const int _missingMinSdkVersionExitCode = 3;
const int _unknownVersionMappingExitCode = 4;

/// The set of possible validators.
///
/// Exposed for testing so that unit tests can target a single validator's
Expand Down Expand Up @@ -113,7 +117,10 @@ class ValidateCommand extends PackageLoopingCommand {
);

/// The minimum version of Flutter that is allowed for any package.
late final String _minMinFlutterVersion;
Version? _minMinFlutterVersion;

/// The minimum version of Dart that is allowed for any package.
Version? _minMinDartVersion;

@override
final String name = 'validate';
Expand Down Expand Up @@ -149,7 +156,20 @@ class ValidateCommand extends PackageLoopingCommand {
}
if (_shouldRun(Validator.pubspec)) {
await _loadAllowedDependencies();
_minMinFlutterVersion = _loadMinMinFlutterVersion();
final (flutter: Version? minFlutter, dart: Version? minDart) =
_loadMinMinSdkVersions();
_minMinFlutterVersion = minFlutter;
_minMinDartVersion =
minDart ??
(minFlutter == null ? null : getDartSdkForFlutterSdk(minFlutter));
if (_minMinDartVersion == null) {
printError(
'Dart SDK version for Flutter SDK version $_minMinFlutterVersion is unknown. '
'Please update the map for getDartSdkForFlutterSdk with the '
'corresponding Dart version.',
);
throw ToolExit(_unknownVersionMappingExitCode);
}
}
if (_shouldRun(Validator.dependabot)) {
_dependabotCoverage = DependabotValidator.loadConfig(repoRoot: _repoRoot);
Expand Down Expand Up @@ -220,6 +240,7 @@ class ValidateCommand extends PackageLoopingCommand {
allowedPackages: _allowedPackages,
repoRoot: rootDir,
minMinFlutterVersion: _minMinFlutterVersion,
minMinDartVersion: _minMinDartVersion,
);
return validator.validatePubspec(package);
}
Expand Down Expand Up @@ -323,13 +344,20 @@ class ValidateCommand extends PackageLoopingCommand {
_allowedPackages.pinned.addAll(allowedDeps.pinned);
}

String _loadMinMinFlutterVersion() {
final String? minVersion = getMinFlutterVersion(_repoRoot);
if (minVersion == null) {
printError('min_flutter is missing in $configFilename');
return '';
({Version? flutter, Version? dart}) _loadMinMinSdkVersions() {
final String? minFlutter = getMinFlutterVersion(_repoRoot);
final String? minDart = getMinDartVersion(_repoRoot);
if (minFlutter == null && minDart == null) {
printError(
'Either min_flutter or min_dart must be provided '
'in the repo tool configuration.',
);
throw ToolExit(_missingMinSdkVersionExitCode);
}
return minVersion.trim();
return (
flutter: minFlutter == null ? null : Version.parse(minFlutter),
dart: minDart == null ? null : Version.parse(minDart),
);
Comment thread
stuartmorgan-g marked this conversation as resolved.
}

Pubspec? _tryParsePubspec(String pubspecContents) {
Expand Down
49 changes: 18 additions & 31 deletions script/tool/lib/src/validators/pubspec_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,23 @@ class PubspecValidator {
required AllowPackageLists allowedPackages,
required void Function(String) warningLogger,
required Directory repoRoot,
required String minMinFlutterVersion,
Version? minMinFlutterVersion,
Version? minMinDartVersion,
}) : _path = path,
_indentation = indentation,
_allowedPackages = allowedPackages,
_logWarning = warningLogger,
_repoRoot = repoRoot,
_minMinFlutterVersion = minMinFlutterVersion;
_minMinFlutterVersion = minMinFlutterVersion,
_minMinDartVersion = minMinDartVersion;

final path.Context _path;
final String _indentation;
final AllowPackageLists _allowedPackages;
final void Function(String) _logWarning;
final Directory _repoRoot;
final String _minMinFlutterVersion;
final Version? _minMinFlutterVersion;
final Version? _minMinDartVersion;

/// Validates that the pubspec of a package follows repository conventions,
/// returning a list of errors.
Expand Down Expand Up @@ -115,9 +118,6 @@ class PubspecValidator {
final String? minVersionError = _checkForMinimumVersionError(
pubspec,
package,
minMinFlutterVersion: _minMinFlutterVersion.isEmpty
? null
: Version.parse(_minMinFlutterVersion),
);
if (minVersionError != null) {
printError('$_indentation$minVersionError');
Expand Down Expand Up @@ -423,24 +423,8 @@ class PubspecValidator {
/// Returns an error string if validation fails.
String? _checkForMinimumVersionError(
Pubspec pubspec,
RepositoryPackage package, {
Version? minMinFlutterVersion,
}) {
String unknownDartVersionError(Version flutterVersion) {
return 'Dart SDK version for Flutter SDK version '
'$flutterVersion is unknown. '
'Please update the map for getDartSdkForFlutterSdk with the '
'corresponding Dart version.';
}

Version? minMinDartVersion;
if (minMinFlutterVersion != null) {
minMinDartVersion = getDartSdkForFlutterSdk(minMinFlutterVersion);
if (minMinDartVersion == null) {
return unknownDartVersionError(minMinFlutterVersion);
}
}

RepositoryPackage package,
) {
final Version? dartConstraintMin = _minimumForConstraint(
pubspec.environment['sdk'],
);
Expand All @@ -449,19 +433,19 @@ class PubspecValidator {
);

// Validate the Flutter constraint, if any.
if (flutterConstraintMin != null && minMinFlutterVersion != null) {
if (flutterConstraintMin < minMinFlutterVersion) {
if (flutterConstraintMin != null && _minMinFlutterVersion != null) {
if (flutterConstraintMin < _minMinFlutterVersion) {
return 'Minimum allowed Flutter version $flutterConstraintMin is less '
'than $minMinFlutterVersion';
'than $_minMinFlutterVersion';
}
}

// Validate the Dart constraint, if any.
if (dartConstraintMin != null) {
// Ensure that it satisfies the minimum.
if (minMinDartVersion != null) {
if (dartConstraintMin < minMinDartVersion) {
return 'Minimum allowed Dart version $dartConstraintMin is less than $minMinDartVersion';
if (_minMinDartVersion != null) {
if (dartConstraintMin < _minMinDartVersion) {
return 'Minimum allowed Dart version $dartConstraintMin is less than $_minMinDartVersion';
}
}

Expand All @@ -471,7 +455,10 @@ class PubspecValidator {
flutterConstraintMin,
);
if (dartVersionForFlutterMinimum == null) {
return unknownDartVersionError(flutterConstraintMin);
return 'Dart SDK version for Flutter SDK version '
'$flutterConstraintMin is unknown. '
'Please update the map for getDartSdkForFlutterSdk with the '
'corresponding Dart version.';
}
if (dartVersionForFlutterMinimum != dartConstraintMin) {
return 'The minimum Dart version is $dartConstraintMin, but the '
Expand Down
11 changes: 11 additions & 0 deletions script/tool/lib/src/validators/repo_info_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,17 @@ class RepoInfoValidator {
}) {
final errors = <String>[];
final File ciYamlFile = repoRoot.childFile('.ci.yaml');
if (!ciYamlFile.existsSync()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we bar batch release based on .ci.yaml? IIRC, batch release infra are mostly based on GA and didn't rely on ci.yaml. Or is this a indirect way to disable the batch release for core-package?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the next non-new line in this file unconditionally reads ciYamlFile, and that will explode if, as is currently the case in core-packages, there is no such file.

Rather than speculatively implement an alternate codepath that does the same validation for a non-LUCI-based configuration that we've never built and may never need, I'm just describing the actual state of the tooling.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh ok, I think we can just return without the print error then.

The reason behind this check for batch release is the we have to add a enable_branch in ci.yaml to make sure the luci ci runs.

If it doesn't exist, then we don't need to worry about having the enable_branch check.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't the same basic requirement apply to a repo using GA to run CI, given that GA also has branch restrictions in configurations?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah the same will apply if we use GA to run ci, but this helper method is specific for ci.yaml.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see the reasoning, so until we implement the GA restriction, batch release is not supported. yeah I think that makes sense too.

if (isBatchRelease) {
printError(
'${_indentation}Batched release is currently only supported for '
'repos using .ci.yaml',
);
errors.add('Missing .ci.yaml');
}
return errors;
}

final String content = ciYamlFile.readAsStringSync();
final yaml = loadYaml(content) as YamlMap;

Expand Down
2 changes: 1 addition & 1 deletion script/tool/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: flutter_plugin_tools
description: Productivity and CI utils for Flutter team package repositories.
repository: https://github.com/flutter/packages/tree/main/script/tool
version: 0.14.0
version: 0.14.1

dependencies:
args: ^2.1.0
Expand Down
4 changes: 4 additions & 0 deletions script/tool/test/util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ void setToolConfig(
Directory repoRoot, {
String repoName = 'flutter/packages',
String? minFlutterVersion,
String? minDartVersion,
List<String>? pinnedDependencies,
List<String>? unpinnedDependencies,
Map<String, String>? packageLabels,
Expand All @@ -606,6 +607,9 @@ void setToolConfig(
if (minFlutterVersion != null) {
editor.update(['min_flutter'], minFlutterVersion);
}
if (minDartVersion != null) {
editor.update(['min_dart'], minDartVersion);
}
if (pinnedDependencies != null || unpinnedDependencies != null) {
const allowedDependenciesKey = 'allowed_dependencies';
const pinnedKey = 'pinned';
Expand Down
Loading
Loading