Unverified Commit d193f050 authored by Casey Hillers's avatar Casey Hillers Committed by GitHub

[conductor] Remove cherrypick option (#121405)

[conductor] Remove cherrypick option
parent a5c60f41
...@@ -418,50 +418,6 @@ abstract class Repository { ...@@ -418,50 +418,6 @@ abstract class Repository {
return exitcode == 0; return exitcode == 0;
} }
/// Determines if a commit will cherry-pick to current HEAD without conflict.
Future<bool> 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(
<String>['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(
<String>['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<void> cherryPick(String commit) async {
assert(
await gitCheckoutClean(),
'cannot cherry-pick because git checkout ${(await checkoutDirectory).path} is not clean',
);
await git.run(
<String>['cherry-pick', commit],
'cherry-pick $commit',
workingDirectory: (await checkoutDirectory).path,
);
}
/// Resets repository HEAD to [ref]. /// Resets repository HEAD to [ref].
Future<void> reset(String ref) async { Future<void> reset(String ref) async {
await git.run( await git.run(
...@@ -825,12 +781,6 @@ class HostFrameworkRepository extends FrameworkRepository { ...@@ -825,12 +781,6 @@ class HostFrameworkRepository extends FrameworkRepository {
'checkout not implemented for the host repository'); 'checkout not implemented for the host repository');
} }
@override
Future<String> cherryPick(String commit) async {
throw ConductorException(
'cherryPick not implemented for the host repository');
}
@override @override
Future<String> reset(String ref) async { Future<String> reset(String ref) async {
throw ConductorException('reset not implemented for the host repository'); throw ConductorException('reset not implemented for the host repository');
......
...@@ -22,9 +22,7 @@ import 'version.dart'; ...@@ -22,9 +22,7 @@ import 'version.dart';
const String kCandidateOption = 'candidate-branch'; const String kCandidateOption = 'candidate-branch';
const String kDartRevisionOption = 'dart-revision'; const String kDartRevisionOption = 'dart-revision';
const String kEngineCherrypicksOption = 'engine-cherrypicks';
const String kEngineUpstreamOption = 'engine-upstream'; const String kEngineUpstreamOption = 'engine-upstream';
const String kFrameworkCherrypicksOption = 'framework-cherrypicks';
const String kFrameworkMirrorOption = 'framework-mirror'; const String kFrameworkMirrorOption = 'framework-mirror';
const String kFrameworkUpstreamOption = 'framework-upstream'; const String kFrameworkUpstreamOption = 'framework-upstream';
const String kEngineMirrorOption = 'engine-mirror'; const String kEngineMirrorOption = 'engine-mirror';
...@@ -80,16 +78,6 @@ class StartCommand extends Command<void> { ...@@ -80,16 +78,6 @@ class StartCommand extends Command<void> {
defaultsTo: defaultPath, defaultsTo: defaultPath,
help: 'Path to persistent state file. Defaults to $defaultPath', help: 'Path to persistent state file. Defaults to $defaultPath',
); );
argParser.addMultiOption(
kEngineCherrypicksOption,
help: 'Engine cherrypick hashes to be applied.',
defaultsTo: <String>[],
);
argParser.addMultiOption(
kFrameworkCherrypicksOption,
help: 'Framework cherrypick hashes to be applied.',
defaultsTo: <String>[],
);
argParser.addOption( argParser.addOption(
kDartRevisionOption, kDartRevisionOption,
help: 'New Dart revision to cherrypick.', help: 'New Dart revision to cherrypick.',
...@@ -161,16 +149,6 @@ class StartCommand extends Command<void> { ...@@ -161,16 +149,6 @@ class StartCommand extends Command<void> {
argumentResults, argumentResults,
platform.environment, platform.environment,
)!; )!;
final List<String> frameworkCherrypickRevisions = getValuesFromEnvOrArgs(
kFrameworkCherrypicksOption,
argumentResults,
platform.environment,
);
final List<String> engineCherrypickRevisions = getValuesFromEnvOrArgs(
kEngineCherrypicksOption,
argumentResults,
platform.environment,
);
final String? dartRevision = getValueFromEnvOrArgs( final String? dartRevision = getValueFromEnvOrArgs(
kDartRevisionOption, kDartRevisionOption,
argumentResults, argumentResults,
...@@ -201,11 +179,9 @@ class StartCommand extends Command<void> { ...@@ -201,11 +179,9 @@ class StartCommand extends Command<void> {
candidateBranch: candidateBranch, candidateBranch: candidateBranch,
checkouts: checkouts, checkouts: checkouts,
dartRevision: dartRevision, dartRevision: dartRevision,
engineCherrypickRevisions: engineCherrypickRevisions,
engineMirror: engineMirror, engineMirror: engineMirror,
engineUpstream: engineUpstream, engineUpstream: engineUpstream,
conductorVersion: conductorVersion, conductorVersion: conductorVersion,
frameworkCherrypickRevisions: frameworkCherrypickRevisions,
frameworkMirror: frameworkMirror, frameworkMirror: frameworkMirror,
frameworkUpstream: frameworkUpstream, frameworkUpstream: frameworkUpstream,
processManager: processManager, processManager: processManager,
...@@ -226,10 +202,8 @@ class StartContext extends Context { ...@@ -226,10 +202,8 @@ class StartContext extends Context {
StartContext({ StartContext({
required this.candidateBranch, required this.candidateBranch,
required this.dartRevision, required this.dartRevision,
required this.engineCherrypickRevisions,
required this.engineMirror, required this.engineMirror,
required this.engineUpstream, required this.engineUpstream,
required this.frameworkCherrypickRevisions,
required this.frameworkMirror, required this.frameworkMirror,
required this.frameworkUpstream, required this.frameworkUpstream,
required this.conductorVersion, required this.conductorVersion,
...@@ -268,10 +242,8 @@ class StartContext extends Context { ...@@ -268,10 +242,8 @@ class StartContext extends Context {
final String candidateBranch; final String candidateBranch;
final String? dartRevision; final String? dartRevision;
final List<String> engineCherrypickRevisions;
final String engineMirror; final String engineMirror;
final String engineUpstream; final String engineUpstream;
final List<String> frameworkCherrypickRevisions;
final String frameworkMirror; final String frameworkMirror;
final String frameworkUpstream; final String frameworkUpstream;
final String conductorVersion; final String conductorVersion;
...@@ -336,31 +308,7 @@ class StartContext extends Context { ...@@ -336,31 +308,7 @@ class StartContext extends Context {
await engine.updateDartRevision(dartRevision!); await engine.updateDartRevision(dartRevision!);
await engine.commit('Update Dart SDK to $dartRevision', addFirst: true); await engine.commit('Update Dart SDK to $dartRevision', addFirst: true);
} }
final List<pb.Cherrypick> 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'); final String engineHead = await engine.reverseParse('HEAD');
state.engine = pb.Repository( state.engine = pb.Repository(
candidateBranch: candidateBranch, candidateBranch: candidateBranch,
...@@ -368,38 +316,12 @@ class StartContext extends Context { ...@@ -368,38 +316,12 @@ class StartContext extends Context {
startingGitHead: engineHead, startingGitHead: engineHead,
currentGitHead: engineHead, currentGitHead: engineHead,
checkoutPath: (await engine.checkoutDirectory).path, checkoutPath: (await engine.checkoutDirectory).path,
cherrypicks: engineCherrypicks,
dartRevision: dartRevision, dartRevision: dartRevision,
upstream: pb.Remote(name: 'upstream', url: engine.upstreamRemote.url), upstream: pb.Remote(name: 'upstream', url: engine.upstreamRemote.url),
mirror: pb.Remote(name: 'mirror', url: engine.mirrorRemote!.url), mirror: pb.Remote(name: 'mirror', url: engine.mirrorRemote!.url),
); );
await framework.newBranch(workingBranchName); await framework.newBranch(workingBranchName);
final List<pb.Cherrypick> 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 // Get framework version
final Version lastVersion = Version.fromString(await framework.getFullTag( final Version lastVersion = Version.fromString(await framework.getFullTag(
...@@ -446,7 +368,6 @@ class StartContext extends Context { ...@@ -446,7 +368,6 @@ class StartContext extends Context {
startingGitHead: frameworkHead, startingGitHead: frameworkHead,
currentGitHead: frameworkHead, currentGitHead: frameworkHead,
checkoutPath: (await framework.checkoutDirectory).path, checkoutPath: (await framework.checkoutDirectory).path,
cherrypicks: frameworkCherrypicks,
upstream: pb.Remote(name: 'upstream', url: framework.upstreamRemote.url), upstream: pb.Remote(name: 'upstream', url: framework.upstreamRemote.url),
mirror: pb.Remote(name: 'mirror', url: framework.mirrorRemote!.url), mirror: pb.Remote(name: 'mirror', url: framework.mirrorRemote!.url),
); );
...@@ -530,74 +451,4 @@ class StartContext extends Context { ...@@ -530,74 +451,4 @@ class StartContext extends Context {
stdio.printStatus('The actual release will be version $nextVersion.'); stdio.printStatus('The actual release will be version $nextVersion.');
return nextVersion; return nextVersion;
} }
// To minimize merge conflicts, sort the commits by rev-list order.
Future<List<String>> _sortCherrypicks({
required Repository repository,
required List<String> 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<String> unknownCherrypicks = <String>[];
// Full 40-char hashes parsed by git.
final List<String> validatedCherrypicks = <String>[];
// Final, validated, sorted list of cherrypicks to be applied.
final List<String> sortedCherrypicks = <String>[];
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<String> upstreamRevlist = (await repository.revList(<String>[
'--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 <String>[
...validatedCherrypicks,
...unknownCherrypicks
]) {
stdio.printError('\t$cp');
}
throw ConductorException(
'${validatedCherrypicks.length + unknownCherrypicks.length} unknown cherrypicks provided!',
);
}
} }
...@@ -31,161 +31,6 @@ void main() { ...@@ -31,161 +31,6 @@ void main() {
stdio = TestStdio(); stdio = TestStdio();
}); });
test('canCherryPick returns true if git cherry-pick returns 0', () async {
processManager.addCommands(<FakeCommand>[
FakeCommand(command: <String>[
'git',
'clone',
'--origin',
'upstream',
'--',
FrameworkRepository.defaultUpstream,
fileSystem.path
.join(rootDir, 'flutter_conductor_checkouts', 'framework'),
]),
const FakeCommand(command: <String>[
'git',
'checkout',
FrameworkRepository.defaultBranch,
]),
const FakeCommand(command: <String>[
'git',
'rev-parse',
'HEAD',
], stdout: revision),
const FakeCommand(command: <String>[
'git',
'status',
'--porcelain',
]),
const FakeCommand(command: <String>[
'git',
'cherry-pick',
'--no-commit',
revision,
]),
const FakeCommand(command: <String>[
'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>[
FakeCommand(command: <String>[
'git',
'clone',
'--origin',
'upstream',
'--',
FrameworkRepository.defaultUpstream,
fileSystem.path
.join(rootDir, 'flutter_conductor_checkouts', 'framework'),
]),
const FakeCommand(command: <String>[
'git',
'checkout',
FrameworkRepository.defaultBranch,
]),
const FakeCommand(command: <String>[
'git',
'rev-parse',
'HEAD',
], stdout: commit),
const FakeCommand(command: <String>[
'git',
'status',
'--porcelain',
]),
const FakeCommand(command: <String>[
'git',
'cherry-pick',
'--no-commit',
commit,
], exitCode: 1),
const FakeCommand(command: <String>[
'git',
'diff',
]),
const FakeCommand(command: <String>[
'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>[
FakeCommand(command: <String>[
'git',
'clone',
'--origin',
'upstream',
'--',
FrameworkRepository.defaultUpstream,
fileSystem.path
.join(rootDir, 'flutter_conductor_checkouts', 'framework'),
]),
const FakeCommand(command: <String>[
'git',
'checkout',
FrameworkRepository.defaultBranch,
]),
const FakeCommand(command: <String>[
'git',
'rev-parse',
'HEAD',
], stdout: commit),
const FakeCommand(command: <String>[
'git',
'status',
'--porcelain',
]),
const FakeCommand(command: <String>[
'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 { test('updateDartRevision() updates the DEPS file', () async {
const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef'; const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef';
const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e'; const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e';
......
...@@ -1143,10 +1143,8 @@ void main() { ...@@ -1143,10 +1143,8 @@ void main() {
candidateBranch: candidateBranch, candidateBranch: candidateBranch,
checkouts: checkouts, checkouts: checkouts,
dartRevision: nextDartRevision, dartRevision: nextDartRevision,
engineCherrypickRevisions: <String>[],
engineMirror: engineMirror, engineMirror: engineMirror,
engineUpstream: EngineRepository.defaultUpstream, engineUpstream: EngineRepository.defaultUpstream,
frameworkCherrypickRevisions: <String>[],
frameworkMirror: frameworkMirror, frameworkMirror: frameworkMirror,
frameworkUpstream: FrameworkRepository.defaultUpstream, frameworkUpstream: FrameworkRepository.defaultUpstream,
releaseChannel: releaseChannel, releaseChannel: releaseChannel,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment