Add pre-push git hook (#26699)

DEPS

@@ -712,5 +712,13 @@ hooks = [
       'python3',
       'src/build/win/generate_winrt_headers.py',
     ]
+  },
+  {
+    'name': 'Setup githooks',
+    'pattern': '.',
+    'action': [
+      'python3',
+      'src/flutter/tools/githooks/setup.py',
+    ]
   }
 ]

ci/analyze.sh

@@ -32,6 +32,7 @@ SRC_DIR="$(cd "$SCRIPT_DIR/../.."; pwd -P)"
 FLUTTER_DIR="$SRC_DIR/flutter"
 DART_BIN="$SRC_DIR/third_party/dart/tools/sdks/dart-sdk/bin"
 PUB="$DART_BIN/pub"
+DART="$DART_BIN/dart"
 DART_ANALYZER="$DART_BIN/dartanalyzer"
 
 echo "Using analyzer from $DART_ANALYZER"
@@ -125,7 +126,13 @@ analyze \
   --options "$FLUTTER_DIR/analysis_options.yaml" \
   "$FLUTTER_DIR/testing/symbols"
 
+echo "Analyzing githooks..."
+analyze \
+  --packages="$FLUTTER_DIR/tools/githooks/.dart_tool/package_config.json" \
+  --options "$FLUTTER_DIR/analysis_options.yaml" \
+  "$FLUTTER_DIR/tools/githooks"
+
 # Check that dart libraries conform.
 echo "Checking web_ui api conformance..."
-(cd "$FLUTTER_DIR/web_sdk"; pub get)
-(cd "$FLUTTER_DIR"; dart "web_sdk/test/api_conform_test.dart")
+(cd "$FLUTTER_DIR/web_sdk"; "$PUB" get)
+(cd "$FLUTTER_DIR"; "$DART" "web_sdk/test/api_conform_test.dart")

testing/run_tests.py

@@ -485,6 +485,21 @@ def RunBenchmarkTests(build_dir):
       cwd=test_dir)
 
 
+def RunGithooksTests(build_dir):
+  test_dir = os.path.join(buildroot_dir, 'flutter', 'tools', 'githooks')
+  dart_tests = glob.glob('%s/test/*_test.dart' % test_dir)
+  for dart_test_file in dart_tests:
+    opts = [
+      '--disable-dart-dev',
+      dart_test_file]
+    RunEngineExecutable(
+      build_dir,
+      os.path.join('dart-sdk', 'bin', 'dart'),
+      None,
+      flags=opts,
+      cwd=test_dir)
+
+
 def main():
   parser = argparse.ArgumentParser()
 
@@ -526,6 +541,7 @@ def main():
     dart_filter = args.dart_filter.split(',') if args.dart_filter else None
     RunDartSmokeTest(build_dir, args.verbose_dart_snapshot)
     RunLitetestTests(build_dir)
+    RunGithooksTests(build_dir)
     RunDartTests(build_dir, dart_filter, args.verbose_dart_snapshot)
     RunConstFinderTests(build_dir)
     RunFrontEndServerTests(build_dir)

tools/githooks/README.md

+# Git Hooks
+
+The behavior of `git` commands can be customized through the use of "hooks".
+These hooks are described in detail in git's
+[documentation](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks).
+
+`git` looks for an executables by name in the directory specified by
+the `core.hooksPath` `git config` setting. The script `setup.py` here points
+`core.hooksPath` at this directory. It runs during a `gclient sync` or a
+`gclient runhooks`.
+
+The hooks here are implemented in Dart by the program with
+entrypoint `bin/main.dart` in this directory. The commands of the program
+are the implementation of the different hooks, for example
+`bin/main.dart pre-push ...`. Since the Dart program itself isn't an executable,
+these commands are invoked by small Python wrapper scripts. These wrapper
+scripts have the names that `git` will look for.
+
+## pre-push
+
+This hooks runs when pushing commits to a remote branch, for example to
+create or update a pull request: `git push origin my-local-branch`.
+
+The `pre-push` hook runs `ci/lint.sh` and `ci/format.sh`. `ci/analyze.sh` and
+`ci/licenses.sh` are more expensive and are not run.
+
+### Adding new pre-push checks
+
+Since the pre-push checks run on every `git push`, they should run quickly.
+New checks can be added by modifying the `run()` method of the `PrePushCommand`
+class in `lib/src/pre_push_command.dart`.
+
+## Creating a new hook
+
+1. Check the `git` documentation, and copy `pre-push` into a script with
+the right name.
+1. Make sure the script has the executable bit set
+(`chmod +x <script>`).
+1. Add a new `Command` implementation under `lib/src`. Give the new
+`Command` the same name as the new hook.
+1. Add the new `Command` to the `CommandRunner` in `lib/githooks.dart`.
+1. Make sure the script from step (1) is passing the new command to the Dart
+program.

tools/githooks/bin/main.dart

+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.12
+
+import 'package:githooks/githooks.dart';
+
+Future<int> main(List<String> args) async {
+  return run(args);
+}

tools/githooks/lib/githooks.dart

+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.12
+
+import 'dart:io' as io;
+
+import 'package:args/args.dart';
+import 'package:args/command_runner.dart';
+
+import 'src/pre_push_command.dart';
+
+/// Runs the githooks
+Future<int> run(List<String> args) async {
+  final CommandRunner<bool> runner = CommandRunner<bool> (
+    'githooks',
+    'Githooks implementation for the flutter/engine repo.',
+  )
+  ..addCommand(PrePushCommand());
+
+  // Add top-level arguments.
+  runner.argParser
+    ..addOption(
+      'flutter',
+      abbr: 'f',
+      help: 'The absolute path to the root of the flutter/engine checkout.',
+    )
+    ..addFlag(
+      'verbose',
+      abbr: 'v',
+      help: 'Runs with verbose logging',
+      defaultsTo: false,
+    );
+
+  if (args.isEmpty) {
+    // The tool was invoked with no arguments. Print usage.
+    runner.printUsage();
+    return 1;
+  }
+
+  final ArgResults argResults = runner.parse(args);
+  final String? argMessage = _checkArgs(argResults);
+  if (argMessage != null) {
+    io.stderr.writeln(argMessage);
+    runner.printUsage();
+    return 1;
+  }
+
+  final bool commandResult = await runner.runCommand(argResults) ?? false;
+  return commandResult ? 0 : 1;
+}
+
+String? _checkArgs(ArgResults argResults) {
+  if (argResults.command?.name == 'help') {
+    return null;
+  }
+
+  if (argResults['help'] as bool) {
+    return null;
+  }
+
+  if (argResults['flutter'] == null) {
+    return 'The --flutter option is required';
+  }
+
+  final io.Directory dir = io.Directory(argResults['flutter'] as String);
+  if (!dir.isAbsolute) {
+    return 'The --flutter option must be an absolute path';
+  }
+
+  if (!dir.existsSync()) {
+    return 'The directory specified by the --flutter option must exist';
+  }
+
+  return null;
+}

tools/githooks/lib/src/pre_push_command.dart

+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.12
+
+import 'dart:io' as io;
+
+import 'package:args/command_runner.dart';
+import 'package:path/path.dart' as path;
+
+/// The command that implements the pre-push githook
+class PrePushCommand extends Command<bool> {
+  @override
+  final String name = 'pre-push';
+
+  @override
+  final String description = 'Checks to run before a "git push"';
+
+  @override
+  Future<bool> run() async {
+    final Stopwatch sw = Stopwatch()..start();
+    final bool verbose = globalResults!['verbose']! as bool;
+    final String flutterRoot = globalResults!['flutter']! as String;
+    final List<bool> checkResults = await Future.wait<bool>(<Future<bool>>[
+      _runLinter(flutterRoot, verbose),
+      _runFormatter(flutterRoot, verbose),
+    ]);
+    sw.stop();
+    io.stdout.writeln('pre-push checks finished in ${sw.elapsed}');
+    return !checkResults.contains(false);
+  }
+
+  Future<bool> _runLinter(String flutterRoot, bool verbose) async {
+    if (io.Platform.isWindows) {
+      return true;
+    }
+    return _runCheck(
+      flutterRoot,
+      path.join(flutterRoot, 'ci', 'lint.sh'),
+      <String>[],
+      'Linting check',
+      verbose: verbose,
+    );
+  }
+
+  Future<bool> _runFormatter(String flutterRoot, bool verbose) {
+    final String ext = io.Platform.isWindows ? '.bat' : '.sh';
+    return _runCheck(
+      flutterRoot,
+      path.join(flutterRoot, 'ci', 'format$ext'),
+      <String>[],
+      'Formatting check',
+      verbose: verbose,
+    );
+  }
+
+  Future<bool> _runCheck(
+    String flutterRoot,
+    String scriptPath,
+    List<String> scriptArgs,
+    String checkName, {
+    bool verbose = false,
+  }) async {
+    if (verbose) {
+      io.stdout.writeln('Starting "$checkName": $scriptPath');
+    }
+    final io.ProcessResult result = await io.Process.run(
+      scriptPath,
+      scriptArgs,
+      workingDirectory: flutterRoot,
+    );
+    if (result.exitCode != 0) {
+      final StringBuffer message = StringBuffer();
+      message.writeln('Check "$checkName" failed.');
+      message.writeln('command: $scriptPath ${scriptArgs.join(" ")}');
+      message.writeln('working directory: $flutterRoot');
+      message.writeln('exit code: ${result.exitCode}');
+      message.writeln('stdout:');
+      message.writeln(result.stdout);
+      message.writeln('stderr:');
+      message.writeln(result.stderr);
+      io.stderr.write(message.toString());
+      return false;
+    }
+    if (verbose) {
+      io.stdout.writeln('Check "$checkName" finished successfully.');
+    }
+    return true;
+  }
+}

tools/githooks/pre-push

+#!/usr/bin/env python3
+# Copyright 2013 The Flutter Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+'''
+Runs the pre-push githooks.
+'''
+
+import os
+import subprocess
+import sys
+
+
+SRC_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))
+FLUTTER_DIR = os.path.join(SRC_ROOT, 'flutter')
+DART_BIN = os.path.join(SRC_ROOT, 'third_party', 'dart', 'tools', 'sdks', 'dart-sdk', 'bin')
+
+
+def Main(argv):
+  result = subprocess.run([
+    os.path.join(DART_BIN, 'dart'),
+    '--disable-dart-dev',
+    os.path.join(FLUTTER_DIR, 'tools', 'githooks', 'bin', 'main.dart'),
+    '--flutter',
+    FLUTTER_DIR,
+    'pre-push',
+  ], cwd=SRC_ROOT)
+  return result.returncode
+
+
+if __name__ == '__main__':
+    sys.exit(Main(sys.argv))

tools/githooks/pubspec.yaml

+# Copyright 2013 The Flutter Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+name: githooks
+publish_to: none
+environment:
+  sdk: '>=2.12.0-0.0.dev <3.0.0'
+
+# Do not add any dependencies that require more than what is provided in
+# //third_party.pkg, //third_party/dart/pkg, or
+# //third_party/dart/third_party/pkg. In particular, package:test is not usable
+# here.
+
+# If you do add packages here, make sure you can run `pub get --offline`, and
+# check the .packages and .package_config to make sure all the paths are
+# relative to this directory into //third_party/dart
+
+dependencies:
+  args: any
+  meta: any
+  path: any
+
+dev_dependencies:
+  async_helper: any
+  expect: any
+  litetest: any
+
+dependency_overrides:
+  args:
+    path: ../../../third_party/dart/third_party/pkg/args
+  async_helper:
+    path: ../../../third_party/dart/pkg/async_helper
+  expect:
+    path: ../../../third_party/dart/pkg/expect
+  litetest:
+    path: ../../testing/litetest
+  meta:
+    path: ../../../third_party/dart/pkg/meta
+  path:
+    path: ../../../third_party/dart/third_party/pkg/path

tools/githooks/setup.py

+#!/usr/bin/env python3
+# Copyright 2013 The Flutter Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+'''
+Sets up githooks.
+'''
+
+import os
+import subprocess
+import sys
+
+
+SRC_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))
+FLUTTER_DIR = os.path.join(SRC_ROOT, 'flutter')
+
+
+def Main(argv):
+  result = subprocess.run([
+    'git',
+    'config',
+    'core.hooksPath',
+    os.path.join(FLUTTER_DIR, 'tools', 'githooks'),
+  ], cwd=FLUTTER_DIR)
+  return result.returncode
+
+
+if __name__ == '__main__':
+    sys.exit(Main(sys.argv))

tools/githooks/test/githooks_test.dart

+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.12
+
+import 'dart:io' as io;
+
+import 'package:githooks/githooks.dart';
+import 'package:litetest/litetest.dart';
+
+void main() {
+  test('Fails gracefully without a command', () async {
+    int? result;
+    try {
+      result = await run(<String>[]);
+    } catch (e, st) {
+      fail('Unexpected exception: $e\n$st');
+    }
+    expect(result, equals(1));
+  });
+
+  test('Fails gracefully with an unknown command', () async {
+    int? result;
+    try {
+      result = await run(<String>['blah']);
+    } catch (e, st) {
+      fail('Unexpected exception: $e\n$st');
+    }
+    expect(result, equals(1));
+  });
+
+  test('Fails gracefully without --flutter', () async {
+    int? result;
+    try {
+      result = await run(<String>['pre-push']);
+    } catch (e, st) {
+      fail('Unexpected exception: $e\n$st');
+    }
+    expect(result, equals(1));
+  });
+
+  test('Fails gracefully when --flutter is not an absolute path', () async {
+    int? result;
+    try {
+      result = await run(<String>[
+        'pre-push',
+        '--flutter',
+        'non/absolute',
+      ]);
+    } catch (e, st) {
+      fail('Unexpected exception: $e\n$st');
+    }
+    expect(result, equals(1));
+  });
+
+  test('Fails gracefully when --flutter does not exist', () async {
+    int? result;
+    try {
+      result = await run(<String>[
+        'pre-push',
+        '--flutter',
+        if (io.Platform.isWindows) r'C:\does\not\exist'
+        else '/does/not/exist',
+      ]);
+    } catch (e, st) {
+      fail('Unexpected exception: $e\n$st');
+    }
+    expect(result, equals(1));
+  });
+}

tools/pub_get_offline.py

@@ -23,6 +23,7 @@ ALL_PACKAGES = [
   os.path.join("src", "flutter", "testing", "symbols"),
   os.path.join("src", "flutter", "tools", "android_lint"),
   os.path.join("src", "flutter", "tools", "const_finder"),
+  os.path.join("src", "flutter", "tools", "githooks"),
   os.path.join("src", "flutter", "tools", "licenses"),
 ]