[flutter_conductor] Re-land auto-apply dart revision (#82985)

2021-05-20 16:58:11 -07:00
parent 0902576aba
commit 660ec207e6
9 changed files with 282 additions and 23 deletions
--- a/dev/tools/lib/globals.dart
+++ b/dev/tools/lib/globals.dart
@@ -95,20 +95,25 @@ bool assertsEnabled() {
 ///
 /// The environment is favored over CLI args since the latter can have a default
 /// value, which the environment should be able to override.
-String getValueFromEnvOrArgs(
+String? getValueFromEnvOrArgs(
  String name,
  ArgResults argResults,
-  Map<String, String> env,
+  Map<String, String> env, {
+    bool allowNull = false,
+  }
 ) {
  final String envName = fromArgToEnvName(name);
  if (env[envName] != null ) {
-    return env[envName]!;
+    return env[envName];
  }
  final String? argValue = argResults[name] as String?;
  if (argValue != null) {
    return argValue;
  }

+  if (allowNull) {
+    return null;
+  }
  throw ConductorException(
    'Expected either the CLI arg --$name or the environment variable $envName '
    'to be provided!');
--- a/dev/tools/lib/proto/conductor_state.pb.dart
+++ b/dev/tools/lib/proto/conductor_state.pb.dart
@@ -202,6 +202,8 @@ class Repository extends $pb.GeneratedMessage {
    ..pc<Cherrypick>(
        7, const $core.bool.fromEnvironment('protobuf.omit_field_names') ? '' : 'cherrypicks', $pb.PbFieldType.PM,
        subBuilder: Cherrypick.create)
+    ..aOS(8, const $core.bool.fromEnvironment('protobuf.omit_field_names') ? '' : 'dartRevision',
+        protoName: 'dartRevision')
    ..hasRequiredFields = false;

  Repository._() : super();
@@ -213,6 +215,7 @@ class Repository extends $pb.GeneratedMessage {
    Remote upstream,
    Remote mirror,
    $core.Iterable<Cherrypick> cherrypicks,
+    $core.String dartRevision,
  }) {
    final _result = create();
    if (candidateBranch != null) {
@@ -236,6 +239,9 @@ class Repository extends $pb.GeneratedMessage {
    if (cherrypicks != null) {
      _result.cherrypicks.addAll(cherrypicks);
    }
+    if (dartRevision != null) {
+      _result.dartRevision = dartRevision;
+    }
    return _result;
  }
  factory Repository.fromBuffer($core.List<$core.int> i, [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) =>
@@ -338,6 +344,18 @@ class Repository extends $pb.GeneratedMessage {

  @$pb.TagNumber(7)
  $core.List<Cherrypick> get cherrypicks => $_getList(6);
+
+  @$pb.TagNumber(8)
+  $core.String get dartRevision => $_getSZ(7);
+  @$pb.TagNumber(8)
+  set dartRevision($core.String v) {
+    $_setString(7, v);
+  }
+
+  @$pb.TagNumber(8)
+  $core.bool hasDartRevision() => $_has(7);
+  @$pb.TagNumber(8)
+  void clearDartRevision() => clearField(8);
 }

 class ConductorState extends $pb.GeneratedMessage {
--- a/dev/tools/lib/proto/conductor_state.pbjson.dart
+++ b/dev/tools/lib/proto/conductor_state.pbjson.dart
@@ -80,12 +80,13 @@ const Repository$json = const {
    const {'1': 'upstream', '3': 5, '4': 1, '5': 11, '6': '.conductor_state.Remote', '10': 'upstream'},
    const {'1': 'mirror', '3': 6, '4': 1, '5': 11, '6': '.conductor_state.Remote', '10': 'mirror'},
    const {'1': 'cherrypicks', '3': 7, '4': 3, '5': 11, '6': '.conductor_state.Cherrypick', '10': 'cherrypicks'},
+    const {'1': 'dartRevision', '3': 8, '4': 1, '5': 9, '10': 'dartRevision'},
  ],
 };

 /// Descriptor for `Repository`. Decode as a `google.protobuf.DescriptorProto`.
 final $typed_data.Uint8List repositoryDescriptor = $convert.base64Decode(
-    'CgpSZXBvc2l0b3J5EigKD2NhbmRpZGF0ZUJyYW5jaBgBIAEoCVIPY2FuZGlkYXRlQnJhbmNoEigKD3N0YXJ0aW5nR2l0SGVhZBgCIAEoCVIPc3RhcnRpbmdHaXRIZWFkEiYKDmN1cnJlbnRHaXRIZWFkGAMgASgJUg5jdXJyZW50R2l0SGVhZBIiCgxjaGVja291dFBhdGgYBCABKAlSDGNoZWNrb3V0UGF0aBIzCgh1cHN0cmVhbRgFIAEoCzIXLmNvbmR1Y3Rvcl9zdGF0ZS5SZW1vdGVSCHVwc3RyZWFtEi8KBm1pcnJvchgGIAEoCzIXLmNvbmR1Y3Rvcl9zdGF0ZS5SZW1vdGVSBm1pcnJvchI9CgtjaGVycnlwaWNrcxgHIAMoCzIbLmNvbmR1Y3Rvcl9zdGF0ZS5DaGVycnlwaWNrUgtjaGVycnlwaWNrcw==');
+    'CgpSZXBvc2l0b3J5EigKD2NhbmRpZGF0ZUJyYW5jaBgBIAEoCVIPY2FuZGlkYXRlQnJhbmNoEigKD3N0YXJ0aW5nR2l0SGVhZBgCIAEoCVIPc3RhcnRpbmdHaXRIZWFkEiYKDmN1cnJlbnRHaXRIZWFkGAMgASgJUg5jdXJyZW50R2l0SGVhZBIiCgxjaGVja291dFBhdGgYBCABKAlSDGNoZWNrb3V0UGF0aBIzCgh1cHN0cmVhbRgFIAEoCzIXLmNvbmR1Y3Rvcl9zdGF0ZS5SZW1vdGVSCHVwc3RyZWFtEi8KBm1pcnJvchgGIAEoCzIXLmNvbmR1Y3Rvcl9zdGF0ZS5SZW1vdGVSBm1pcnJvchI9CgtjaGVycnlwaWNrcxgHIAMoCzIbLmNvbmR1Y3Rvcl9zdGF0ZS5DaGVycnlwaWNrUgtjaGVycnlwaWNrcxIiCgxkYXJ0UmV2aXNpb24YCCABKAlSDGRhcnRSZXZpc2lvbg==');
@$core.Deprecated('Use conductorStateDescriptor instead')
 const ConductorState$json = const {
  '1': 'ConductorState',
--- a/dev/tools/lib/proto/conductor_state.proto
+++ b/dev/tools/lib/proto/conductor_state.proto
@@ -79,6 +79,9 @@ message Repository {

  // Desired cherrypicks.
  repeated Cherrypick cherrypicks = 7;
+
+  // Only for engine repositories.
+  string dartRevision = 8;
 }

 message ConductorState {
--- a/dev/tools/lib/repository.dart
+++ b/dev/tools/lib/repository.dart
@@ -365,6 +365,26 @@ abstract class Repository {
    );
  }

+  String commit(
+    String message, {
+    bool addFirst = false,
+  }) {
+    assert(!message.contains("'"));
+    if (addFirst) {
+      git.run(
+        <String>['add', '--all'],
+        'add all changes to the index',
+        workingDirectory: checkoutDirectory.path,
+      );
+    }
+    git.run(
+      <String>['commit', "--message='$message'"],
+      'commit changes',
+      workingDirectory: checkoutDirectory.path,
+    );
+    return reverseParse('HEAD');
+  }
+
  /// Create an empty commit and return the revision.
  @visibleForTesting
  String authorEmptyCommit([String message = 'An empty commit']) {
@@ -617,6 +637,31 @@ class EngineRepository extends Repository {
  static const String defaultUpstream = 'https://github.com/flutter/engine.git';
  static const String defaultBranch = 'master';

+  /// Update the `dart_revision` entry in the DEPS file.
+  void updateDartRevision(
+    String newRevision, {
+    @visibleForTesting File? depsFile,
+  }) {
+    assert(newRevision.length == 40);
+    depsFile ??= checkoutDirectory.childFile('DEPS');
+    final String fileContent = depsFile.readAsStringSync();
+    final RegExp dartPattern = RegExp("[ ]+'dart_revision': '([a-z0-9]{40})',");
+    final Iterable<RegExpMatch> allMatches = dartPattern.allMatches(fileContent);
+    if (allMatches.length != 1) {
+      throw ConductorException(
+        'Unexpected content in the DEPS file at ${depsFile.path}\n'
+        'Expected to find pattern ${dartPattern.pattern} 1 times, but got '
+        '${allMatches.length}.'
+      );
+    }
+    final String updatedFileContent = fileContent.replaceFirst(
+      dartPattern,
+      "  'dart_revision': '$newRevision',",
+    );
+
+    depsFile.writeAsStringSync(updatedFileContent);
+  }
+
  @override
  Repository cloneRepository(String? cloneName) {
    assert(localUpstream);
--- a/dev/tools/lib/start.dart
+++ b/dev/tools/lib/start.dart
@@ -22,14 +22,15 @@ import './state.dart';
 import './stdio.dart';

 const String kCandidateOption = 'candidate-branch';
-const String kReleaseOption = 'release-channel';
-const String kStateOption = 'state-file';
-const String kFrameworkMirrorOption = 'framework-mirror';
-const String kEngineMirrorOption = 'engine-mirror';
-const String kFrameworkUpstreamOption = 'framework-upstream';
+const String kDartRevisionOption = 'dart-revision';
+const String kEngineCherrypicksOption = 'engine-cherrypicks';
 const String kEngineUpstreamOption = 'engine-upstream';
 const String kFrameworkCherrypicksOption = 'framework-cherrypicks';
-const String kEngineCherrypicksOption = 'engine-cherrypicks';
+const String kFrameworkMirrorOption = 'framework-mirror';
+const String kFrameworkUpstreamOption = 'framework-upstream';
+const String kEngineMirrorOption = 'engine-mirror';
+const String kReleaseOption = 'release-channel';
+const String kStateOption = 'state-file';

 /// Command to print the status of the current Flutter release.
 class StartCommand extends Command<void> {
@@ -86,6 +87,10 @@ class StartCommand extends Command<void> {
      help: 'Framework cherrypick hashes to be applied.',
      defaultsTo: <String>[],
    );
+    argParser.addOption(
+      kDartRevisionOption,
+      help: 'New Dart revision to cherrypick.',
+    );
    final Git git = Git(processManager);
    conductorVersion = git.getOutput(
      <String>['rev-parse', 'HEAD'],
@@ -172,10 +177,17 @@ class StartCommand extends Command<void> {
      argResults,
      platform.environment,
    );
+    final String dartRevision = getValueFromEnvOrArgs(
+      kDartRevisionOption,
+      argResults,
+      platform.environment,
+      allowNull: true,
+    );
    if (!releaseCandidateBranchRegex.hasMatch(candidateBranch)) {
      throw ConductorException(
-          'Invalid release candidate branch "$candidateBranch". '
-          'Text should match the regex pattern /${releaseCandidateBranchRegex.pattern}/.');
+        'Invalid release candidate branch "$candidateBranch". Text should '
+        'match the regex pattern /${releaseCandidateBranchRegex.pattern}/.',
+      );
    }

    final Int64 unixDate = Int64(DateTime.now().millisecondsSinceEpoch);
@@ -197,9 +209,15 @@ class StartCommand extends Command<void> {
        url: engineMirror,
      ),
    );
+
    // Create a new branch so that we don't accidentally push to upstream
    // candidateBranch.
    engine.newBranch('cherrypicks-$candidateBranch');
+
+    if (dartRevision != null && dartRevision.isNotEmpty) {
+      engine.updateDartRevision(dartRevision);
+      engine.commit('Update Dart SDK to $dartRevision', addFirst: true);
+    }
    final List<pb.Cherrypick> engineCherrypicks = _sortCherrypicks(
      repository: engine,
      cherrypicks: engineCherrypickRevisions,
@@ -230,6 +248,7 @@ class StartCommand extends Command<void> {
      currentGitHead: engineHead,
      checkoutPath: engine.checkoutDirectory.path,
      cherrypicks: engineCherrypicks,
+      dartRevision: dartRevision,
    );
    final FrameworkRepository framework = FrameworkRepository(
      checkouts,
--- a/dev/tools/lib/state.dart
+++ b/dev/tools/lib/state.dart
@@ -57,6 +57,9 @@ String presentState(pb.ConductorState state) {
  } else {
    buffer.writeln('0 Engine cherrypicks.');
  }
+  if (state.engine.dartRevision != null && state.engine.dartRevision.isNotEmpty) {
+    buffer.writeln('New Dart SDK revision: ${state.engine.dartRevision}');
+  }
  buffer.writeln('Framework Repo');
  buffer.writeln('\tCandidate branch: ${state.framework.candidateBranch}');
  buffer.writeln('\tStarting git HEAD: ${state.framework.startingGitHead}');
--- a/dev/tools/test/repository_test.dart
+++ b/dev/tools/test/repository_test.dart
@@ -3,6 +3,7 @@
 // found in the LICENSE file.

 import 'package:dev_tools/repository.dart';
+import 'package:file/file.dart';
 import 'package:file/memory.dart';
 import 'package:platform/platform.dart';

@@ -12,9 +13,20 @@ import './common.dart';

 void main() {
  group('repository', () {
+    late FakePlatform platform;
+    const String rootDir = '/';
+
+    setUp(() {
+      final String pathSeparator = const LocalPlatform().pathSeparator;
+      platform = FakePlatform(
+        environment: <String, String>{
+          'HOME': <String>['path', 'to', 'home'].join(pathSeparator),
+        },
+        pathSeparator: pathSeparator,
+      );
+    });
+
    test('canCherryPick returns true if git cherry-pick returns 0', () {
-      const LocalPlatform platform = LocalPlatform();
-      const String rootDir = '/';
      const String commit = 'abc123';

      final TestStdio stdio = TestStdio();
@@ -66,8 +78,6 @@ void main() {
    });

    test('canCherryPick returns false if git cherry-pick returns non-zero', () {
-      const LocalPlatform platform = LocalPlatform();
-      const String rootDir = '/';
      const String commit = 'abc123';

      final TestStdio stdio = TestStdio();
@@ -123,8 +133,6 @@ void main() {
    });

    test('cherryPick() applies the commit', () {
-      const LocalPlatform platform = LocalPlatform();
-      const String rootDir = '/';
      const String commit = 'abc123';

      final TestStdio stdio = TestStdio();
@@ -168,5 +176,122 @@ void main() {
      repository.cherryPick(commit);
      expect(processManager.hasRemainingExpectations, false);
    });
+
+    test('updateDartRevision() updates the DEPS file', () {
+      const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef';
+      const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e';
+      final TestStdio stdio = TestStdio();
+      final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+      final FakeProcessManager processManager = FakeProcessManager.empty();
+
+      final Checkouts checkouts = Checkouts(
+        fileSystem: fileSystem,
+        parentDirectory: fileSystem.directory(rootDir),
+        platform: platform,
+        processManager: processManager,
+        stdio: stdio,
+      );
+
+      final EngineRepository repo = EngineRepository(checkouts);
+      final File depsFile = fileSystem.file('/DEPS');
+      depsFile.writeAsStringSync(generateMockDeps(previousDartRevision));
+      repo.updateDartRevision(nextDartRevision, depsFile: depsFile);
+      final String updatedDepsFileContent = depsFile.readAsStringSync();
+      expect(updatedDepsFileContent, generateMockDeps(nextDartRevision));
+    });
+
+    test('updateDartRevision() throws exception on malformed DEPS file', () {
+      const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e';
+      final TestStdio stdio = TestStdio();
+      final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+      final FakeProcessManager processManager = FakeProcessManager.empty();
+
+      final Checkouts checkouts = Checkouts(
+        fileSystem: fileSystem,
+        parentDirectory: fileSystem.directory(rootDir),
+        platform: platform,
+        processManager: processManager,
+        stdio: stdio,
+      );
+
+      final EngineRepository repo = EngineRepository(checkouts);
+      final File depsFile = fileSystem.file('/DEPS');
+      depsFile.writeAsStringSync('''
+vars = {
+}''');
+      expect(
+        () => repo.updateDartRevision(nextDartRevision, depsFile: depsFile),
+        throwsExceptionWith('Unexpected content in the DEPS file at'),
+      );
+    });
+
+    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>[
+        FakeCommand(command: <String>[
+          'git',
+          'clone',
+          '--origin',
+          'upstream',
+          '--',
+          EngineRepository.defaultUpstream,
+          fileSystem.path
+              .join(rootDir, 'flutter_conductor_checkouts', 'engine'),
+        ]),
+        const FakeCommand(command: <String>[
+          'git',
+          'checkout',
+          'upstream/master',
+        ]),
+        const FakeCommand(command: <String>[
+          'git',
+          'rev-parse',
+          'HEAD',
+        ], stdout: commit1),
+        const FakeCommand(command: <String>[
+          'git',
+          'commit',
+          "--message='$message'",
+        ]),
+        const FakeCommand(command: <String>[
+          '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);
+      repo.commit(message);
+    });
  });
 }
+
+String generateMockDeps(String dartRevision) {
+  return '''
+vars = {
+  'chromium_git': 'https://chromium.googlesource.com',
+  'swiftshader_git': 'https://swiftshader.googlesource.com',
+  'dart_git': 'https://dart.googlesource.com',
+  'flutter_git': 'https://flutter.googlesource.com',
+  'fuchsia_git': 'https://fuchsia.googlesource.com',
+  'github_git': 'https://github.com',
+  'skia_git': 'https://skia.googlesource.com',
+  'ocmock_git': 'https://github.com/erikdoe/ocmock.git',
+  'skia_revision': '4e9d5e2bdf04c58bc0bff57be7171e469e5d7175',
+
+  'dart_revision': '$dartRevision',
+  'dart_boringssl_gen_rev': '7322fc15cc065d8d2957fccce6b62a509dc4d641',
+}''';
+}
--- a/dev/tools/test/start_test.dart
+++ b/dev/tools/test/start_test.dart
@@ -120,6 +120,14 @@ void main() {
    test('creates state file if provided correct inputs', () async {
      const String revision2 = 'def789';
      const String revision3 = '123abc';
+      const String previousDartRevision = '171876a4e6cf56ee6da1f97d203926bd7afda7ef';
+      const String nextDartRevision = 'f6c91128be6b77aef8351e1e3a9d07c85bc2e46e';
+
+      final Directory engine = fileSystem.directory(checkoutsParentDirectory)
+          .childDirectory('flutter_conductor_checkouts')
+          .childDirectory('engine');
+
+      final File depsFile = engine.childFile('DEPS');

      final List<FakeCommand> engineCommands = <FakeCommand>[
        FakeCommand(
@@ -130,12 +138,13 @@ void main() {
            'upstream',
            '--',
            EngineRepository.defaultUpstream,
-            fileSystem.path.join(
-              checkoutsParentDirectory,
-              'flutter_conductor_checkouts',
-              'engine',
-            ),
+            engine.path,
          ],
+          onRun: () {
+            // Create the DEPS file which the tool will update
+            engine.createSync(recursive: true);
+            depsFile.writeAsStringSync(generateMockDeps(previousDartRevision));
+          }
        ),
        const FakeCommand(
          command: <String>['git', 'remote', 'add', 'mirror', engineMirror],
@@ -158,6 +167,16 @@ void main() {
            'cherrypicks-$candidateBranch',
          ],
        ),
+        const FakeCommand(
+          command: <String>['git', 'add', '--all'],
+        ),
+        const FakeCommand(
+          command: <String>['git', 'commit', "--message='Update Dart SDK to $nextDartRevision'"],
+        ),
+        const FakeCommand(
+          command: <String>['git', 'rev-parse', 'HEAD'],
+          stdout: revision2,
+        ),
        const FakeCommand(
          command: <String>['git', 'rev-parse', 'HEAD'],
          stdout: revision2,
@@ -233,6 +252,8 @@ void main() {
        releaseChannel,
        '--$kStateOption',
        stateFilePath,
+        '--$kDartRevisionOption',
+        nextDartRevision,
      ]);

      final File stateFile = fileSystem.file(stateFilePath);
@@ -246,6 +267,7 @@ void main() {
      expect(state.releaseChannel, releaseChannel);
      expect(state.engine.candidateBranch, candidateBranch);
      expect(state.engine.startingGitHead, revision2);
+      expect(state.engine.dartRevision, nextDartRevision);
      expect(state.framework.candidateBranch, candidateBranch);
      expect(state.framework.startingGitHead, revision3);
      expect(state.lastPhase, ReleasePhase.INITIALIZE);
@@ -255,3 +277,21 @@ void main() {
    'windows': const Skip('Flutter Conductor only supported on macos/linux'),
  });
 }
+
+String generateMockDeps(String dartRevision) {
+  return '''
+vars = {
+  'chromium_git': 'https://chromium.googlesource.com',
+  'swiftshader_git': 'https://swiftshader.googlesource.com',
+  'dart_git': 'https://dart.googlesource.com',
+  'flutter_git': 'https://flutter.googlesource.com',
+  'fuchsia_git': 'https://fuchsia.googlesource.com',
+  'github_git': 'https://github.com',
+  'skia_git': 'https://skia.googlesource.com',
+  'ocmock_git': 'https://github.com/erikdoe/ocmock.git',
+  'skia_revision': '4e9d5e2bdf04c58bc0bff57be7171e469e5d7175',
+
+  'dart_revision': '$dartRevision',
+  'dart_boringssl_gen_rev': '7322fc15cc065d8d2957fccce6b62a509dc4d641',
+}''';
+}