diff --git a/dev/conductor/lib/globals.dart b/dev/conductor/lib/globals.dart index 68267470fa..fa79b0f018 100644 --- a/dev/conductor/lib/globals.dart +++ b/dev/conductor/lib/globals.dart @@ -182,19 +182,25 @@ String getNewPrLink({ if (repoName == 'engine') { if (state.engine.dartRevision.isNotEmpty) { // shorten hashes to make final link manageable + // prefix with github org/repo so GitHub will auto-generate a hyperlink body.writeln('- Roll dart revision: dart-lang/sdk@${state.engine.dartRevision.substring(0, 9)}'); } - body.writeAll( - state.engine.cherrypicks.map((pb.Cherrypick cp) => '- commit: ${cp.trunkRevision.substring(0, 9)}'), - '\n', - ); + for (final pb.Cherrypick cp in state.engine.cherrypicks) { + // Only list commits that map to a commit that exists upstream. + if (cp.trunkRevision.isNotEmpty) { + body.writeln('- commit: flutter/engine@${cp.trunkRevision.substring(0, 9)}'); + } + } } else { - body.writeAll( - state.framework.cherrypicks.map((pb.Cherrypick cp) => '- commit: ${cp.trunkRevision.substring(0, 9)}'), - '\n', - ); + for (final pb.Cherrypick cp in state.framework.cherrypicks) { + // Only list commits that map to a commit that exists upstream. + if (cp.trunkRevision.isNotEmpty) { + body.writeln('- commit: ${cp.trunkRevision.substring(0, 9)}'); + } + } } - return 'https://github.com/flutter/$repoName/compare/$candidateBranch...$userName:$workingBranch?' + return 'https://github.com/flutter/$repoName/compare/' + '$candidateBranch...$userName:$workingBranch?' 'expand=1' '&title=${Uri.encodeQueryComponent(title)}' '&body=${Uri.encodeQueryComponent(body.toString())}'; diff --git a/dev/conductor/lib/next.dart b/dev/conductor/lib/next.dart index d3686c5145..ea9fb52778 100644 --- a/dev/conductor/lib/next.dart +++ b/dev/conductor/lib/next.dart @@ -95,11 +95,31 @@ void runNext({ switch (state.currentPhase) { case pb.ReleasePhase.APPLY_ENGINE_CHERRYPICKS: - final List unappliedCherrypicks = []; - for (final pb.Cherrypick cherrypick in state.engine.cherrypicks) { - if (!finishedStates.contains(cherrypick.state)) { - unappliedCherrypicks.add(cherrypick); - } + final Remote upstream = Remote( + name: RemoteName.upstream, + url: state.engine.upstream.url, + ); + final EngineRepository engine = EngineRepository( + checkouts, + initialRef: state.engine.workingBranch, + upstreamRemote: upstream, + previousCheckoutLocation: state.engine.checkoutPath, + ); + final String headRevision = engine.reverseParse('HEAD'); + + // check if the candidate branch is enabled in .ci.yaml + if (!engine.ciYaml.enabledBranches.contains(state.engine.candidateBranch)) { + engine.ciYaml.enableBranch(state.engine.candidateBranch); + // commit + final String revision = engine.commit( + 'add branch ${state.engine.candidateBranch} to enabled_branches in .ci.yaml', + addFirst: true, + ); + // append to list of cherrypicks so we know a PR is required + state.engine.cherrypicks.add(pb.Cherrypick( + appliedRevision: revision, + state: pb.CherrypickState.COMPLETED, + )); } if (!requiresEnginePR(state)) { @@ -109,6 +129,13 @@ void runNext({ break; } + final List unappliedCherrypicks = []; + for (final pb.Cherrypick cherrypick in state.engine.cherrypicks) { + if (!finishedStates.contains(cherrypick.state)) { + unappliedCherrypicks.add(cherrypick); + } + } + if (unappliedCherrypicks.isEmpty) { stdio.printStatus('All engine cherrypicks have been auto-applied by the conductor.\n'); } else { @@ -128,21 +155,11 @@ void runNext({ return; } } - final Remote upstream = Remote( - name: RemoteName.upstream, - url: state.engine.upstream.url, - ); - final EngineRepository engine = EngineRepository( - checkouts, - initialRef: state.engine.workingBranch, - upstreamRemote: upstream, - previousCheckoutLocation: state.engine.checkoutPath, - ); - final String headRevision = engine.reverseParse('HEAD'); engine.pushRef( fromRef: headRevision, - toRef: state.engine.workingBranch, + // Explicitly create new branch + toRef: 'refs/heads/${state.engine.workingBranch}', remote: state.engine.mirror.name, ); @@ -166,13 +183,6 @@ void runNext({ } break; case pb.ReleasePhase.APPLY_FRAMEWORK_CHERRYPICKS: - final List unappliedCherrypicks = []; - for (final pb.Cherrypick cherrypick in state.framework.cherrypicks) { - if (!finishedStates.contains(cherrypick.state)) { - unappliedCherrypicks.add(cherrypick); - } - } - if (state.engine.cherrypicks.isEmpty && state.engine.dartRevision.isEmpty) { stdio.printStatus( 'This release has no engine cherrypicks, and thus the engine.version file\n' @@ -213,11 +223,41 @@ void runNext({ ); final String headRevision = framework.reverseParse('HEAD'); + // Check if the current candidate branch is enabled + if (!framework.ciYaml.enabledBranches.contains(state.framework.candidateBranch)) { + framework.ciYaml.enableBranch(state.framework.candidateBranch); + // commit + final String revision = framework.commit( + 'add branch ${state.framework.candidateBranch} to enabled_branches in .ci.yaml', + addFirst: true, + ); + // append to list of cherrypicks so we know a PR is required + state.framework.cherrypicks.add(pb.Cherrypick( + appliedRevision: revision, + state: pb.CherrypickState.COMPLETED, + )); + } + stdio.printStatus('Rolling new engine hash $engineRevision to framework checkout...'); - framework.updateEngineRevision(engineRevision); - framework.commit( + final bool needsCommit = framework.updateEngineRevision(engineRevision); + if (needsCommit) { + final String revision = framework.commit( 'Update Engine revision to $engineRevision for ${state.releaseChannel} release ${state.releaseVersion}', - addFirst: true); + addFirst: true, + ); + // append to list of cherrypicks so we know a PR is required + state.framework.cherrypicks.add(pb.Cherrypick( + appliedRevision: revision, + state: pb.CherrypickState.COMPLETED, + )); + } + + final List unappliedCherrypicks = []; + for (final pb.Cherrypick cherrypick in state.framework.cherrypicks) { + if (!finishedStates.contains(cherrypick.state)) { + unappliedCherrypicks.add(cherrypick); + } + } if (state.framework.cherrypicks.isEmpty) { stdio.printStatus( @@ -251,7 +291,8 @@ void runNext({ framework.pushRef( fromRef: headRevision, - toRef: state.framework.workingBranch, + // Explicitly create new branch + toRef: 'refs/heads/${state.framework.workingBranch}', remote: state.framework.mirror.name, ); break; diff --git a/dev/conductor/lib/repository.dart b/dev/conductor/lib/repository.dart index 07aeab476f..bf33ea8bda 100644 --- a/dev/conductor/lib/repository.dart +++ b/dev/conductor/lib/repository.dart @@ -9,6 +9,7 @@ import 'package:file/file.dart'; import 'package:meta/meta.dart'; import 'package:platform/platform.dart'; import 'package:process/process.dart'; +import 'package:yaml/yaml.dart'; import './git.dart'; import './globals.dart'; @@ -69,6 +70,7 @@ abstract class Repository { 'Provided previousCheckoutLocation $previousCheckoutLocation does not exist on disk!'); } if (initialRef != null) { + assert(initialRef != ''); git.run( ['fetch', upstreamRemote.name], 'Fetch ${upstreamRemote.name} to ensure we have latest refs', @@ -398,6 +400,14 @@ abstract class Repository { bool addFirst = false, }) { assert(!message.contains("'")); + final bool hasChanges = git.getOutput( + ['status', '--porcelain'], + 'check for uncommitted changes', + workingDirectory: checkoutDirectory.path, + ).trim().isNotEmpty; + if (!hasChanges) { + throw ConductorException('Tried to commit with message $message but no changes were present'); + } if (addFirst) { git.run( ['add', '--all'], @@ -490,6 +500,7 @@ class FrameworkRepository extends Repository { } final Checkouts checkouts; + late final CiYaml ciYaml = CiYaml(checkoutDirectory.childFile('.ci.yaml')); static const String defaultUpstream = 'https://github.com/flutter/flutter.git'; @@ -586,7 +597,10 @@ class FrameworkRepository extends Repository { return Version.fromString(versionJson['frameworkVersion'] as String); } - void updateEngineRevision( + /// Update this framework's engine version file. + /// + /// Returns [true] if the version file was updated and a commit is needed. + bool updateEngineRevision( String newEngine, { @visibleForTesting File? engineVersionFile, }) { @@ -597,8 +611,19 @@ class FrameworkRepository extends Repository { .childFile('engine.version'); assert(engineVersionFile.existsSync()); final String oldEngine = engineVersionFile.readAsStringSync(); + if (oldEngine.trim() == newEngine.trim()) { + stdio.printTrace( + 'Tried to update the engine revision but version file is already up to date at: $newEngine', + ); + return false; + } stdio.printStatus('Updating engine revision from $oldEngine to $newEngine'); - engineVersionFile.writeAsStringSync(newEngine.trim(), flush: true); + engineVersionFile.writeAsStringSync( + // Version files have trailing newlines + '${newEngine.trim()}\n', + flush: true, + ); + return true; } } @@ -699,6 +724,8 @@ class EngineRepository extends Repository { final Checkouts checkouts; + late final CiYaml ciYaml = CiYaml(checkoutDirectory.childFile('.ci.yaml')); + static const String defaultUpstream = 'https://github.com/flutter/engine.git'; static const String defaultBranch = 'master'; @@ -766,3 +793,68 @@ class Checkouts { final ProcessManager processManager; final Stdio stdio; } + +class CiYaml { + CiYaml(this.file) { + if (!file.existsSync()) { + throw ConductorException('Could not find the .ci.yaml file at ${file.path}'); + } + } + + /// Underlying [File] that this object wraps. + final File file; + + /// Returns the raw string contents of this file. + /// + /// This is not cached as the contents can be written to while the conductor + /// is running. + String get stringContents => file.readAsStringSync(); + + /// Returns the parsed contents of the file as a [YamlMap]. + /// + /// This is not cached as the contents can be written to while the conductor + /// is running. + YamlMap get contents => loadYaml(stringContents) as YamlMap; + + List get enabledBranches { + final YamlList yamlList = contents['enabled_branches'] as YamlList; + return yamlList.map((dynamic element) { + return element as String; + }).toList(); + } + + static final RegExp _enabledBranchPattern = RegExp(r'^enabled_branches:'); + + /// Update this .ci.yaml file with the given branch name. + /// + /// The underlying [File] is written to, but not committed to git. This method + /// will throw a [ConductorException] if the [branchName] is already present + /// in the file or if the file does not have an "enabled_branches:" field. + void enableBranch(String branchName) { + final List newStrings = []; + if (enabledBranches.contains(branchName)) { + throw ConductorException('${file.path} already contains the branch $branchName'); + } + if (!_enabledBranchPattern.hasMatch(stringContents)) { + throw ConductorException( + 'Did not find the expected string "enabled_branches:" in the file ${file.path}', + ); + } + final List lines = stringContents.split('\n'); + bool insertedCurrentBranch = false; + for (final String line in lines) { + // Every existing line should be copied to the new Yaml + newStrings.add(line); + if (insertedCurrentBranch) { + continue; + } + if (_enabledBranchPattern.hasMatch(line)) { + insertedCurrentBranch = true; + // Indent two spaces + final String indent = ' ' * 2; + newStrings.add('$indent- ${branchName.trim()}'); + } + } + file.writeAsStringSync(newStrings.join('\n'), flush: true); + } +} diff --git a/dev/conductor/lib/state.dart b/dev/conductor/lib/state.dart index 9af4e8f16b..0143816a71 100644 --- a/dev/conductor/lib/state.dart +++ b/dev/conductor/lib/state.dart @@ -267,8 +267,7 @@ bool requiresEnginePR(pb.ConductorState state) { /// This release will require a new Framework PR. /// /// The logic is if there was an Engine PR OR there are framework cherrypicks -/// that have not been abandoned OR the increment level is 'm', then return -/// true, else false. +/// that have not been abandoned. bool requiresFrameworkPR(pb.ConductorState state) { if (requiresEnginePR(state)) { return true; @@ -278,9 +277,5 @@ bool requiresFrameworkPR(pb.ConductorState state) { if (hasRequiredCherrypicks) { return true; } - if (state.incrementLevel == 'm') { - // requires an update to .ci.yaml - return true; - } return false; } diff --git a/dev/conductor/test/globals_test.dart b/dev/conductor/test/globals_test.dart index cf28b19056..7d01934686 100644 --- a/dev/conductor/test/globals_test.dart +++ b/dev/conductor/test/globals_test.dart @@ -89,8 +89,9 @@ void main() { ## Scheduled Cherrypicks - Roll dart revision: dart-lang/sdk@${dartRevision.substring(0, 9)} -- commit: ${engineCherrypick1.substring(0, 9)} -- commit: ${engineCherrypick2.substring(0, 9)}'''; +- commit: flutter/engine@${engineCherrypick1.substring(0, 9)} +- commit: flutter/engine@${engineCherrypick2.substring(0, 9)} +'''; expect( Uri.decodeQueryComponent(bodyPattern.firstMatch(link)?.group(1) ?? ''), expectedBody, @@ -120,7 +121,8 @@ void main() { ## Scheduled Cherrypicks -- commit: ${frameworkCherrypick.substring(0, 9)}'''; +- commit: ${frameworkCherrypick.substring(0, 9)} +'''; expect( Uri.decodeQueryComponent(bodyPattern.firstMatch(link)?.group(1) ?? ''), expectedBody, diff --git a/dev/conductor/test/next_test.dart b/dev/conductor/test/next_test.dart index a96bcea2bf..ef73409d39 100644 --- a/dev/conductor/test/next_test.dart +++ b/dev/conductor/test/next_test.dart @@ -21,11 +21,13 @@ void main() { const String checkoutsParentDirectory = '$flutterRoot/dev/conductor'; const String candidateBranch = 'flutter-1.2-candidate.3'; const String workingBranch = 'cherrypicks-$candidateBranch'; + const String remoteUrl = 'https://github.com/org/repo.git'; final String localPathSeparator = const LocalPlatform().pathSeparator; final String localOperatingSystem = const LocalPlatform().pathSeparator; const String revision1 = 'd3af60d18e01fcb36e0c0fa06c8502e4935ed095'; const String revision2 = 'f99555c1e1392bf2a8135056b9446680c2af4ddf'; const String revision3 = '98a5ca242b9d270ce000b26309b8a3cdc9c89df5'; + const String revision4 = '280e23318a0d8341415c66aa32581352a421d974'; const String releaseVersion = '1.2.0-3.0.pre'; const String releaseChannel = 'beta'; late MemoryFileSystem fileSystem; @@ -77,9 +79,16 @@ void main() { group('APPLY_ENGINE_CHERRYPICKS to CODESIGN_ENGINE_BINARIES', () { test('does not prompt user and updates currentPhase if there are no engine cherrypicks', () async { - final FakeProcessManager processManager = FakeProcessManager.list( - [], - ); + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand(command: ['git', 'fetch', 'upstream']), + const FakeCommand( + command: ['git', 'checkout', workingBranch], + ), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision1, + ), + ]); final FakePlatform platform = FakePlatform( environment: { 'HOME': ['path', 'to', 'home'].join(localPathSeparator), @@ -87,10 +96,18 @@ void main() { operatingSystem: localOperatingSystem, pathSeparator: localPathSeparator, ); + final File ciYaml = fileSystem.file('$checkoutsParentDirectory/engine/.ci.yaml') + ..createSync(recursive: true); + // this branch already present in ciYaml + _initializeCiYamlFile(ciYaml, enabledBranches: [candidateBranch]); final pb.ConductorState state = pb.ConductorState( currentPhase: ReleasePhase.APPLY_ENGINE_CHERRYPICKS, engine: pb.Repository( + candidateBranch: candidateBranch, + checkoutPath: fileSystem.path.join(checkoutsParentDirectory, 'engine'), + workingBranch: workingBranch, startingGitHead: revision1, + upstream: pb.Remote(name: 'upstream', url: remoteUrl), ), ); writeStateToFile( @@ -125,9 +142,32 @@ void main() { }); test('confirms to stdout when all engine cherrypicks were auto-applied', () async { - const String remoteUrl = 'https://github.com/org/repo.git'; stdio.stdin.add('n'); - final FakeProcessManager processManager = FakeProcessManager.empty(); + final File ciYaml = fileSystem.file('$checkoutsParentDirectory/engine/.ci.yaml') + ..createSync(recursive: true); + _initializeCiYamlFile(ciYaml); + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand(command: ['git', 'fetch', 'upstream']), + const FakeCommand(command: ['git', 'checkout', workingBranch]), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision1, + ), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM blah', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: [ + 'git', + 'commit', + "--message='add branch $candidateBranch to enabled_branches in .ci.yaml'", + ]), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision2, + ), + ]); final FakePlatform platform = FakePlatform( environment: { 'HOME': ['path', 'to', 'home'].join(localPathSeparator), @@ -137,12 +177,14 @@ void main() { ); final pb.ConductorState state = pb.ConductorState( engine: pb.Repository( + candidateBranch: candidateBranch, cherrypicks: [ pb.Cherrypick( trunkRevision: 'abc123', state: pb.CherrypickState.COMPLETED, ), ], + checkoutPath: fileSystem.path.join(checkoutsParentDirectory, 'engine'), workingBranch: workingBranch, upstream: pb.Remote(name: 'upstream', url: remoteUrl), mirror: pb.Remote(name: 'mirror', url: remoteUrl), @@ -183,12 +225,29 @@ void main() { const FakeCommand( command: ['git', 'fetch', 'upstream'], ), - const FakeCommand(command: ['git', 'checkout', workingBranch]), + FakeCommand( + command: const ['git', 'checkout', workingBranch], + onRun: () { + final File file = fileSystem.file('$checkoutsParentDirectory/engine/.ci.yaml') + ..createSync(recursive: true); + _initializeCiYamlFile(file); + }, + ), const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision1, ), - const FakeCommand(command: ['git', 'push', 'mirror', '$revision1:$workingBranch']), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM .ci.yaml', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: ['git', 'commit', "--message='add branch $candidateBranch to enabled_branches in .ci.yaml'"]), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision2, + ), + const FakeCommand(command: ['git', 'push', 'mirror', '$revision1:refs/heads/$workingBranch']), ]); final FakePlatform platform = FakePlatform( environment: { @@ -201,6 +260,7 @@ void main() { currentPhase: ReleasePhase.APPLY_ENGINE_CHERRYPICKS, engine: pb.Repository( candidateBranch: candidateBranch, + checkoutPath: fileSystem.path.join(checkoutsParentDirectory, 'engine'), cherrypicks: [ pb.Cherrypick( trunkRevision: revision2, @@ -219,9 +279,11 @@ void main() { state, [], ); + // engine dir is expected to already exist + fileSystem.directory(checkoutsParentDirectory).childDirectory('engine').createSync(recursive: true); final Checkouts checkouts = Checkouts( fileSystem: fileSystem, - parentDirectory: fileSystem.directory(checkoutsParentDirectory)..createSync(recursive: true), + parentDirectory: fileSystem.directory(checkoutsParentDirectory), platform: platform, processManager: processManager, stdio: stdio, @@ -454,29 +516,54 @@ void main() { test('with no engine cherrypicks but a dart revision update, updates engine revision', () async { stdio.stdin.add('n'); - processManager.addCommands(const [ - FakeCommand(command: ['git', 'fetch', 'upstream']), + processManager.addCommands([ + const FakeCommand(command: ['git', 'fetch', 'upstream']), // we want merged upstream commit, not local working commit - FakeCommand(command: ['git', 'checkout', 'upstream/$candidateBranch']), - FakeCommand( + const FakeCommand(command: ['git', 'checkout', 'upstream/$candidateBranch']), + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision1, ), - FakeCommand(command: ['git', 'fetch', 'upstream']), - FakeCommand(command: ['git', 'checkout', workingBranch]), + const FakeCommand(command: ['git', 'fetch', 'upstream']), FakeCommand( + command: const ['git', 'checkout', workingBranch], + onRun: () { + final File file = fileSystem.file('$checkoutsParentDirectory/framework/.ci.yaml') + ..createSync(); + _initializeCiYamlFile(file); + }, + ), + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision2, ), - FakeCommand(command: ['git', 'add', '--all']), - FakeCommand(command: [ + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM /path/to/.ci.yaml', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: [ + 'git', + 'commit', + "--message='add branch $candidateBranch to enabled_branches in .ci.yaml'", + ]), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision3, + ), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM /path/to/engine.version', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: [ 'git', 'commit', "--message='Update Engine revision to $revision1 for $releaseChannel release $releaseVersion'", ]), - FakeCommand( + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], - stdout: revision3, + stdout: revision4, ), ]); final pb.ConductorState state = pb.ConductorState( @@ -518,34 +605,59 @@ void main() { expect(processManager, hasNoRemainingExpectations); expect(stdio.stdout, contains('Updating engine revision from $oldEngineVersion to $revision1')); - expect(stdio.stdout, contains('a framework PR is still\nrequired to roll engine cherrypicks.')); + expect(stdio.stdout, contains('Are you ready to push your framework branch')); }); test('does not update state.currentPhase if user responds no', () async { stdio.stdin.add('n'); - processManager.addCommands(const [ - FakeCommand(command: ['git', 'fetch', 'upstream']), + processManager.addCommands([ + const FakeCommand(command: ['git', 'fetch', 'upstream']), // we want merged upstream commit, not local working commit - FakeCommand(command: ['git', 'checkout', 'upstream/$candidateBranch']), FakeCommand( + command: const ['git', 'checkout', 'upstream/$candidateBranch'], + onRun: () { + final File file = fileSystem.file('$checkoutsParentDirectory/framework/.ci.yaml') + ..createSync(); + _initializeCiYamlFile(file); + }, + ), + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision1, ), - FakeCommand(command: ['git', 'fetch', 'upstream']), - FakeCommand(command: ['git', 'checkout', workingBranch]), - FakeCommand( + const FakeCommand(command: ['git', 'fetch', 'upstream']), + const FakeCommand(command: ['git', 'checkout', workingBranch]), + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision2, ), - FakeCommand(command: ['git', 'add', '--all']), - FakeCommand(command: [ + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM path/to/.ci.yaml', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: [ + 'git', + 'commit', + "--message='add branch $candidateBranch to enabled_branches in .ci.yaml'", + ]), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision3, + ), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM path/to/engine.version', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: [ 'git', 'commit', "--message='Update Engine revision to $revision1 for $releaseChannel release $releaseVersion'", ]), - FakeCommand( + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], - stdout: revision3, + stdout: revision4, ), ]); writeStateToFile( @@ -578,34 +690,59 @@ void main() { test('updates state.currentPhase if user responds yes', () async { stdio.stdin.add('y'); - processManager.addCommands(const [ + processManager.addCommands([ // Engine repo - FakeCommand(command: ['git', 'fetch', 'upstream']), + const FakeCommand(command: ['git', 'fetch', 'upstream']), // we want merged upstream commit, not local working commit - FakeCommand(command: ['git', 'checkout', 'upstream/$candidateBranch']), - FakeCommand( + const FakeCommand(command: ['git', 'checkout', 'upstream/$candidateBranch']), + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision1, ), // Framework repo - FakeCommand(command: ['git', 'fetch', 'upstream']), - FakeCommand(command: ['git', 'checkout', workingBranch]), + const FakeCommand(command: ['git', 'fetch', 'upstream']), FakeCommand( + command: const ['git', 'checkout', workingBranch], + onRun: () { + final File file = fileSystem.file('$checkoutsParentDirectory/framework/.ci.yaml') + ..createSync(); + _initializeCiYamlFile(file); + }, + ), + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], stdout: revision2, ), - FakeCommand(command: ['git', 'add', '--all']), - FakeCommand(command: [ + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM path/to/.ci.yaml', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: [ + 'git', + 'commit', + "--message='add branch $candidateBranch to enabled_branches in .ci.yaml'", + ]), + const FakeCommand( + command: ['git', 'rev-parse', 'HEAD'], + stdout: revision3, + ), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM path/to/engine.version', + ), + const FakeCommand(command: ['git', 'add', '--all']), + const FakeCommand(command: [ 'git', 'commit', "--message='Update Engine revision to $revision1 for $releaseChannel release $releaseVersion'", ]), - FakeCommand( + const FakeCommand( command: ['git', 'rev-parse', 'HEAD'], - stdout: revision3, + stdout: revision4, ), - FakeCommand( - command: ['git', 'push', 'mirror', '$revision2:$workingBranch'], + const FakeCommand( + command: ['git', 'push', 'mirror', '$revision2:refs/heads/$workingBranch'], ), ]); writeStateToFile( @@ -646,7 +783,7 @@ void main() { ); expect( stdio.stdout, - contains('Executed command: `git push mirror $revision2:$workingBranch`'), + contains('Executed command: `git push mirror $revision2:refs/heads/$workingBranch`'), ); expect(stdio.error, isEmpty); }); @@ -941,3 +1078,34 @@ void main() { 'windows': const Skip('Flutter Conductor only supported on macos/linux'), }); } + +void _initializeCiYamlFile( + File file, { + List? enabledBranches, +}) { + enabledBranches ??= ['master', 'dev', 'beta', 'stable']; + file.createSync(recursive: true); + final StringBuffer buffer = StringBuffer('enabled_branches:\n'); + for (final String branch in enabledBranches) { + buffer.writeln(' - $branch'); + } + buffer.writeln(''' + +platform_properties: + linux: + properties: + caches: ["name":"openjdk","path":"java"] + +targets: + - name: Linux analyze + recipe: flutter/flutter + timeout: 60 + properties: + tags: > + ["framework","hostonly"] + validation: analyze + validation_name: Analyze + scheduler: luci +'''); + file.writeAsStringSync(buffer.toString()); +} diff --git a/dev/conductor/test/repository_test.dart b/dev/conductor/test/repository_test.dart index 0a5e88bbb5..825d8ba47e 100644 --- a/dev/conductor/test/repository_test.dart +++ b/dev/conductor/test/repository_test.dart @@ -224,7 +224,7 @@ vars = { ); }); - test('commit() passes correct commit message', () { + test('commit() throws if there are no local changes to commit', () { const String commit1 = 'abc123'; const String commit2 = 'def456'; const String message = 'This is a commit message.'; @@ -251,6 +251,69 @@ vars = { 'rev-parse', 'HEAD', ], stdout: commit1), + const FakeCommand(command: [ + 'git', + 'status', + '--porcelain', + ]), + const FakeCommand(command: [ + 'git', + 'commit', + "--message='$message'", + ]), + const FakeCommand(command: [ + 'git', + 'rev-parse', + 'HEAD', + ], stdout: commit2), + ]); + + final Checkouts checkouts = Checkouts( + fileSystem: fileSystem, + parentDirectory: fileSystem.directory(rootDir), + platform: platform, + processManager: processManager, + stdio: stdio, + ); + + final EngineRepository repo = EngineRepository(checkouts); + expect( + () => repo.commit(message), + throwsExceptionWith('Tried to commit with message $message but no changes were present'), + ); + }); + + test('commit() passes correct commit message', () { + const String commit1 = 'abc123'; + const String commit2 = 'def456'; + const String message = 'This is a commit message.'; + final TestStdio stdio = TestStdio(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + final FakeProcessManager processManager = FakeProcessManager.list([ + FakeCommand(command: [ + 'git', + 'clone', + '--origin', + 'upstream', + '--', + EngineRepository.defaultUpstream, + fileSystem.path + .join(rootDir, 'flutter_conductor_checkouts', 'engine'), + ]), + const FakeCommand(command: [ + 'git', + 'checkout', + 'upstream/master', + ]), + const FakeCommand(command: [ + 'git', + 'rev-parse', + 'HEAD', + ], stdout: commit1), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM path/to/file.txt', + ), const FakeCommand(command: [ 'git', 'commit', @@ -274,6 +337,167 @@ vars = { final EngineRepository repo = EngineRepository(checkouts); repo.commit(message); }); + + test('updateEngineRevision() returns false if newCommit is the same as version file', () { + const String commit1 = 'abc123'; + const String commit2 = 'def456'; + final TestStdio stdio = TestStdio(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + final File engineVersionFile = fileSystem.file('/engine.version')..writeAsStringSync(commit2); + final FakeProcessManager processManager = FakeProcessManager.list([ + FakeCommand(command: [ + 'git', + 'clone', + '--origin', + 'upstream', + '--', + FrameworkRepository.defaultUpstream, + fileSystem.path + .join(rootDir, 'flutter_conductor_checkouts', 'framework'), + ]), + const FakeCommand(command: [ + 'git', + 'rev-parse', + 'HEAD', + ], stdout: commit1), + ]); + + final Checkouts checkouts = Checkouts( + fileSystem: fileSystem, + parentDirectory: fileSystem.directory(rootDir), + platform: platform, + processManager: processManager, + stdio: stdio, + ); + + final FrameworkRepository repo = FrameworkRepository(checkouts); + final bool didUpdate = repo.updateEngineRevision(commit2, engineVersionFile: engineVersionFile); + expect(didUpdate, false); + }); + + test('CiYaml(file) will throw if file does not exist', () { + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + final File file = fileSystem.file('/non/existent/file.txt'); + + expect( + () => CiYaml(file), + throwsExceptionWith('Could not find the .ci.yaml file at /non/existent/file.txt'), + ); + }); + + test('ciYaml.enableBranch() will prepend the given branch to the yaml list of enabled_branches', () { + const String commit1 = 'abc123'; + final TestStdio stdio = TestStdio(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + final File ciYaml = fileSystem.file('/flutter_conductor_checkouts/framework/.ci.yaml'); + final ProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ + 'git', + 'clone', + '--origin', + 'upstream', + '--', + FrameworkRepository.defaultUpstream, + fileSystem.path + .join(rootDir, 'flutter_conductor_checkouts', 'framework'), + ], + onRun: () { + ciYaml.createSync(recursive: true); + ciYaml.writeAsStringSync(''' +enabled_branches: + - master + - dev + - beta + - stable +'''); + }), + const FakeCommand(command: [ + 'git', + 'rev-parse', + 'HEAD', + ], stdout: commit1), + ]); + final Checkouts checkouts = Checkouts( + fileSystem: fileSystem, + parentDirectory: fileSystem.directory(rootDir), + platform: platform, + processManager: processManager, + stdio: stdio, + ); + + final FrameworkRepository framework = FrameworkRepository(checkouts); + expect( + framework.ciYaml.enabledBranches, + ['master', 'dev', 'beta', 'stable'], + ); + + framework.ciYaml.enableBranch('foo'); + expect( + framework.ciYaml.enabledBranches, + ['foo', 'master', 'dev', 'beta', 'stable'], + ); + + expect( + framework.ciYaml.stringContents, + ''' +enabled_branches: + - foo + - master + - dev + - beta + - stable +''' + ); + }); + + test('ciYaml.enableBranch() will throw if the input branch is already present in the yaml file', () { + const String commit1 = 'abc123'; + final TestStdio stdio = TestStdio(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + final File ciYaml = fileSystem.file('/flutter_conductor_checkouts/framework/.ci.yaml'); + final ProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ + 'git', + 'clone', + '--origin', + 'upstream', + '--', + FrameworkRepository.defaultUpstream, + fileSystem.path + .join(rootDir, 'flutter_conductor_checkouts', 'framework'), + ], + onRun: () { + ciYaml.createSync(recursive: true); + ciYaml.writeAsStringSync(''' +enabled_branches: + - master + - dev + - beta + - stable +'''); + }), + const FakeCommand(command: [ + 'git', + 'rev-parse', + 'HEAD', + ], stdout: commit1), + ]); + final Checkouts checkouts = Checkouts( + fileSystem: fileSystem, + parentDirectory: fileSystem.directory(rootDir), + platform: platform, + processManager: processManager, + stdio: stdio, + ); + + final FrameworkRepository framework = FrameworkRepository(checkouts); + expect( + () => framework.ciYaml.enableBranch('master'), + throwsExceptionWith('.ci.yaml already contains the branch master'), + ); + }); }); } diff --git a/dev/conductor/test/start_test.dart b/dev/conductor/test/start_test.dart index 24937d4bda..f56e789eea 100644 --- a/dev/conductor/test/start_test.dart +++ b/dev/conductor/test/start_test.dart @@ -161,6 +161,10 @@ void main() { 'cherrypicks-$candidateBranch', ], ), + const FakeCommand( + command: ['git', 'status', '--porcelain'], + stdout: 'MM path/to/DEPS', + ), const FakeCommand( command: ['git', 'add', '--all'], ), @@ -272,6 +276,7 @@ void main() { jsonDecode(stateFile.readAsStringSync()), ); + expect(processManager.hasRemainingExpectations, false); expect(state.isInitialized(), true); expect(state.releaseChannel, releaseChannel); expect(state.releaseVersion, nextVersion);