From 2107fef430dc6d4b13bf70e129206b6dcb536fec Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Mon, 8 Nov 2021 15:28:07 -0800 Subject: [PATCH] [flutter_conductor] ensure release branch point is always tagged (#93082) --- dev/conductor/core/lib/src/globals.dart | 2 + dev/conductor/core/lib/src/start.dart | 71 ++++++++++++++++++------- dev/conductor/core/test/start_test.dart | 19 ++++++- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/dev/conductor/core/lib/src/globals.dart b/dev/conductor/core/lib/src/globals.dart index 82396c9df1..c2a4391a3f 100644 --- a/dev/conductor/core/lib/src/globals.dart +++ b/dev/conductor/core/lib/src/globals.dart @@ -8,6 +8,8 @@ import 'proto/conductor_state.pb.dart' as pb; const String gsutilBinary = 'gsutil.py'; +const String kFrameworkDefaultBranch = 'master'; + const List kReleaseChannels = [ 'stable', 'beta', diff --git a/dev/conductor/core/lib/src/start.dart b/dev/conductor/core/lib/src/start.dart index 91134567d0..9977290488 100644 --- a/dev/conductor/core/lib/src/start.dart +++ b/dev/conductor/core/lib/src/start.dart @@ -362,26 +362,9 @@ class StartContext { framework.upstreamRemote.name, candidateBranch, exact: false, ))..ensureValid(candidateBranch, incrementLetter); - Version nextVersion; - if (incrementLetter == 'm') { - nextVersion = Version.fromCandidateBranch(candidateBranch); - } else { - if (incrementLetter == 'z') { - if (lastVersion.type == VersionType.stable) { - nextVersion = Version.increment(lastVersion, incrementLetter); - } else { - // This is the first stable release, so hardcode the z as 0 - nextVersion = Version( - x: lastVersion.x, - y: lastVersion.y, - z: 0, - type: VersionType.stable, - ); - } - } else { - nextVersion = Version.increment(lastVersion, incrementLetter); - } - } + Version nextVersion = calculateNextVersion(lastVersion); + nextVersion = await ensureBranchPointTagged(nextVersion, framework); + state.releaseVersion = nextVersion.toString(); final String frameworkHead = await framework.reverseParse('HEAD'); @@ -417,6 +400,54 @@ class StartContext { state_import.writeStateToFile(stateFile, state, logs); } + /// Determine this release's version number from the [lastVersion] and the [incrementLetter]. + Version calculateNextVersion(Version lastVersion) { + if (incrementLetter == 'm') { + return Version.fromCandidateBranch(candidateBranch); + } + 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); + } + + /// 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 ensureBranchPointTagged( + Version requestedVersion, + FrameworkRepository framework, + ) async { + if (incrementLetter != 'm') { + // in this case, there must have been a previous tagged release, so skip + // tagging the branch point + return requestedVersion; + } + final String branchPoint = await framework.branchPoint( + candidateBranch, + kFrameworkDefaultBranch, + ); + stdio.printStatus('Applying the tag $requestedVersion at the branch point $branchPoint'); + await framework.tag( + branchPoint, + requestedVersion.toString(), + frameworkUpstream, + ); + final Version nextVersion = Version.increment(requestedVersion, 'n'); + stdio.printStatus('The actual release will be version $nextVersion.'); + return nextVersion; + } + // To minimize merge conflicts, sort the commits by rev-list order. Future> _sortCherrypicks({ required Repository repository, diff --git a/dev/conductor/core/test/start_test.dart b/dev/conductor/core/test/start_test.dart index efbee1ff76..c1add2b14d 100644 --- a/dev/conductor/core/test/start_test.dart +++ b/dev/conductor/core/test/start_test.dart @@ -125,10 +125,14 @@ void main() { test('creates state file if provided correct inputs', () async { const String revision2 = 'def789'; const String revision3 = '123abc'; + const String branchPointRevision='deadbeef'; const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef'; const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e'; const String previousVersion = '1.2.0-1.0.pre'; - const String nextVersion = '1.2.0-3.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'; final Directory engine = fileSystem.directory(checkoutsParentDirectory) @@ -243,6 +247,16 @@ void main() { ], stdout: '$previousVersion-42-gabc123', ), + const FakeCommand( + command: ['git', 'merge-base', candidateBranch, 'master'], + stdout: branchPointRevision, + ), + const FakeCommand( + command: ['git', 'tag', branchPointTag, branchPointRevision], + ), + const FakeCommand( + command: ['git', 'push', FrameworkRepository.defaultUpstream, branchPointTag], + ), const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision3, @@ -300,6 +314,9 @@ void main() { 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); }); test('can convert from dev style version to stable version', () async {