From 38dbbb17f8cefb557b081d41a92d5c3ffbb4af55 Mon Sep 17 00:00:00 2001 From: Christopher Fujino <christopherfujino@gmail.com> Date: Tue, 1 Mar 2022 12:11:22 -0800 Subject: [PATCH] [flutter_conductor] deprecate increment (#99189) --- dev/conductor/README.md | 6 +- dev/conductor/core/lib/src/globals.dart | 2 - .../core/lib/src/proto/compile_proto.sh | 3 +- .../lib/src/proto/conductor_state.pb.dart | 24 ++- .../lib/src/proto/conductor_state.pbenum.dart | 23 ++ .../lib/src/proto/conductor_state.pbjson.dart | 18 +- .../core/lib/src/proto/conductor_state.proto | 21 +- dev/conductor/core/lib/src/start.dart | 137 +++++++----- dev/conductor/core/lib/src/version.dart | 13 +- dev/conductor/core/test/start_test.dart | 204 +++++++++--------- dev/conductor/core/test/state_test.dart | 4 +- dev/conductor/core/test/version_test.dart | 23 +- 12 files changed, 274 insertions(+), 204 deletions(-) diff --git a/dev/conductor/README.md b/dev/conductor/README.md index 54b3479312..528b8b5c64 100644 --- a/dev/conductor/README.md +++ b/dev/conductor/README.md @@ -44,9 +44,13 @@ conductor start \ --engine-cherrypicks=72114dafe28c8700f1d5d629c6ae9d34172ba395 \ --framework-cherrypicks=a3e66b396746f6581b2b7efd1b0d0f0074215128,d8d853436206e86f416236b930e97779b143a100 \ --dart-revision=4511eb2a779a612d9d6b2012123575013e0aef12 \ - --increment=m ``` +The conductor will, based on the release channel and the presence/lack of +previous tags, determine which part of the release version should be +incremented. In the cases where this is not correct, the version can be +overridden with `--version-override=3.0.0`. + For more details on these command line arguments, see `conductor help start`. This command will write to disk a state file that will persist until the release is completed. If you already have a persistent state file, this command will diff --git a/dev/conductor/core/lib/src/globals.dart b/dev/conductor/core/lib/src/globals.dart index f32ac5543c..e9c7bfa31c 100644 --- a/dev/conductor/core/lib/src/globals.dart +++ b/dev/conductor/core/lib/src/globals.dart @@ -16,8 +16,6 @@ const List<String> kBaseReleaseChannels = <String>['stable', 'beta']; const List<String> kReleaseChannels = <String>[...kBaseReleaseChannels, FrameworkRepository.defaultBranch]; -const List<String> kReleaseIncrements = <String>['y', 'z', 'm', 'n']; - const String kReleaseDocumentationUrl = 'https://github.com/flutter/flutter/wiki/Flutter-Cherrypick-Process'; const String kLuciPackagingConsoleLink = 'https://ci.chromium.org/p/flutter/g/packaging/console'; diff --git a/dev/conductor/core/lib/src/proto/compile_proto.sh b/dev/conductor/core/lib/src/proto/compile_proto.sh index 06cc5a145e..4ef9281dbd 100755 --- a/dev/conductor/core/lib/src/proto/compile_proto.sh +++ b/dev/conductor/core/lib/src/proto/compile_proto.sh @@ -5,7 +5,6 @@ # //flutter/dev/tools/lib/proto DIR="$(cd "$(dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -DARTFMT="$DIR/../../../../bin/cache/dart-sdk/bin/dartfmt" # Ensure dart-sdk is cached "$DIR/../../../../bin/dart" --version @@ -29,7 +28,7 @@ protoc --dart_out="$DIR" --proto_path="$DIR" "$DIR/conductor_state.proto" for SOURCE_FILE in $(ls "$DIR"/*.pb*.dart); do # Format in place file - "$DARTFMT" --overwrite --line-length 120 "$SOURCE_FILE" + dart format --output=write --line-length 120 "$SOURCE_FILE" # Create temp copy with the license header prepended cp "$DIR/license_header.txt" "${SOURCE_FILE}.tmp" diff --git a/dev/conductor/core/lib/src/proto/conductor_state.pb.dart b/dev/conductor/core/lib/src/proto/conductor_state.pb.dart index 1ab4e96b1a..2cc6554aaf 100644 --- a/dev/conductor/core/lib/src/proto/conductor_state.pb.dart +++ b/dev/conductor/core/lib/src/proto/conductor_state.pb.dart @@ -403,8 +403,12 @@ class ConductorState extends $pb.GeneratedMessage { enumValues: ReleasePhase.values) ..aOS(10, const $core.bool.fromEnvironment('protobuf.omit_field_names') ? '' : 'conductorVersion', protoName: 'conductorVersion') - ..aOS(11, const $core.bool.fromEnvironment('protobuf.omit_field_names') ? '' : 'incrementLevel', - protoName: 'incrementLevel') + ..e<ReleaseType>( + 11, const $core.bool.fromEnvironment('protobuf.omit_field_names') ? '' : 'releaseType', $pb.PbFieldType.OE, + protoName: 'releaseType', + defaultOrMaker: ReleaseType.STABLE_INITIAL, + valueOf: ReleaseType.valueOf, + enumValues: ReleaseType.values) ..hasRequiredFields = false; ConductorState._() : super(); @@ -418,7 +422,7 @@ class ConductorState extends $pb.GeneratedMessage { $core.Iterable<$core.String>? logs, ReleasePhase? currentPhase, $core.String? conductorVersion, - $core.String? incrementLevel, + ReleaseType? releaseType, }) { final _result = create(); if (releaseChannel != null) { @@ -448,8 +452,8 @@ class ConductorState extends $pb.GeneratedMessage { if (conductorVersion != null) { _result.conductorVersion = conductorVersion; } - if (incrementLevel != null) { - _result.incrementLevel = incrementLevel; + if (releaseType != null) { + _result.releaseType = releaseType; } return _result; } @@ -580,14 +584,14 @@ class ConductorState extends $pb.GeneratedMessage { void clearConductorVersion() => clearField(10); @$pb.TagNumber(11) - $core.String get incrementLevel => $_getSZ(9); + ReleaseType get releaseType => $_getN(9); @$pb.TagNumber(11) - set incrementLevel($core.String v) { - $_setString(9, v); + set releaseType(ReleaseType v) { + setField(11, v); } @$pb.TagNumber(11) - $core.bool hasIncrementLevel() => $_has(9); + $core.bool hasReleaseType() => $_has(9); @$pb.TagNumber(11) - void clearIncrementLevel() => clearField(11); + void clearReleaseType() => clearField(11); } diff --git a/dev/conductor/core/lib/src/proto/conductor_state.pbenum.dart b/dev/conductor/core/lib/src/proto/conductor_state.pbenum.dart index dfdc5f7074..7fc843527d 100644 --- a/dev/conductor/core/lib/src/proto/conductor_state.pbenum.dart +++ b/dev/conductor/core/lib/src/proto/conductor_state.pbenum.dart @@ -67,3 +67,26 @@ class CherrypickState extends $pb.ProtobufEnum { const CherrypickState._($core.int v, $core.String n) : super(v, n); } + +class ReleaseType extends $pb.ProtobufEnum { + static const ReleaseType STABLE_INITIAL = + ReleaseType._(0, const $core.bool.fromEnvironment('protobuf.omit_enum_names') ? '' : 'STABLE_INITIAL'); + static const ReleaseType STABLE_HOTFIX = + ReleaseType._(1, const $core.bool.fromEnvironment('protobuf.omit_enum_names') ? '' : 'STABLE_HOTFIX'); + static const ReleaseType BETA_INITIAL = + ReleaseType._(2, const $core.bool.fromEnvironment('protobuf.omit_enum_names') ? '' : 'BETA_INITIAL'); + static const ReleaseType BETA_HOTFIX = + ReleaseType._(3, const $core.bool.fromEnvironment('protobuf.omit_enum_names') ? '' : 'BETA_HOTFIX'); + + static const $core.List<ReleaseType> values = <ReleaseType>[ + STABLE_INITIAL, + STABLE_HOTFIX, + BETA_INITIAL, + BETA_HOTFIX, + ]; + + static final $core.Map<$core.int, ReleaseType> _byValue = $pb.ProtobufEnum.initByValue(values); + static ReleaseType? valueOf($core.int value) => _byValue[value]; + + const ReleaseType._($core.int v, $core.String n) : super(v, n); +} diff --git a/dev/conductor/core/lib/src/proto/conductor_state.pbjson.dart b/dev/conductor/core/lib/src/proto/conductor_state.pbjson.dart index f35275d028..d238e97734 100644 --- a/dev/conductor/core/lib/src/proto/conductor_state.pbjson.dart +++ b/dev/conductor/core/lib/src/proto/conductor_state.pbjson.dart @@ -44,6 +44,20 @@ const CherrypickState$json = const { /// Descriptor for `CherrypickState`. Decode as a `google.protobuf.EnumDescriptorProto`. final $typed_data.Uint8List cherrypickStateDescriptor = $convert.base64Decode( 'Cg9DaGVycnlwaWNrU3RhdGUSCwoHUEVORElORxAAEhkKFVBFTkRJTkdfV0lUSF9DT05GTElDVBABEg0KCUNPTVBMRVRFRBACEg0KCUFCQU5ET05FRBAD'); +@$core.Deprecated('Use releaseTypeDescriptor instead') +const ReleaseType$json = const { + '1': 'ReleaseType', + '2': const [ + const {'1': 'STABLE_INITIAL', '2': 0}, + const {'1': 'STABLE_HOTFIX', '2': 1}, + const {'1': 'BETA_INITIAL', '2': 2}, + const {'1': 'BETA_HOTFIX', '2': 3}, + ], +}; + +/// Descriptor for `ReleaseType`. Decode as a `google.protobuf.EnumDescriptorProto`. +final $typed_data.Uint8List releaseTypeDescriptor = $convert.base64Decode( + 'CgtSZWxlYXNlVHlwZRISCg5TVEFCTEVfSU5JVElBTBAAEhEKDVNUQUJMRV9IT1RGSVgQARIQCgxCRVRBX0lOSVRJQUwQAhIPCgtCRVRBX0hPVEZJWBAD'); @$core.Deprecated('Use remoteDescriptor instead') const Remote$json = const { '1': 'Remote', @@ -101,10 +115,10 @@ const ConductorState$json = const { const {'1': 'logs', '3': 8, '4': 3, '5': 9, '10': 'logs'}, const {'1': 'currentPhase', '3': 9, '4': 1, '5': 14, '6': '.conductor_state.ReleasePhase', '10': 'currentPhase'}, const {'1': 'conductorVersion', '3': 10, '4': 1, '5': 9, '10': 'conductorVersion'}, - const {'1': 'incrementLevel', '3': 11, '4': 1, '5': 9, '10': 'incrementLevel'}, + const {'1': 'releaseType', '3': 11, '4': 1, '5': 14, '6': '.conductor_state.ReleaseType', '10': 'releaseType'}, ], }; /// Descriptor for `ConductorState`. Decode as a `google.protobuf.DescriptorProto`. final $typed_data.Uint8List conductorStateDescriptor = $convert.base64Decode( - 'Cg5Db25kdWN0b3JTdGF0ZRImCg5yZWxlYXNlQ2hhbm5lbBgBIAEoCVIOcmVsZWFzZUNoYW5uZWwSJgoOcmVsZWFzZVZlcnNpb24YAiABKAlSDnJlbGVhc2VWZXJzaW9uEjMKBmVuZ2luZRgEIAEoCzIbLmNvbmR1Y3Rvcl9zdGF0ZS5SZXBvc2l0b3J5UgZlbmdpbmUSOQoJZnJhbWV3b3JrGAUgASgLMhsuY29uZHVjdG9yX3N0YXRlLlJlcG9zaXRvcnlSCWZyYW1ld29yaxIgCgtjcmVhdGVkRGF0ZRgGIAEoA1ILY3JlYXRlZERhdGUSKAoPbGFzdFVwZGF0ZWREYXRlGAcgASgDUg9sYXN0VXBkYXRlZERhdGUSEgoEbG9ncxgIIAMoCVIEbG9ncxJBCgxjdXJyZW50UGhhc2UYCSABKA4yHS5jb25kdWN0b3Jfc3RhdGUuUmVsZWFzZVBoYXNlUgxjdXJyZW50UGhhc2USKgoQY29uZHVjdG9yVmVyc2lvbhgKIAEoCVIQY29uZHVjdG9yVmVyc2lvbhImCg5pbmNyZW1lbnRMZXZlbBgLIAEoCVIOaW5jcmVtZW50TGV2ZWw='); + 'Cg5Db25kdWN0b3JTdGF0ZRImCg5yZWxlYXNlQ2hhbm5lbBgBIAEoCVIOcmVsZWFzZUNoYW5uZWwSJgoOcmVsZWFzZVZlcnNpb24YAiABKAlSDnJlbGVhc2VWZXJzaW9uEjMKBmVuZ2luZRgEIAEoCzIbLmNvbmR1Y3Rvcl9zdGF0ZS5SZXBvc2l0b3J5UgZlbmdpbmUSOQoJZnJhbWV3b3JrGAUgASgLMhsuY29uZHVjdG9yX3N0YXRlLlJlcG9zaXRvcnlSCWZyYW1ld29yaxIgCgtjcmVhdGVkRGF0ZRgGIAEoA1ILY3JlYXRlZERhdGUSKAoPbGFzdFVwZGF0ZWREYXRlGAcgASgDUg9sYXN0VXBkYXRlZERhdGUSEgoEbG9ncxgIIAMoCVIEbG9ncxJBCgxjdXJyZW50UGhhc2UYCSABKA4yHS5jb25kdWN0b3Jfc3RhdGUuUmVsZWFzZVBoYXNlUgxjdXJyZW50UGhhc2USKgoQY29uZHVjdG9yVmVyc2lvbhgKIAEoCVIQY29uZHVjdG9yVmVyc2lvbhI+CgtyZWxlYXNlVHlwZRgLIAEoDjIcLmNvbmR1Y3Rvcl9zdGF0ZS5SZWxlYXNlVHlwZVILcmVsZWFzZVR5cGU='); diff --git a/dev/conductor/core/lib/src/proto/conductor_state.proto b/dev/conductor/core/lib/src/proto/conductor_state.proto index c61d1c9728..cc32ca0e3c 100644 --- a/dev/conductor/core/lib/src/proto/conductor_state.proto +++ b/dev/conductor/core/lib/src/proto/conductor_state.proto @@ -45,6 +45,24 @@ enum CherrypickState { ABANDONED = 3; } +// The type of release that is being created. +// +// This determines how the version will be calculated. +enum ReleaseType { + // All pre-release metadata from previous beta releases will be discarded. The + // z must be 0. + STABLE_INITIAL = 0; + + // Increment z. + STABLE_HOTFIX = 1; + + // Compute x, y, and m from the candidate branch name. z and n should be 0. + BETA_INITIAL = 2; + + // Increment n. + BETA_HOTFIX = 3; +} + message Cherrypick { // The revision on trunk to cherrypick. string trunkRevision = 1; @@ -113,6 +131,5 @@ message ConductorState { // that created the [ConductorState] object. string conductorVersion = 10; - // One of x, y, z, m, or n. - string incrementLevel = 11; + ReleaseType releaseType = 11; } diff --git a/dev/conductor/core/lib/src/start.dart b/dev/conductor/core/lib/src/start.dart index 4187ea0223..9fb03e767f 100644 --- a/dev/conductor/core/lib/src/start.dart +++ b/dev/conductor/core/lib/src/start.dart @@ -6,6 +6,7 @@ import 'package:args/args.dart'; import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:fixnum/fixnum.dart'; +import 'package:meta/meta.dart'; import 'package:platform/platform.dart'; import 'package:process/process.dart'; @@ -13,7 +14,7 @@ import 'context.dart'; import 'git.dart'; import 'globals.dart'; import 'proto/conductor_state.pb.dart' as pb; -import 'proto/conductor_state.pbenum.dart' show ReleasePhase; +import 'proto/conductor_state.pbenum.dart'; import 'repository.dart'; import 'state.dart' as state_import; import 'stdio.dart'; @@ -26,10 +27,10 @@ const String kEngineUpstreamOption = 'engine-upstream'; const String kFrameworkCherrypicksOption = 'framework-cherrypicks'; const String kFrameworkMirrorOption = 'framework-mirror'; const String kFrameworkUpstreamOption = 'framework-upstream'; -const String kIncrementOption = 'increment'; const String kEngineMirrorOption = 'engine-mirror'; const String kReleaseOption = 'release-channel'; const String kStateOption = 'state-file'; +const String kVersionOverrideOption = 'version-override'; /// Command to print the status of the current Flutter release. class StartCommand extends Command<void> { @@ -89,23 +90,16 @@ class StartCommand extends Command<void> { kDartRevisionOption, help: 'New Dart revision to cherrypick.', ); - argParser.addOption( - kIncrementOption, - help: 'Specifies which part of the x.y.z version number to increment. Required.', - valueHelp: 'level', - allowed: kReleaseIncrements, - allowedHelp: <String, String>{ - 'y': 'Indicates the first dev release after a beta release.', - 'z': 'Indicates a hotfix to a stable release.', - 'm': 'Indicates a standard dev release.', - 'n': 'Indicates a hotfix to a dev or beta release.', - }, - ); argParser.addFlag( kForceFlag, abbr: 'f', help: 'Override all validations of the command line inputs.', ); + argParser.addOption( + kVersionOverrideOption, + help: 'Explicitly set the desired version. This should only be used if ' + 'the version computed by the tool is not correct.', + ); } final Checkouts checkouts; @@ -177,11 +171,6 @@ class StartCommand extends Command<void> { platform.environment, allowNull: true, ); - final String incrementLetter = getValueFromEnvOrArgs( - kIncrementOption, - argumentResults, - platform.environment, - )!; final bool force = getBoolFromEnvOrArgs( kForceFlag, argumentResults, @@ -190,6 +179,16 @@ class StartCommand extends Command<void> { final File stateFile = checkouts.fileSystem.file( getValueFromEnvOrArgs(kStateOption, argumentResults, platform.environment), ); + final String? versionOverrideString = getValueFromEnvOrArgs( + kVersionOverrideOption, + argumentResults, + platform.environment, + allowNull: true, + ); + Version? versionOverride; + if (versionOverrideString != null) { + versionOverride = Version.fromString(versionOverrideString); + } final StartContext context = StartContext( candidateBranch: candidateBranch, @@ -202,11 +201,11 @@ class StartCommand extends Command<void> { frameworkCherrypickRevisions: frameworkCherrypickRevisions, frameworkMirror: frameworkMirror, frameworkUpstream: frameworkUpstream, - incrementLetter: incrementLetter, processManager: processManager, releaseChannel: releaseChannel, stateFile: stateFile, force: force, + versionOverride: versionOverride, ); return context.run(); } @@ -226,12 +225,12 @@ class StartContext extends Context { required this.frameworkMirror, required this.frameworkUpstream, required this.conductorVersion, - required this.incrementLetter, required this.processManager, required this.releaseChannel, required Checkouts checkouts, required File stateFile, this.force = false, + this.versionOverride, }) : git = Git(processManager), engine = EngineRepository( checkouts, @@ -270,10 +269,10 @@ class StartContext extends Context { final String frameworkMirror; final String frameworkUpstream; final String conductorVersion; - final String incrementLetter; final Git git; final ProcessManager processManager; final String releaseChannel; + final Version? versionOverride; /// If validations should be overridden. final bool force; @@ -281,6 +280,26 @@ class StartContext extends Context { final EngineRepository engine; final FrameworkRepository framework; + /// Determine which part of the version to increment in the next release. + /// + /// If [atBranchPoint] is true, then this is a [ReleaseType.BETA_INITIAL]. + @visibleForTesting + ReleaseType computeReleaseType(Version lastVersion, bool atBranchPoint) { + if (atBranchPoint) { + return ReleaseType.BETA_INITIAL; + } + + if (releaseChannel == 'stable') { + if (lastVersion.type == VersionType.stable) { + return ReleaseType.STABLE_HOTFIX; + } else { + return ReleaseType.STABLE_INITIAL; + } + } + + return ReleaseType.BETA_HOTFIX; + } + Future<void> run() async { if (stateFile.existsSync()) { throw ConductorException('Error! A persistent state file already found at ${stateFile.path}.\n\n' @@ -299,7 +318,6 @@ class StartContext extends Context { state.releaseChannel = releaseChannel; state.createdDate = unixDate; state.lastUpdatedDate = unixDate; - state.incrementLevel = incrementLetter; // Create a new branch so that we don't accidentally push to upstream // candidateBranch. @@ -378,21 +396,34 @@ class StartContext extends Context { exact: false, )); + final String frameworkHead = await framework.reverseParse('HEAD'); + final String branchPoint = await framework.branchPoint(candidateBranch, FrameworkRepository.defaultBranch); + final bool atBranchPoint = branchPoint == frameworkHead; + + final ReleaseType releaseType = computeReleaseType(lastVersion, atBranchPoint); + state.releaseType = releaseType; + try { - lastVersion.ensureValid(candidateBranch, incrementLetter); + lastVersion.ensureValid(candidateBranch, releaseType); } on ConductorException catch (e) { // Let the user know, but resume execution stdio.printError(e.message); } - Version nextVersion = calculateNextVersion(lastVersion); - if (!force) { - nextVersion = await ensureBranchPointTagged(nextVersion, framework); + Version nextVersion; + if (versionOverride != null) { + nextVersion = versionOverride!; + } else { + nextVersion = calculateNextVersion(lastVersion, releaseType); + nextVersion = await ensureBranchPointTagged( + branchPoint: branchPoint, + requestedVersion: nextVersion, + framework: framework, + ); } state.releaseVersion = nextVersion.toString(); - final String frameworkHead = await framework.reverseParse('HEAD'); state.framework = pb.Repository( candidateBranch: candidateBranch, workingBranch: workingBranchName, @@ -416,37 +447,39 @@ class StartContext extends Context { } /// Determine this release's version number from the [lastVersion] and the [incrementLetter]. - Version calculateNextVersion(Version lastVersion) { - if (incrementLetter == 'm') { - return Version.fromCandidateBranch(candidateBranch); + Version calculateNextVersion(Version lastVersion, ReleaseType releaseType) { + late final Version nextVersion; + switch (releaseType) { + case ReleaseType.STABLE_INITIAL: + nextVersion = Version( + x: lastVersion.x, + y: lastVersion.y, + z: 0, + type: VersionType.stable, + ); + break; + case ReleaseType.STABLE_HOTFIX: + nextVersion = Version.increment(lastVersion, 'z'); + break; + case ReleaseType.BETA_INITIAL: + nextVersion = Version.fromCandidateBranch(candidateBranch); + break; + case ReleaseType.BETA_HOTFIX: + nextVersion = Version.increment(lastVersion, 'n'); + break; } - if (incrementLetter == 'z') { - if (lastVersion.type == VersionType.stable) { - return Version.increment(lastVersion, incrementLetter); - } - // This is the first stable release, so hardcode the z as 0 - return Version( - x: lastVersion.x, - y: lastVersion.y, - z: 0, - type: VersionType.stable, - ); - } - return Version.increment(lastVersion, incrementLetter); + return nextVersion; } /// Ensures the branch point [candidateBranch] and `master` has a version tag. /// /// This is necessary for version reporting for users on the `master` channel /// to be correct. - Future<Version> ensureBranchPointTagged( - Version requestedVersion, - FrameworkRepository framework, - ) async { - final String branchPoint = await framework.branchPoint( - candidateBranch, - FrameworkRepository.defaultBranch, - ); + Future<Version> ensureBranchPointTagged({ + required Version requestedVersion, + required String branchPoint, + required FrameworkRepository framework, + }) async { if (await framework.isCommitTagged(branchPoint)) { // The branch point is tagged, no work to be done return requestedVersion; diff --git a/dev/conductor/core/lib/src/version.dart b/dev/conductor/core/lib/src/version.dart index 8477b6d841..118f1707a0 100644 --- a/dev/conductor/core/lib/src/version.dart +++ b/dev/conductor/core/lib/src/version.dart @@ -2,7 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import './globals.dart' show ConductorException, kReleaseIncrements, releaseCandidateBranchRegex; +import 'globals.dart' show ConductorException, releaseCandidateBranchRegex; + +import 'proto/conductor_state.pbenum.dart'; /// Possible string formats that `flutter --version` can return. enum VersionType { @@ -262,10 +264,7 @@ class Version { /// /// Will throw a [ConductorException] if the version is not possible given the /// [candidateBranch] and [incrementLetter]. - void ensureValid(String candidateBranch, String incrementLetter) { - if (!kReleaseIncrements.contains(incrementLetter)) { - throw ConductorException('Invalid incrementLetter: $incrementLetter'); - } + void ensureValid(String candidateBranch, ReleaseType releaseType) { final RegExpMatch? branchMatch = releaseCandidateBranchRegex.firstMatch(candidateBranch); if (branchMatch == null) { throw ConductorException( @@ -292,12 +291,12 @@ class Version { } // stable type versions don't have an m field set - if (type != VersionType.stable && incrementLetter != 'm') { + if (type != VersionType.stable && releaseType != ReleaseType.STABLE_HOTFIX && releaseType != ReleaseType.STABLE_INITIAL) { final String branchM = branchMatch.group(3)!; if (m != int.tryParse(branchM)) { throw ConductorException( 'Parsed version ${toString()} has a different m value than candidate ' - 'branch $candidateBranch', + 'branch $candidateBranch with type $type', ); } } diff --git a/dev/conductor/core/test/start_test.dart b/dev/conductor/core/test/start_test.dart index ba937a5a62..ecdbfe53e1 100644 --- a/dev/conductor/core/test/start_test.dart +++ b/dev/conductor/core/test/start_test.dart @@ -5,9 +5,8 @@ import 'dart:convert' show jsonDecode; import 'package:args/command_runner.dart'; -import 'package:conductor_core/src/globals.dart'; import 'package:conductor_core/src/proto/conductor_state.pb.dart' as pb; -import 'package:conductor_core/src/proto/conductor_state.pbenum.dart' show ReleasePhase; +import 'package:conductor_core/src/proto/conductor_state.pbenum.dart'; import 'package:conductor_core/src/repository.dart'; import 'package:conductor_core/src/start.dart'; import 'package:conductor_core/src/state.dart'; @@ -25,7 +24,7 @@ void main() { const String frameworkMirror = 'https://github.com/user/flutter.git'; const String engineMirror = 'https://github.com/user/engine.git'; const String candidateBranch = 'flutter-1.2-candidate.3'; - const String releaseChannel = 'stable'; + const String releaseChannel = 'beta'; const String revision = 'abcd1234'; const String conductorVersion = 'deadbeef'; late Checkouts checkouts; @@ -97,8 +96,6 @@ void main() { 'beta', '--$kStateOption', '/path/to/statefile.json', - '--$kIncrementOption', - 'y', ]), throwsExceptionWith( 'Error! This tool is only supported on macOS and Linux', @@ -124,7 +121,36 @@ void main() { ); }); - test('does not fail if version wrong but --force provided', () async { + test('throws if provided an invalid --$kVersionOverrideOption', () async { + final CommandRunner<void> runner = createRunner(); + + final String stateFilePath = fileSystem.path.join( + platform.environment['HOME']!, + kStateFileName, + ); + + await expectLater( + () async => runner.run(<String>[ + 'start', + '--$kFrameworkMirrorOption', + frameworkMirror, + '--$kEngineMirrorOption', + engineMirror, + '--$kCandidateOption', + candidateBranch, + '--$kReleaseOption', + releaseChannel, + '--$kStateOption', + stateFilePath, + '--$kVersionOverrideOption', + 'an invalid version string', + ]), + throwsExceptionWith('an invalid version string cannot be parsed'), + ); + }); + + test('creates state file if provided correct inputs', () async { + stdio.stdin.add('y'); // accept prompt from ensureBranchPointTagged() const String revision2 = 'def789'; const String revision3 = '123abc'; const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef'; @@ -132,7 +158,7 @@ void main() { const String previousVersion = '1.2.0-1.0.pre'; // This is what this release will be const String nextVersion = '1.2.0-1.1.pre'; - const String incrementLevel = 'n'; + const String candidateBranch = 'flutter-1.2-candidate.1'; final Directory engine = fileSystem .directory(checkoutsParentDirectory) @@ -250,6 +276,14 @@ void main() { command: <String>['git', 'rev-parse', 'HEAD'], stdout: revision3, ), + const FakeCommand( + command: <String>['git', 'merge-base', candidateBranch, 'master'], + stdout: branchPointRevision, + ), + // check if commit is tagged, zero exit code means it is tagged + const FakeCommand( + command: <String>['git', 'describe', '--exact-match', '--tags', branchPointRevision], + ), ]; final CommandRunner<void> runner = createRunner( @@ -278,9 +312,6 @@ void main() { stateFilePath, '--$kDartRevisionOption', nextDartRevision, - '--$kIncrementOption', - incrementLevel, - '--$kForceFlag', ]); final File stateFile = fileSystem.file(stateFilePath); @@ -290,7 +321,9 @@ void main() { jsonDecode(stateFile.readAsStringSync()), ); - expect(processManager.hasRemainingExpectations, false); + expect(state.releaseType, ReleaseType.BETA_HOTFIX); + expect(stdio.error, isNot(contains('Tried to tag the branch point, however the target version'))); + expect(processManager, hasNoRemainingExpectations); expect(state.isInitialized(), true); expect(state.releaseChannel, releaseChannel); expect(state.releaseVersion, nextVersion); @@ -303,21 +336,17 @@ void main() { expect(state.framework.upstream.url, 'git@github.com:flutter/flutter.git'); expect(state.currentPhase, ReleasePhase.APPLY_ENGINE_CHERRYPICKS); expect(state.conductorVersion, conductorVersion); - expect(state.incrementLevel, incrementLevel); }); - test('creates state file if provided correct inputs', () async { + test('uses --$kVersionOverrideOption', () async { stdio.stdin.add('y'); // accept prompt from ensureBranchPointTagged() const String revision2 = 'def789'; const String revision3 = '123abc'; const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef'; const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e'; const String previousVersion = '1.2.0-1.0.pre'; - // This is a git tag applied to the branch point, not an actual release - const String branchPointTag = '1.2.0-3.0.pre'; - // This is what this release will be - const String nextVersion = '1.2.0-3.1.pre'; - const String incrementLevel = 'm'; + const String candidateBranch = 'flutter-1.2-candidate.1'; + const String versionOverride = '42.0.0-42.0.pre'; final Directory engine = fileSystem .directory(checkoutsParentDirectory) @@ -431,26 +460,14 @@ void main() { ], stdout: '$previousVersion-42-gabc123', ), - const FakeCommand( - command: <String>['git', 'merge-base', candidateBranch, 'master'], - stdout: branchPointRevision, - ), - // check if commit is tagged - const FakeCommand( - command: <String>['git', 'describe', '--exact-match', '--tags', branchPointRevision], - // non-zero exit means commit is not tagged - exitCode: 128, - ), - const FakeCommand( - command: <String>['git', 'tag', branchPointTag, branchPointRevision], - ), - const FakeCommand( - command: <String>['git', 'push', FrameworkRepository.defaultUpstream, branchPointTag], - ), const FakeCommand( command: <String>['git', 'rev-parse', 'HEAD'], stdout: revision3, ), + const FakeCommand( + command: <String>['git', 'merge-base', candidateBranch, 'master'], + stdout: branchPointRevision, + ), ]; final CommandRunner<void> runner = createRunner( @@ -479,8 +496,8 @@ void main() { stateFilePath, '--$kDartRevisionOption', nextDartRevision, - '--$kIncrementOption', - incrementLevel, + '--$kVersionOverrideOption', + versionOverride, ]); final File stateFile = fileSystem.file(stateFilePath); @@ -490,23 +507,8 @@ void main() { jsonDecode(stateFile.readAsStringSync()), ); - expect(processManager.hasRemainingExpectations, false); - expect(state.isInitialized(), true); - expect(state.releaseChannel, releaseChannel); - expect(state.releaseVersion, nextVersion); - expect(state.engine.candidateBranch, candidateBranch); - expect(state.engine.startingGitHead, revision2); - expect(state.engine.dartRevision, nextDartRevision); - expect(state.engine.upstream.url, 'git@github.com:flutter/engine.git'); - expect(state.framework.candidateBranch, candidateBranch); - expect(state.framework.startingGitHead, revision3); - expect(state.framework.upstream.url, 'git@github.com:flutter/flutter.git'); - expect(state.currentPhase, ReleasePhase.APPLY_ENGINE_CHERRYPICKS); - expect(state.conductorVersion, conductorVersion); - expect(state.incrementLevel, incrementLevel); - expect(stdio.stdout, contains('Applying the tag $branchPointTag at the branch point $branchPointRevision')); - expect(stdio.stdout, contains('The actual release will be version $nextVersion')); - expect(branchPointTag != nextVersion, true); + expect(processManager, hasNoRemainingExpectations); + expect(state.releaseVersion, versionOverride); }); test('logs to STDERR but does not fail on an unexpected candidate branch', () async { @@ -517,11 +519,8 @@ void main() { const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e'; // note that this significantly behind the candidate branch name const String previousVersion = '0.9.0-1.0.pre'; - // This is a git tag applied to the branch point, not an actual release - const String branchPointTag = '1.2.0-3.0.pre'; // This is what this release will be - const String nextVersion = '1.2.0-3.1.pre'; - const String incrementLevel = 'm'; + const String nextVersion = '0.9.0-1.1.pre'; final Directory engine = fileSystem .directory(checkoutsParentDirectory) @@ -635,25 +634,17 @@ void main() { ], stdout: '$previousVersion-42-gabc123', ), + const FakeCommand( + command: <String>['git', 'rev-parse', 'HEAD'], + stdout: revision3, + ), const FakeCommand( command: <String>['git', 'merge-base', candidateBranch, 'master'], stdout: branchPointRevision, ), - // check if commit is tagged + // check if commit is tagged, 0 exit code means it is tagged const FakeCommand( command: <String>['git', 'describe', '--exact-match', '--tags', branchPointRevision], - // non-zero exit means commit is not tagged - exitCode: 128, - ), - const FakeCommand( - command: <String>['git', 'tag', branchPointTag, branchPointRevision], - ), - const FakeCommand( - command: <String>['git', 'push', FrameworkRepository.defaultUpstream, branchPointTag], - ), - const FakeCommand( - command: <String>['git', 'rev-parse', 'HEAD'], - stdout: revision3, ), ]; @@ -683,8 +674,6 @@ void main() { stateFilePath, '--$kDartRevisionOption', nextDartRevision, - '--$kIncrementOption', - incrementLevel, ]); final File stateFile = fileSystem.file(stateFilePath); @@ -694,7 +683,8 @@ void main() { jsonDecode(stateFile.readAsStringSync()), ); - expect(processManager.hasRemainingExpectations, false); + expect(stdio.error, isNot(contains('Tried to tag the branch point, however'))); + expect(processManager, hasNoRemainingExpectations); expect(state.isInitialized(), true); expect(state.releaseChannel, releaseChannel); expect(state.releaseVersion, nextVersion); @@ -707,10 +697,7 @@ void main() { expect(state.framework.upstream.url, 'git@github.com:flutter/flutter.git'); expect(state.currentPhase, ReleasePhase.APPLY_ENGINE_CHERRYPICKS); expect(state.conductorVersion, conductorVersion); - expect(state.incrementLevel, incrementLevel); - expect(stdio.stdout, contains('Applying the tag $branchPointTag at the branch point $branchPointRevision')); - expect(stdio.stdout, contains('The actual release will be version $nextVersion')); - expect(branchPointTag != nextVersion, true); + expect(state.releaseType, ReleaseType.BETA_HOTFIX); expect(stdio.error, contains('Parsed version $previousVersion.42 has a different x value than candidate branch $candidateBranch')); }); @@ -721,7 +708,6 @@ void main() { const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e'; const String previousVersion = '1.2.0-3.0.pre'; const String nextVersion = '1.2.0'; - const String incrementLevel = 'z'; final Directory engine = fileSystem .directory(checkoutsParentDirectory) @@ -835,6 +821,10 @@ void main() { ], stdout: '$previousVersion-42-gabc123', ), + const FakeCommand( + command: <String>['git', 'rev-parse', 'HEAD'], + stdout: revision3, + ), const FakeCommand( command: <String>['git', 'merge-base', candidateBranch, 'master'], stdout: branchPointRevision, @@ -843,10 +833,6 @@ void main() { const FakeCommand( command: <String>['git', 'describe', '--exact-match', '--tags', branchPointRevision], ), - const FakeCommand( - command: <String>['git', 'rev-parse', 'HEAD'], - stdout: revision3, - ), ]; final CommandRunner<void> runner = createRunner( @@ -870,13 +856,11 @@ void main() { '--$kCandidateOption', candidateBranch, '--$kReleaseOption', - releaseChannel, + 'stable', '--$kStateOption', stateFilePath, '--$kDartRevisionOption', nextDartRevision, - '--$kIncrementOption', - incrementLevel, ]); final File stateFile = fileSystem.file(stateFilePath); @@ -888,7 +872,7 @@ void main() { expect(processManager.hasRemainingExpectations, false); expect(state.isInitialized(), true); - expect(state.releaseChannel, releaseChannel); + expect(state.releaseChannel, 'stable'); expect(state.releaseVersion, nextVersion); expect(state.engine.candidateBranch, candidateBranch); expect(state.engine.startingGitHead, revision2); @@ -897,7 +881,7 @@ void main() { expect(state.framework.startingGitHead, revision3); expect(state.currentPhase, ReleasePhase.APPLY_ENGINE_CHERRYPICKS); expect(state.conductorVersion, conductorVersion); - expect(state.incrementLevel, incrementLevel); + expect(state.releaseType, ReleaseType.STABLE_INITIAL); }); test('StartContext gets engine and framework checkout directories after run', () async { @@ -910,8 +894,6 @@ void main() { const String previousVersion = '1.2.0-1.0.pre'; // This is a git tag applied to the branch point, not an actual release const String branchPointTag = '1.2.0-3.0.pre'; - // This is what this release will be - const String incrementLevel = 'm'; final Directory engine = fileSystem .directory(checkoutsParentDirectory) @@ -1026,6 +1008,11 @@ void main() { ], stdout: '$previousVersion-42-gabc123', ), + // HEAD and branch point are same + const FakeCommand( + command: <String>['git', 'rev-parse', 'HEAD'], + stdout: branchPointRevision, + ), const FakeCommand( command: <String>['git', 'merge-base', candidateBranch, 'master'], stdout: branchPointRevision, @@ -1042,10 +1029,6 @@ void main() { const FakeCommand( command: <String>['git', 'push', FrameworkRepository.defaultUpstream, branchPointTag], ), - const FakeCommand( - command: <String>['git', 'rev-parse', 'HEAD'], - stdout: revision3, - ), ]; final String operatingSystem = const LocalPlatform().operatingSystem; @@ -1081,25 +1064,32 @@ void main() { ); final StartContext startContext = StartContext( - candidateBranch: candidateBranch, - checkouts: checkouts, - dartRevision: nextDartRevision, - engineCherrypickRevisions: <String>[], - engineMirror: engineMirror, - engineUpstream: EngineRepository.defaultUpstream, - frameworkCherrypickRevisions: <String>[], - frameworkMirror: frameworkMirror, - frameworkUpstream: FrameworkRepository.defaultUpstream, - releaseChannel: releaseChannel, - incrementLetter: incrementLevel, - processManager: processManager, - conductorVersion: conductorVersion, - stateFile: stateFile); + candidateBranch: candidateBranch, + checkouts: checkouts, + dartRevision: nextDartRevision, + engineCherrypickRevisions: <String>[], + engineMirror: engineMirror, + engineUpstream: EngineRepository.defaultUpstream, + frameworkCherrypickRevisions: <String>[], + frameworkMirror: frameworkMirror, + frameworkUpstream: FrameworkRepository.defaultUpstream, + releaseChannel: releaseChannel, + processManager: processManager, + conductorVersion: conductorVersion, + stateFile: stateFile, + ); await startContext.run(); + final pb.ConductorState state = pb.ConductorState(); + state.mergeFromProto3Json( + jsonDecode(stateFile.readAsStringSync()), + ); + expect((await startContext.engine.checkoutDirectory).path, equals(engine.path)); expect((await startContext.framework.checkoutDirectory).path, equals(framework.path)); + expect(state.releaseType, ReleaseType.BETA_INITIAL); + expect(processManager, hasNoRemainingExpectations); }); }, onPlatform: <String, dynamic>{ 'windows': const Skip('Flutter Conductor only supported on macos/linux'), diff --git a/dev/conductor/core/test/state_test.dart b/dev/conductor/core/test/state_test.dart index 2f4cfc7b54..89577326c5 100644 --- a/dev/conductor/core/test/state_test.dart +++ b/dev/conductor/core/test/state_test.dart @@ -18,7 +18,6 @@ void main() { final pb.ConductorState state = pb.ConductorState( releaseChannel: 'stable', releaseVersion: '2.3.4', - incrementLevel: 'z', engine: pb.Repository( candidateBranch: candidateBranch, upstream: pb.Remote( @@ -60,8 +59,7 @@ void main() { }, "logs": [ "[status] hello world" - ], - "incrementLevel": "z" + ] }'''; expect(serializedState, expectedString); }); diff --git a/dev/conductor/core/test/version_test.dart b/dev/conductor/core/test/version_test.dart index 6eae350af1..3d66ec91f9 100644 --- a/dev/conductor/core/test/version_test.dart +++ b/dev/conductor/core/test/version_test.dart @@ -2,9 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:conductor_core/src/proto/conductor_state.pbenum.dart'; import 'package:conductor_core/src/version.dart'; -import './common.dart'; +import 'common.dart'; void main() { group('Version.fromString()', () { @@ -106,7 +107,7 @@ void main() { const String candidateBranch = 'flutter-3.2-candidate.4'; final Version version = Version.fromString(versionString); expect( - () => version.ensureValid(candidateBranch, 'n'), + () => version.ensureValid(candidateBranch, ReleaseType.BETA_HOTFIX), throwsExceptionWith( 'Parsed version $versionString has a different x value than ' 'candidate branch $candidateBranch', @@ -119,7 +120,7 @@ void main() { const String candidateBranch = 'flutter-1.15-candidate.4'; final Version version = Version.fromString(versionString); expect( - () => version.ensureValid(candidateBranch, 'm'), + () => version.ensureValid(candidateBranch, ReleaseType.BETA_INITIAL), throwsExceptionWith( 'Parsed version $versionString has a different y value than ' 'candidate branch $candidateBranch', @@ -132,7 +133,7 @@ void main() { const String candidateBranch = 'flutter-1.2-candidate.0'; final Version version = Version.fromString(versionString); expect( - () => version.ensureValid(candidateBranch, 'n'), + () => version.ensureValid(candidateBranch, ReleaseType.BETA_HOTFIX), throwsExceptionWith( 'Parsed version $versionString has a different m value than ' 'candidate branch $candidateBranch', @@ -145,7 +146,7 @@ void main() { const String candidateBranch = 'flutter-1.2-candidate.98'; final Version version = Version.fromString(versionString); expect( - () => version.ensureValid(candidateBranch, 'n'), + () => version.ensureValid(candidateBranch, ReleaseType.STABLE_HOTFIX), isNot(throwsException), ); }); @@ -155,21 +156,11 @@ void main() { const String candidateBranch = 'stable'; final Version version = Version.fromString(versionString); expect( - () => version.ensureValid(candidateBranch, 'z'), + () => version.ensureValid(candidateBranch, ReleaseType.STABLE_HOTFIX), throwsExceptionWith( 'Candidate branch $candidateBranch does not match the pattern', ), ); }); - - test('does not validate m if incrementLetter is m', () { - const String versionString = '1.2.0-0.0.pre'; - const String candidateBranch = 'flutter-1.2-candidate.42'; - final Version version = Version.fromString(versionString); - expect( - () => version.ensureValid(candidateBranch, 'm'), - isNot(throwsException), - ); - }); }); } -- 2.21.0