diff --git a/dev/conductor/core/lib/src/repository.dart b/dev/conductor/core/lib/src/repository.dart index bac53f4baf..99d64b9a60 100644 --- a/dev/conductor/core/lib/src/repository.dart +++ b/dev/conductor/core/lib/src/repository.dart @@ -418,50 +418,6 @@ abstract class Repository { return exitcode == 0; } - /// Determines if a commit will cherry-pick to current HEAD without conflict. - Future canCherryPick(String commit) async { - assert( - await gitCheckoutClean(), - 'cannot cherry-pick because git checkout ${(await checkoutDirectory).path} is not clean', - ); - - final int exitcode = await git.run( - ['cherry-pick', '--no-commit', commit], - 'attempt to cherry-pick $commit without committing', - allowNonZeroExitCode: true, - workingDirectory: (await checkoutDirectory).path, - ); - - final bool result = exitcode == 0; - - if (result == false) { - stdio.printError(await git.getOutput( - ['diff'], - 'get diff of failed cherry-pick', - workingDirectory: (await checkoutDirectory).path, - )); - } - - await reset('HEAD'); - return result; - } - - /// Cherry-pick a [commit] to the current HEAD. - /// - /// This method will throw a [GitException] if the command fails. - Future cherryPick(String commit) async { - assert( - await gitCheckoutClean(), - 'cannot cherry-pick because git checkout ${(await checkoutDirectory).path} is not clean', - ); - - await git.run( - ['cherry-pick', commit], - 'cherry-pick $commit', - workingDirectory: (await checkoutDirectory).path, - ); - } - /// Resets repository HEAD to [ref]. Future reset(String ref) async { await git.run( @@ -825,12 +781,6 @@ class HostFrameworkRepository extends FrameworkRepository { 'checkout not implemented for the host repository'); } - @override - Future cherryPick(String commit) async { - throw ConductorException( - 'cherryPick not implemented for the host repository'); - } - @override Future reset(String ref) async { throw ConductorException('reset not implemented for the host repository'); diff --git a/dev/conductor/core/lib/src/start.dart b/dev/conductor/core/lib/src/start.dart index d96c30ec81..90cdbd538d 100644 --- a/dev/conductor/core/lib/src/start.dart +++ b/dev/conductor/core/lib/src/start.dart @@ -22,9 +22,7 @@ import 'version.dart'; const String kCandidateOption = 'candidate-branch'; const String kDartRevisionOption = 'dart-revision'; -const String kEngineCherrypicksOption = 'engine-cherrypicks'; const String kEngineUpstreamOption = 'engine-upstream'; -const String kFrameworkCherrypicksOption = 'framework-cherrypicks'; const String kFrameworkMirrorOption = 'framework-mirror'; const String kFrameworkUpstreamOption = 'framework-upstream'; const String kEngineMirrorOption = 'engine-mirror'; @@ -80,16 +78,6 @@ class StartCommand extends Command { defaultsTo: defaultPath, help: 'Path to persistent state file. Defaults to $defaultPath', ); - argParser.addMultiOption( - kEngineCherrypicksOption, - help: 'Engine cherrypick hashes to be applied.', - defaultsTo: [], - ); - argParser.addMultiOption( - kFrameworkCherrypicksOption, - help: 'Framework cherrypick hashes to be applied.', - defaultsTo: [], - ); argParser.addOption( kDartRevisionOption, help: 'New Dart revision to cherrypick.', @@ -161,16 +149,6 @@ class StartCommand extends Command { argumentResults, platform.environment, )!; - final List frameworkCherrypickRevisions = getValuesFromEnvOrArgs( - kFrameworkCherrypicksOption, - argumentResults, - platform.environment, - ); - final List engineCherrypickRevisions = getValuesFromEnvOrArgs( - kEngineCherrypicksOption, - argumentResults, - platform.environment, - ); final String? dartRevision = getValueFromEnvOrArgs( kDartRevisionOption, argumentResults, @@ -201,11 +179,9 @@ class StartCommand extends Command { candidateBranch: candidateBranch, checkouts: checkouts, dartRevision: dartRevision, - engineCherrypickRevisions: engineCherrypickRevisions, engineMirror: engineMirror, engineUpstream: engineUpstream, conductorVersion: conductorVersion, - frameworkCherrypickRevisions: frameworkCherrypickRevisions, frameworkMirror: frameworkMirror, frameworkUpstream: frameworkUpstream, processManager: processManager, @@ -226,10 +202,8 @@ class StartContext extends Context { StartContext({ required this.candidateBranch, required this.dartRevision, - required this.engineCherrypickRevisions, required this.engineMirror, required this.engineUpstream, - required this.frameworkCherrypickRevisions, required this.frameworkMirror, required this.frameworkUpstream, required this.conductorVersion, @@ -268,10 +242,8 @@ class StartContext extends Context { final String candidateBranch; final String? dartRevision; - final List engineCherrypickRevisions; final String engineMirror; final String engineUpstream; - final List frameworkCherrypickRevisions; final String frameworkMirror; final String frameworkUpstream; final String conductorVersion; @@ -336,31 +308,7 @@ class StartContext extends Context { await engine.updateDartRevision(dartRevision!); await engine.commit('Update Dart SDK to $dartRevision', addFirst: true); } - final List engineCherrypicks = (await _sortCherrypicks( - repository: engine, - cherrypicks: engineCherrypickRevisions, - upstreamRef: EngineRepository.defaultBranch, - releaseRef: candidateBranch, - )) - .map((String revision) => pb.Cherrypick( - trunkRevision: revision, - state: pb.CherrypickState.PENDING, - )) - .toList(); - for (final pb.Cherrypick cherrypick in engineCherrypicks) { - final String revision = cherrypick.trunkRevision; - final bool success = await engine.canCherryPick(revision); - stdio.printTrace( - 'Attempt to cherrypick $revision ${success ? 'succeeded' : 'failed'}', - ); - if (success) { - await engine.cherryPick(revision); - cherrypick.state = pb.CherrypickState.COMPLETED; - } else { - cherrypick.state = pb.CherrypickState.PENDING_WITH_CONFLICT; - } - } final String engineHead = await engine.reverseParse('HEAD'); state.engine = pb.Repository( candidateBranch: candidateBranch, @@ -368,38 +316,12 @@ class StartContext extends Context { startingGitHead: engineHead, currentGitHead: engineHead, checkoutPath: (await engine.checkoutDirectory).path, - cherrypicks: engineCherrypicks, dartRevision: dartRevision, upstream: pb.Remote(name: 'upstream', url: engine.upstreamRemote.url), mirror: pb.Remote(name: 'mirror', url: engine.mirrorRemote!.url), ); await framework.newBranch(workingBranchName); - final List frameworkCherrypicks = (await _sortCherrypicks( - repository: framework, - cherrypicks: frameworkCherrypickRevisions, - upstreamRef: FrameworkRepository.defaultBranch, - releaseRef: candidateBranch, - )) - .map((String revision) => pb.Cherrypick( - trunkRevision: revision, - state: pb.CherrypickState.PENDING, - )) - .toList(); - - for (final pb.Cherrypick cherrypick in frameworkCherrypicks) { - final String revision = cherrypick.trunkRevision; - final bool success = await framework.canCherryPick(revision); - stdio.printTrace( - 'Attempt to cherrypick $cherrypick ${success ? 'succeeded' : 'failed'}', - ); - if (success) { - await framework.cherryPick(revision); - cherrypick.state = pb.CherrypickState.COMPLETED; - } else { - cherrypick.state = pb.CherrypickState.PENDING_WITH_CONFLICT; - } - } // Get framework version final Version lastVersion = Version.fromString(await framework.getFullTag( @@ -446,7 +368,6 @@ class StartContext extends Context { startingGitHead: frameworkHead, currentGitHead: frameworkHead, checkoutPath: (await framework.checkoutDirectory).path, - cherrypicks: frameworkCherrypicks, upstream: pb.Remote(name: 'upstream', url: framework.upstreamRemote.url), mirror: pb.Remote(name: 'mirror', url: framework.mirrorRemote!.url), ); @@ -530,74 +451,4 @@ class StartContext extends Context { 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, - required List cherrypicks, - required String upstreamRef, - required String releaseRef, - }) async { - if (cherrypicks.isEmpty) { - return cherrypicks; - } - - // Input cherrypick hashes that failed to be parsed by git. - final List unknownCherrypicks = []; - // Full 40-char hashes parsed by git. - final List validatedCherrypicks = []; - // Final, validated, sorted list of cherrypicks to be applied. - final List sortedCherrypicks = []; - for (final String cherrypick in cherrypicks) { - try { - final String fullRef = await repository.reverseParse(cherrypick); - validatedCherrypicks.add(fullRef); - } on GitException { - // Catch this exception so that we can validate the rest. - unknownCherrypicks.add(cherrypick); - } - } - - final String branchPoint = await repository.branchPoint( - '${repository.upstreamRemote.name}/$upstreamRef', - '${repository.upstreamRemote.name}/$releaseRef', - ); - - // `git rev-list` returns newest first, so reverse this list - final List upstreamRevlist = (await repository.revList([ - '--ancestry-path', - '$branchPoint..$upstreamRef', - ])) - .reversed - .toList(); - - stdio.printStatus('upstreamRevList:\n${upstreamRevlist.join('\n')}\n'); - stdio.printStatus( - 'validatedCherrypicks:\n${validatedCherrypicks.join('\n')}\n'); - for (final String upstreamRevision in upstreamRevlist) { - if (validatedCherrypicks.contains(upstreamRevision)) { - validatedCherrypicks.remove(upstreamRevision); - sortedCherrypicks.add(upstreamRevision); - if (unknownCherrypicks.isEmpty && validatedCherrypicks.isEmpty) { - return sortedCherrypicks; - } - } - } - - // We were given input cherrypicks that were not present in the upstream - // rev-list - stdio.printError( - 'The following ${repository.name} cherrypicks were not found in the ' - 'upstream $upstreamRef branch:', - ); - for (final String cp in [ - ...validatedCherrypicks, - ...unknownCherrypicks - ]) { - stdio.printError('\t$cp'); - } - throw ConductorException( - '${validatedCherrypicks.length + unknownCherrypicks.length} unknown cherrypicks provided!', - ); - } } diff --git a/dev/conductor/core/test/repository_test.dart b/dev/conductor/core/test/repository_test.dart index c7b508483c..bcddb514ba 100644 --- a/dev/conductor/core/test/repository_test.dart +++ b/dev/conductor/core/test/repository_test.dart @@ -31,161 +31,6 @@ void main() { stdio = TestStdio(); }); - test('canCherryPick returns true if git cherry-pick returns 0', () async { - processManager.addCommands([ - FakeCommand(command: [ - 'git', - 'clone', - '--origin', - 'upstream', - '--', - FrameworkRepository.defaultUpstream, - fileSystem.path - .join(rootDir, 'flutter_conductor_checkouts', 'framework'), - ]), - const FakeCommand(command: [ - 'git', - 'checkout', - FrameworkRepository.defaultBranch, - ]), - const FakeCommand(command: [ - 'git', - 'rev-parse', - 'HEAD', - ], stdout: revision), - const FakeCommand(command: [ - 'git', - 'status', - '--porcelain', - ]), - const FakeCommand(command: [ - 'git', - 'cherry-pick', - '--no-commit', - revision, - ]), - const FakeCommand(command: [ - 'git', - 'reset', - 'HEAD', - '--hard', - ]), - ]); - final Checkouts checkouts = Checkouts( - fileSystem: fileSystem, - parentDirectory: fileSystem.directory(rootDir), - platform: platform, - processManager: processManager, - stdio: stdio, - ); - final Repository repository = FrameworkRepository(checkouts); - expect(await repository.canCherryPick(revision), true); - }); - - test('canCherryPick returns false if git cherry-pick returns non-zero', () async { - const String commit = 'abc123'; - - processManager.addCommands([ - FakeCommand(command: [ - 'git', - 'clone', - '--origin', - 'upstream', - '--', - FrameworkRepository.defaultUpstream, - fileSystem.path - .join(rootDir, 'flutter_conductor_checkouts', 'framework'), - ]), - const FakeCommand(command: [ - 'git', - 'checkout', - FrameworkRepository.defaultBranch, - ]), - const FakeCommand(command: [ - 'git', - 'rev-parse', - 'HEAD', - ], stdout: commit), - const FakeCommand(command: [ - 'git', - 'status', - '--porcelain', - ]), - const FakeCommand(command: [ - 'git', - 'cherry-pick', - '--no-commit', - commit, - ], exitCode: 1), - const FakeCommand(command: [ - 'git', - 'diff', - ]), - const FakeCommand(command: [ - 'git', - 'reset', - 'HEAD', - '--hard', - ]), - ]); - final Checkouts checkouts = Checkouts( - fileSystem: fileSystem, - parentDirectory: fileSystem.directory(rootDir), - platform: platform, - processManager: processManager, - stdio: stdio, - ); - final Repository repository = FrameworkRepository(checkouts); - expect(await repository.canCherryPick(commit), false); - }); - - test('cherryPick() applies the commit', () async { - const String commit = 'abc123'; - - processManager.addCommands([ - FakeCommand(command: [ - 'git', - 'clone', - '--origin', - 'upstream', - '--', - FrameworkRepository.defaultUpstream, - fileSystem.path - .join(rootDir, 'flutter_conductor_checkouts', 'framework'), - ]), - const FakeCommand(command: [ - 'git', - 'checkout', - FrameworkRepository.defaultBranch, - ]), - const FakeCommand(command: [ - 'git', - 'rev-parse', - 'HEAD', - ], stdout: commit), - const FakeCommand(command: [ - 'git', - 'status', - '--porcelain', - ]), - const FakeCommand(command: [ - 'git', - 'cherry-pick', - commit, - ]), - ]); - final Checkouts checkouts = Checkouts( - fileSystem: fileSystem, - parentDirectory: fileSystem.directory(rootDir), - platform: platform, - processManager: processManager, - stdio: stdio, - ); - final Repository repository = FrameworkRepository(checkouts); - await repository.cherryPick(commit); - expect(processManager.hasRemainingExpectations, false); - }); - test('updateDartRevision() updates the DEPS file', () async { const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef'; const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e'; diff --git a/dev/conductor/core/test/start_test.dart b/dev/conductor/core/test/start_test.dart index d1d9d3cd4e..5c482aece3 100644 --- a/dev/conductor/core/test/start_test.dart +++ b/dev/conductor/core/test/start_test.dart @@ -1143,10 +1143,8 @@ void main() { candidateBranch: candidateBranch, checkouts: checkouts, dartRevision: nextDartRevision, - engineCherrypickRevisions: [], engineMirror: engineMirror, engineUpstream: EngineRepository.defaultUpstream, - frameworkCherrypickRevisions: [], frameworkMirror: frameworkMirror, frameworkUpstream: FrameworkRepository.defaultUpstream, releaseChannel: releaseChannel,