From fac90f44baf42558de44af86169346cbf7585e58 Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Fri, 25 Feb 2022 15:26:16 -0800 Subject: [PATCH] [flutter_conductor] log to stderr but do not crash conductor if branch name validation fails (#99181) --- dev/conductor/core/lib/src/start.dart | 7 +- dev/conductor/core/test/start_test.dart | 205 ++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 2 deletions(-) diff --git a/dev/conductor/core/lib/src/start.dart b/dev/conductor/core/lib/src/start.dart index fa4131ac282..4187ea0223a 100644 --- a/dev/conductor/core/lib/src/start.dart +++ b/dev/conductor/core/lib/src/start.dart @@ -377,9 +377,12 @@ class StartContext extends Context { candidateBranch, exact: false, )); - // [force] means we know this would fail but need to publish anyway - if (!force) { + + try { lastVersion.ensureValid(candidateBranch, incrementLetter); + } on ConductorException catch (e) { + // Let the user know, but resume execution + stdio.printError(e.message); } Version nextVersion = calculateNextVersion(lastVersion); diff --git a/dev/conductor/core/test/start_test.dart b/dev/conductor/core/test/start_test.dart index fcef75378d2..ba937a5a626 100644 --- a/dev/conductor/core/test/start_test.dart +++ b/dev/conductor/core/test/start_test.dart @@ -509,6 +509,211 @@ void main() { expect(branchPointTag != nextVersion, true); }); + test('logs to STDERR but does not fail on an unexpected candidate branch', () 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'; + // 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'; + + final Directory engine = fileSystem + .directory(checkoutsParentDirectory) + .childDirectory('flutter_conductor_checkouts') + .childDirectory('engine'); + + final File depsFile = engine.childFile('DEPS'); + + final List engineCommands = [ + FakeCommand( + command: [ + 'git', + 'clone', + '--origin', + 'upstream', + '--', + EngineRepository.defaultUpstream, + engine.path, + ], + onRun: () { + // Create the DEPS file which the tool will update + engine.createSync(recursive: true); + depsFile.writeAsStringSync(generateMockDeps(previousDartRevision)); + }), + const FakeCommand( + command: ['git', 'remote', 'add', 'mirror', engineMirror], + ), + const FakeCommand( + command: ['git', 'fetch', 'mirror'], + ), + const FakeCommand( + command: ['git', 'checkout', 'upstream/$candidateBranch'], + ), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision2, + ), + const FakeCommand( + command: [ + 'git', + 'checkout', + '-b', + 'cherrypicks-$candidateBranch', + ], + ), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM path/to/DEPS', + ), + const FakeCommand( + command: ['git', 'add', '--all'], + ), + const FakeCommand( + command: ['git', 'commit', "--message='Update Dart SDK to $nextDartRevision'"], + ), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision2, + ), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision2, + ), + ]; + + final List frameworkCommands = [ + FakeCommand( + command: [ + 'git', + 'clone', + '--origin', + 'upstream', + '--', + FrameworkRepository.defaultUpstream, + fileSystem.path.join( + checkoutsParentDirectory, + 'flutter_conductor_checkouts', + 'framework', + ), + ], + ), + const FakeCommand( + command: ['git', 'remote', 'add', 'mirror', frameworkMirror], + ), + const FakeCommand( + command: ['git', 'fetch', 'mirror'], + ), + const FakeCommand( + command: ['git', 'checkout', 'upstream/$candidateBranch'], + ), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision3, + ), + const FakeCommand( + command: [ + 'git', + 'checkout', + '-b', + 'cherrypicks-$candidateBranch', + ], + ), + const FakeCommand( + command: [ + 'git', + 'describe', + '--match', + '*.*.*', + '--tags', + 'refs/remotes/upstream/$candidateBranch', + ], + stdout: '$previousVersion-42-gabc123', + ), + const FakeCommand( + command: ['git', 'merge-base', candidateBranch, 'master'], + stdout: branchPointRevision, + ), + // check if commit is tagged + const FakeCommand( + command: ['git', 'describe', '--exact-match', '--tags', branchPointRevision], + // non-zero exit means commit is not tagged + exitCode: 128, + ), + const FakeCommand( + command: ['git', 'tag', branchPointTag, branchPointRevision], + ), + const FakeCommand( + command: ['git', 'push', FrameworkRepository.defaultUpstream, branchPointTag], + ), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision3, + ), + ]; + + final CommandRunner runner = createRunner( + commands: [ + ...engineCommands, + ...frameworkCommands, + ], + ); + + final String stateFilePath = fileSystem.path.join( + platform.environment['HOME']!, + kStateFileName, + ); + + await runner.run([ + 'start', + '--$kFrameworkMirrorOption', + frameworkMirror, + '--$kEngineMirrorOption', + engineMirror, + '--$kCandidateOption', + candidateBranch, + '--$kReleaseOption', + releaseChannel, + '--$kStateOption', + stateFilePath, + '--$kDartRevisionOption', + nextDartRevision, + '--$kIncrementOption', + incrementLevel, + ]); + + final File stateFile = fileSystem.file(stateFilePath); + + final pb.ConductorState state = pb.ConductorState(); + state.mergeFromProto3Json( + 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(stdio.error, contains('Parsed version $previousVersion.42 has a different x value than candidate branch $candidateBranch')); + }); + test('can convert from dev style version to stable version', () async { const String revision2 = 'def789'; const String revision3 = '123abc';