[flutter_conductor] catch and warn when pushing working branch to mirror (#94506)

dev/conductor/core/lib/src/git.dart

@@ -74,15 +74,55 @@ class Git {
     if ((result.stderr as String).isNotEmpty) {
       message.writeln('stderr from git:\n${result.stderr}\n');
     }
-    throw GitException(message.toString());
+    throw GitException(message.toString(), args);
   }
 }
 
+enum GitExceptionType {
+  /// Git push failed because the remote branch contained commits the local did
+  /// not.
+  ///
+  /// Either the local branch was wrong, and needs a rebase before pushing
+  /// again, or the remote branch needs to be overwritten with a force push.
+  ///
+  /// Example output:
+  ///
+  /// ```
+  /// To github.com:user/engine.git
+  ///
+  ///  ! [rejected]            HEAD -> cherrypicks-flutter-2.8-candidate.3 (non-fast-forward)
+  /// error: failed to push some refs to 'github.com:user/engine.git'
+  /// hint: Updates were rejected because the tip of your current branch is behind
+  /// hint: its remote counterpart. Integrate the remote changes (e.g.
+  /// hint: 'git pull ...') before pushing again.
+  /// hint: See the 'Note about fast-forwards' in 'git push --help' for details.
+  /// ```
+  PushRejected,
+}
+
+/// An exception created because a git subprocess failed.
+///
+/// Known git failures will be assigned a [GitExceptionType] in the [type]
+/// field. If this field is null it means and unknown git failure.
 class GitException implements Exception {
-  GitException(this.message);
+  GitException(this.message, this.args) {
+    if (_pushRejectedPattern.hasMatch(message)) {
+      type = GitExceptionType.PushRejected;
+    } else {
+      // because type is late final, it must be explicitly set before it is
+      // accessed.
+      type = null;
+    }
+  }
+
+  static final RegExp _pushRejectedPattern = RegExp(
+    r'Updates were rejected because the tip of your current branch is behind',
+  );
 
   final String message;
+  final List<String> args;
+  late final GitExceptionType? type;
 
   @override
-  String toString() => 'Exception: $message';
+  String toString() => 'Exception on command "${args.join(' ')}": $message';
 }

dev/conductor/core/lib/src/next.dart

@@ -4,8 +4,10 @@
 
 import 'package:args/command_runner.dart';
 import 'package:file/file.dart' show File;
+import 'package:meta/meta.dart' show visibleForTesting;
 
 import 'context.dart';
+import 'git.dart';
 import 'globals.dart';
 import 'proto/conductor_state.pb.dart' as pb;
 import 'proto/conductor_state.pbenum.dart';
@@ -152,13 +154,7 @@ class NextContext extends Context {
           }
         }
 
-        await engine.pushRef(
+        await pushWorkingBranch(engine, state.engine);
-            fromRef: 'HEAD',
-            // Explicitly create new branch
-            toRef: 'refs/heads/${state.engine.workingBranch}',
-            remote: state.engine.mirror.name,
-        );
-
         break;
       case pb.ReleasePhase.CODESIGN_ENGINE_BINARIES:
         stdio.printStatus(<String>[
@@ -285,12 +281,7 @@ class NextContext extends Context {
           }
         }
 
-        await framework.pushRef(
+        await pushWorkingBranch(framework, state.framework);
-          fromRef: 'HEAD',
-          // Explicitly create new branch
-          toRef: 'refs/heads/${state.framework.workingBranch}',
-          remote: state.framework.mirror.name,
-        );
         break;
       case pb.ReleasePhase.PUBLISH_VERSION:
         stdio.printStatus('Please ensure that you have merged your framework PR and that');
@@ -381,4 +372,37 @@ class NextContext extends Context {
 
     updateState(state, stdio.logs);
   }
+
+  /// Push the working branch to the user's mirror.
+  ///
+  /// [repository] represents the actual Git repository on disk, and is used to
+  /// call `git push`, while [pbRepository] represents the user-specified
+  /// configuration for the repository, and is used to read the name of the
+  /// working branch and the mirror's remote name.
+  ///
+  /// May throw either a [ConductorException] if the user already has a branch
+  /// of the same name on their mirror, or a [GitException] for any other
+  /// failures from the underlying git process call.
+  @visibleForTesting
+  Future<void> pushWorkingBranch(Repository repository, pb.Repository pbRepository) async {
+    try {
+      await repository.pushRef(
+          fromRef: 'HEAD',
+          // Explicitly create new branch
+          toRef: 'refs/heads/${pbRepository.workingBranch}',
+          remote: pbRepository.mirror.name,
+          force: force,
+      );
+    } on GitException catch (exception) {
+      if (exception.type == GitExceptionType.PushRejected && force == false) {
+        throw ConductorException(
+          'Push failed because the working branch named '
+          '${pbRepository.workingBranch} already exists on your mirror. '
+          'Re-run this command with --force to overwrite the remote branch.\n'
+          '${exception.message}',
+        );
+      }
+      rethrow;
+    }
+  }
 }

dev/conductor/core/lib/src/repository.dart

@@ -556,7 +556,7 @@ class FrameworkRepository extends Repository {
   }
 
   @override
-  Future<Repository> cloneRepository(String? cloneName) async {
+  Future<FrameworkRepository> cloneRepository(String? cloneName) async {
     assert(localUpstream);
     cloneName ??= 'clone-of-$name';
     return FrameworkRepository(

dev/conductor/core/test/next_test.dart

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 import 'package:args/command_runner.dart';
+import 'package:conductor_core/src/git.dart';
+import 'package:conductor_core/src/globals.dart';
 import 'package:conductor_core/src/next.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;
@@ -16,23 +18,24 @@ import 'package:platform/platform.dart';
 import './common.dart';
 
 void main() {
+  const String flutterRoot = '/flutter';
+  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';
+  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';
+  const String stateFile = '/state-file.json';
+  final String localPathSeparator = const LocalPlatform().pathSeparator;
+  final String localOperatingSystem = const LocalPlatform().pathSeparator;
+
   group('next command', () {
-    const String flutterRoot = '/flutter';
-    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;
     late TestStdio stdio;
-    const String stateFile = '/state-file.json';
 
     setUp(() {
       stdio = TestStdio();
@@ -1102,6 +1105,68 @@ void main() {
       );
     });
   });
+
+  group('.pushWorkingBranch()', () {
+    late MemoryFileSystem fileSystem;
+    late TestStdio stdio;
+    late Platform platform;
+
+    setUp(() {
+      stdio = TestStdio();
+      fileSystem = MemoryFileSystem.test();
+      platform = FakePlatform();
+    });
+
+    test('catches GitException if the push was rejected and instead throws a helpful ConductorException', () async {
+      const String gitPushErrorMessage = '''
+ To github.com:user/engine.git
+
+  ! [rejected]            HEAD -> cherrypicks-flutter-2.8-candidate.3 (non-fast-forward)
+ error: failed to push some refs to 'github.com:user/engine.git'
+ hint: Updates were rejected because the tip of your current branch is behind
+ hint: its remote counterpart. Integrate the remote changes (e.g.
+ hint: 'git pull ...') before pushing again.
+ hint: See the 'Note about fast-forwards' in 'git push --help' for details.
+''';
+      final Checkouts checkouts = Checkouts(
+        fileSystem: fileSystem,
+        parentDirectory: fileSystem.directory(checkoutsParentDirectory)..createSync(recursive: true),
+        platform: platform,
+        processManager: FakeProcessManager.empty(),
+        stdio: stdio,
+      );
+      final Repository testRepository = _TestRepository.fromCheckouts(checkouts);
+      final pb.Repository testPbRepository = pb.Repository();
+      (checkouts.processManager as FakeProcessManager).addCommands(<FakeCommand>[
+        FakeCommand(
+          command: <String>['git', 'clone', '--origin', 'upstream', '--', testRepository.upstreamRemote.url, '/flutter/dev/conductor/flutter_conductor_checkouts/test-repo/test-repo'],
+        ),
+        const FakeCommand(
+          command: <String>['git', 'rev-parse', 'HEAD'],
+          stdout: revision1,
+        ),
+        FakeCommand(
+          command: const <String>['git', 'push', '', 'HEAD:refs/heads/'],
+          exception: GitException(gitPushErrorMessage, <String>['git', 'push', '--force', '', 'HEAD:refs/heads/']),
+        )
+      ]);
+      final NextContext nextContext = NextContext(
+        autoAccept: false,
+        checkouts: checkouts,
+        force: false,
+        stateFile: fileSystem.file(stateFile),
+      );
+
+      expect(
+        () => nextContext.pushWorkingBranch(testRepository, testPbRepository),
+        throwsA(isA<ConductorException>().having(
+          (ConductorException exception) => exception.message,
+          'has correct message',
+          contains('Re-run this command with --force to overwrite the remote branch'),
+        )),
+      );
+    });
+  });
 }
 
 /// A [Stdio] that will throw an exception if any of its methods are called.
@@ -1135,6 +1200,24 @@ class _UnimplementedStdio implements Stdio {
   String readLineSync() => _throw();
 }
 
+class _TestRepository extends Repository {
+  _TestRepository.fromCheckouts(Checkouts checkouts, [String name = 'test-repo']) : super(
+    fileSystem: checkouts.fileSystem,
+    parentDirectory: checkouts.directory.childDirectory(name),
+    platform: checkouts.platform,
+    processManager: checkouts.processManager,
+    name: name,
+    requiredLocalBranches: <String>[],
+    stdio: checkouts.stdio,
+    upstreamRemote: const Remote(name: RemoteName.upstream, url: 'git@github.com:upstream/repo.git'),
+  );
+
+  @override
+  Future<_TestRepository> cloneRepository(String? cloneName) async {
+    throw Exception('Unimplemented!');
+  }
+}
+
 class _TestNextContext extends NextContext {
   const _TestNextContext({
     bool autoAccept = false,