From 764577f89a7ec5be78b89cf633257527a61f4a7c Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Fri, 12 Nov 2021 18:31:03 -0800 Subject: [PATCH] [flutter_conductor] allow --force to override validations on start sub-command (#93514) --- dev/conductor/core/lib/src/globals.dart | 13 ++ dev/conductor/core/lib/src/next.dart | 1 - dev/conductor/core/lib/src/start.dart | 22 ++- dev/conductor/core/lib/src/version.dart | 6 + dev/conductor/core/test/globals_test.dart | 66 ++++++++ dev/conductor/core/test/start_test.dart | 183 ++++++++++++++++++++++ 6 files changed, 289 insertions(+), 2 deletions(-) diff --git a/dev/conductor/core/lib/src/globals.dart b/dev/conductor/core/lib/src/globals.dart index c2a4391a3f018..87619d909910f 100644 --- a/dev/conductor/core/lib/src/globals.dart +++ b/dev/conductor/core/lib/src/globals.dart @@ -9,6 +9,7 @@ import 'proto/conductor_state.pb.dart' as pb; const String gsutilBinary = 'gsutil.py'; const String kFrameworkDefaultBranch = 'master'; +const String kForceFlag = 'force'; const List kReleaseChannels = [ 'stable', @@ -81,6 +82,18 @@ String? getValueFromEnvOrArgs( 'to be provided!'); } +bool getBoolFromEnvOrArgs( + String name, + ArgResults argResults, + Map env, +) { + final String envName = fromArgToEnvName(name); + if (env[envName] != null) { + return (env[envName]?.toUpperCase()) == 'TRUE'; + } + return argResults[name] as bool; +} + /// Return multiple values from the environment or fall back to [argResults]. /// /// Values read from an environment variable are assumed to be comma-delimited. diff --git a/dev/conductor/core/lib/src/next.dart b/dev/conductor/core/lib/src/next.dart index d6bd21114bd1c..29ef3c4d407ca 100644 --- a/dev/conductor/core/lib/src/next.dart +++ b/dev/conductor/core/lib/src/next.dart @@ -14,7 +14,6 @@ import './stdio.dart'; const String kStateOption = 'state-file'; const String kYesFlag = 'yes'; -const String kForceFlag = 'force'; /// Command to proceed from one [pb.ReleasePhase] to the next. class NextCommand extends Command { diff --git a/dev/conductor/core/lib/src/start.dart b/dev/conductor/core/lib/src/start.dart index 9977290488be7..47732aea000cf 100644 --- a/dev/conductor/core/lib/src/start.dart +++ b/dev/conductor/core/lib/src/start.dart @@ -102,6 +102,11 @@ class StartCommand extends Command { 'n': 'Indicates a hotfix to a dev or beta release.', }, ); + argParser.addFlag( + kForceFlag, + abbr: 'f', + help: 'Override all validations of the command line inputs.', + ); } final Checkouts checkouts; @@ -178,6 +183,11 @@ class StartCommand extends Command { argumentResults, platform.environment, )!; + final bool force = getBoolFromEnvOrArgs( + kForceFlag, + argumentResults, + platform.environment, + ); final File stateFile = checkouts.fileSystem.file( getValueFromEnvOrArgs(kStateOption, argumentResults, platform.environment), ); @@ -198,6 +208,7 @@ class StartCommand extends Command { releaseChannel: releaseChannel, stateFile: stateFile, stdio: stdio, + force: force, ); return context.run(); } @@ -223,6 +234,7 @@ class StartContext { required this.releaseChannel, required this.stateFile, required this.stdio, + this.force = false, }) : git = Git(processManager); final String candidateBranch; @@ -242,6 +254,9 @@ class StartContext { final File stateFile; final Stdio stdio; + /// If validations should be overridden. + final bool force; + Future run() async { if (stateFile.existsSync()) { throw ConductorException( @@ -361,7 +376,12 @@ class StartContext { final Version lastVersion = Version.fromString(await framework.getFullTag( framework.upstreamRemote.name, candidateBranch, exact: false, - ))..ensureValid(candidateBranch, incrementLetter); + )); + // [force] means we know this would fail but need to publish anyway + if (!force) { + lastVersion.ensureValid(candidateBranch, incrementLetter); + } + Version nextVersion = calculateNextVersion(lastVersion); nextVersion = await ensureBranchPointTagged(nextVersion, framework); diff --git a/dev/conductor/core/lib/src/version.dart b/dev/conductor/core/lib/src/version.dart index 0389a3f34f000..76c6005b9e8e6 100644 --- a/dev/conductor/core/lib/src/version.dart +++ b/dev/conductor/core/lib/src/version.dart @@ -238,12 +238,18 @@ class Version { final int y; /// Number of hotfix releases after a stable release. + /// + /// For non-stable releases, this will be 0. final int z; /// Zero-indexed count of dev releases after a beta release. + /// + /// For stable releases, this will be null. final int? m; /// Number of hotfixes required to make a dev release. + /// + /// For stable releases, this will be null. final int? n; /// Number of commits past last tagged dev release. diff --git a/dev/conductor/core/test/globals_test.dart b/dev/conductor/core/test/globals_test.dart index 60ce6e73d2713..a5fd147fbe2cd 100644 --- a/dev/conductor/core/test/globals_test.dart +++ b/dev/conductor/core/test/globals_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:args/args.dart'; import 'package:conductor_core/src/globals.dart'; import 'package:conductor_core/src/proto/conductor_state.pb.dart' as pb; @@ -129,4 +130,69 @@ void main() { ); }); }); + + group('getBoolFromEnvOrArgs', () { + const String flagName = 'a-cli-flag'; + + test('prefers env over argResults', () { + final ArgResults argResults = FakeArgs(results: { + flagName: false, + }); + final Map env = {'A_CLI_FLAG': 'TRUE'}; + final bool result = getBoolFromEnvOrArgs( + flagName, + argResults, + env, + ); + expect(result, true); + }); + + test('falls back to argResults if env is empty', () { + final ArgResults argResults = FakeArgs(results: { + flagName: false, + }); + final Map env = {}; + final bool result = getBoolFromEnvOrArgs( + flagName, + argResults, + env, + ); + expect(result, false); + }); + }); +} + +class FakeArgs implements ArgResults { + FakeArgs({ + this.arguments = const [], + this.name = 'fake-command', + this.results = const {}, + }); + + final Map results; + + @override + final List arguments; + + @override + final String name; + + @override + ArgResults? get command => throw Exception('Unimplemented'); + + @override + List get rest => throw Exception('Unimplemented'); + + @override + Iterable get options => throw Exception('Unimplemented'); + + @override + bool wasParsed(String name) { + return results[name] != null; + } + + @override + Object? operator[](String name) { + return results[name]; + } } diff --git a/dev/conductor/core/test/start_test.dart b/dev/conductor/core/test/start_test.dart index c1add2b14de36..431ce2983cfe7 100644 --- a/dev/conductor/core/test/start_test.dart +++ b/dev/conductor/core/test/start_test.dart @@ -5,6 +5,7 @@ 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/repository.dart'; @@ -122,6 +123,188 @@ void main() { ); }); + test('does not fail if version wrong but --force provided', () async { + 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 what this release will be + const String nextVersion = '1.2.0-1.1.pre'; + const String incrementLevel = 'n'; + + 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', '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, + '--$kForceFlag', + ]); + + 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); + }); + test('creates state file if provided correct inputs', () async { const String revision2 = 'def789'; const String revision3 = '123abc';