[tool] Add a package-level pre-publish hook (flutter#7156)

stuartmorgan-g · web-flow · commit 50238e7dd30e · 2024-07-25T20:06:18.000Z
Adds the ability for a package to specify a script that should be run before publishing. To minimize the chance of such a script breaking things only in the post-submit `release` step, if the script is present it will also be run during `publish-check`. These should be used with caution since they can cause the published artifacts to be different from that is checked in, but in the intended use case of extension builds this risk is far preferable to the risks associated with checking in binaries that were built on local, ad-hoc basis. (Longer term, we may need an alternate solution however, as generating artifacts in CI can have its own supply chain validation issues.) Also does some minor refactoring to custom test script code to make it follow the same pattern as this new code. Fixes flutter#150210
diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart
@@ -66,6 +66,14 @@ class RepositoryPackage {
   /// The test directory containing the package's Dart tests.
   Directory get testDirectory => directory.childDirectory('test');
 
+  /// The path to the script that is run by the `custom-test` command.
+  File get customTestScript =>
+      directory.childDirectory('tool').childFile('run_tests.dart');
+
+  /// The path to the script that is run before publishing.
+  File get prePublishScript =>
+      directory.childDirectory('tool').childFile('pre_publish.dart');
+
   /// Returns the directory containing support for [platform].
   Directory platformDirectory(FlutterPlatform platform) {
     late final String directoryName;
diff --git a/script/tool/lib/src/custom_test_command.dart b/script/tool/lib/src/custom_test_command.dart
@@ -8,7 +8,6 @@ import 'common/package_looping_command.dart';
 import 'common/pub_utils.dart';
 import 'common/repository_package.dart';
 
-const String _scriptName = 'run_tests.dart';
 const String _legacyScriptName = 'run_tests.sh';
 
 /// A command to run custom, package-local tests on packages.
@@ -32,13 +31,14 @@ class CustomTestCommand extends PackageLoopingCommand {
 
   @override
   final String description = 'Runs package-specific custom tests defined in '
-      "a package's tool/$_scriptName file.\n\n"
+      "a package's custom test script.\n\n"
       'This command requires "dart" to be in your path.';
 
   @override
   Future<PackageResult> runForPackage(RepositoryPackage package) async {
-    final File script =
-        package.directory.childDirectory('tool').childFile(_scriptName);
+    final File script = package.customTestScript;
+    final String relativeScriptPath =
+        getRelativePosixPath(script, from: package.directory);
     final File legacyScript = package.directory.childFile(_legacyScriptName);
     String? customSkipReason;
     bool ranTests = false;
@@ -52,7 +52,7 @@ class CustomTestCommand extends PackageLoopingCommand {
       }
 
       final int testExitCode = await processRunner.runAndStream(
-          'dart', <String>['run', 'tool/$_scriptName'],
+          'dart', <String>['run', relativeScriptPath],
           workingDir: package.directory);
       if (testExitCode != 0) {
         return PackageResult.fail();
@@ -64,7 +64,7 @@ class CustomTestCommand extends PackageLoopingCommand {
     if (legacyScript.existsSync()) {
       if (platform.isWindows) {
         customSkipReason = '$_legacyScriptName is not supported on Windows. '
-            'Please migrate to $_scriptName.';
+            'Please migrate to $relativeScriptPath.';
       } else {
         final int exitCode = await processRunner.runAndStream(
             legacyScript.path, <String>[],
@@ -77,7 +77,8 @@ class CustomTestCommand extends PackageLoopingCommand {
     }
 
     if (!ranTests) {
-      return PackageResult.skip(customSkipReason ?? 'No custom tests');
+      return PackageResult.skip(
+          customSkipReason ?? 'No $relativeScriptPath file');
     }
 
     return PackageResult.success();
diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart
@@ -6,6 +6,7 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io' as io;
 
+import 'package:file/file.dart';
 import 'package:http/http.dart' as http;
 import 'package:pub_semver/pub_semver.dart';
 
@@ -83,6 +84,9 @@ class PublishCheckCommand extends PackageLoopingCommand {
           isError: true);
       result = _PublishCheckResult.error;
     }
+    if (!await _validatePrePublishHook(package)) {
+      result = _PublishCheckResult.error;
+    }
 
     if (result.index > _overallResult.index) {
       _overallResult = result;
@@ -268,6 +272,33 @@ HTTP response: ${pubVersionFinderResponse.httpResponse.body}
     return package.authorsFile.existsSync();
   }
 
+  Future<bool> _validatePrePublishHook(RepositoryPackage package) async {
+    final File script = package.prePublishScript;
+    if (!script.existsSync()) {
+      // If there's no custom step, then it can't block publishing.
+      return true;
+    }
+    final String relativeScriptPath =
+        getRelativePosixPath(script, from: package.directory);
+    print('Running pre-publish hook $relativeScriptPath...');
+
+    // Ensure that dependencies are available.
+    if (!await runPubGet(package, processRunner, platform)) {
+      _printImportantStatusMessage('Failed to get depenedencies',
+          isError: true);
+      return false;
+    }
+
+    final int exitCode = await processRunner.runAndStream(
+        'dart', <String>['run', relativeScriptPath],
+        workingDir: package.directory);
+    if (exitCode != 0) {
+      _printImportantStatusMessage('Pre-publish script failed.', isError: true);
+      return false;
+    }
+    return true;
+  }
+
   void _printImportantStatusMessage(String message, {required bool isError}) {
     final String statusMessage = '${isError ? 'ERROR' : 'SUCCESS'}: $message';
     if (getBoolArg(_machineFlag)) {
diff --git a/script/tool/lib/src/publish_command.dart b/script/tool/lib/src/publish_command.dart
@@ -21,6 +21,7 @@ import 'common/git_version_finder.dart';
 import 'common/output_utils.dart';
 import 'common/package_command.dart';
 import 'common/package_looping_command.dart';
+import 'common/pub_utils.dart';
 import 'common/pub_version_finder.dart';
 import 'common/repository_package.dart';
 
@@ -201,6 +202,10 @@ class PublishCommand extends PackageLoopingCommand {
       return checkResult;
     }
 
+    if (!await _runPrePublishScript(package)) {
+      return PackageResult.fail(<String>['pre-publish failed']);
+    }
+
     if (!await _checkGitStatus(package)) {
       return PackageResult.fail(<String>['uncommitted changes']);
     }
@@ -375,6 +380,31 @@ Safe to ignore if the package is deleted in this commit.
     return getRemoteUrlResult.stdout as String?;
   }
 
+  Future<bool> _runPrePublishScript(RepositoryPackage package) async {
+    final File script = package.prePublishScript;
+    if (!script.existsSync()) {
+      return true;
+    }
+    final String relativeScriptPath =
+        getRelativePosixPath(script, from: package.directory);
+    print('Running pre-publish hook $relativeScriptPath...');
+
+    // Ensure that dependencies are available.
+    if (!await runPubGet(package, processRunner, platform)) {
+      printError('Failed to get depenedencies');
+      return false;
+    }
+
+    final int exitCode = await processRunner.runAndStream(
+        'dart', <String>['run', relativeScriptPath],
+        workingDir: package.directory);
+    if (exitCode != 0) {
+      printError('Pre-publish script failed.');
+      return false;
+    }
+    return true;
+  }
+
   Future<bool> _publish(RepositoryPackage package) async {
     print('Publishing...');
     print('Running `pub publish ${_publishFlags.join(' ')}` in '
diff --git a/script/tool/test/publish_check_command_test.dart b/script/tool/test/publish_check_command_test.dart
@@ -369,6 +369,88 @@ void main() {
           ));
     });
 
+    group('pre-publish script', () {
+      test('runs if present', () async {
+        final RepositoryPackage package =
+            createFakePackage('a_package', packagesDir, examples: <String>[]);
+        package.prePublishScript.createSync(recursive: true);
+
+        final List<String> output = await runCapturingPrint(runner, <String>[
+          'publish-check',
+        ]);
+
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Running pre-publish hook tool/pre_publish.dart...'),
+          ]),
+        );
+        expect(
+            processRunner.recordedCalls,
+            containsAllInOrder(<ProcessCall>[
+              ProcessCall(
+                  'dart',
+                  const <String>[
+                    'pub',
+                    'get',
+                  ],
+                  package.directory.path),
+              ProcessCall(
+                  'dart',
+                  const <String>[
+                    'run',
+                    'tool/pre_publish.dart',
+                  ],
+                  package.directory.path),
+            ]));
+      });
+
+      test('causes command failure if it fails', () async {
+        final RepositoryPackage package = createFakePackage(
+            'a_package', packagesDir,
+            isFlutter: true, examples: <String>[]);
+        package.prePublishScript.createSync(recursive: true);
+
+        processRunner.mockProcessesForExecutable['dart'] = <FakeProcessInfo>[
+          FakeProcessInfo(MockProcess(exitCode: 1),
+              <String>['run']), // run tool/pre_publish.dart
+        ];
+
+        Error? commandError;
+        final List<String> output = await runCapturingPrint(runner, <String>[
+          'publish-check',
+        ], errorHandler: (Error e) {
+          commandError = e;
+        });
+
+        expect(commandError, isA<ToolExit>());
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Pre-publish script failed.'),
+          ]),
+        );
+        expect(
+            processRunner.recordedCalls,
+            containsAllInOrder(<ProcessCall>[
+              ProcessCall(
+                  getFlutterCommand(mockPlatform),
+                  const <String>[
+                    'pub',
+                    'get',
+                  ],
+                  package.directory.path),
+              ProcessCall(
+                  'dart',
+                  const <String>[
+                    'run',
+                    'tool/pre_publish.dart',
+                  ],
+                  package.directory.path),
+            ]));
+      });
+    });
+
     test(
         '--machine: Log JSON with status:no-publish and correct human message, if there are no packages need to be published. ',
         () async {
diff --git a/script/tool/test/publish_command_test.dart b/script/tool/test/publish_command_test.dart
@@ -141,6 +141,91 @@ void main() {
     });
   });
 
+  group('pre-publish script', () {
+    test('runs if present', () async {
+      final RepositoryPackage package =
+          createFakePackage('foo', packagesDir, examples: <String>[]);
+      package.prePublishScript.createSync(recursive: true);
+
+      final List<String> output =
+          await runCapturingPrint(commandRunner, <String>[
+        'publish',
+        '--packages=foo',
+      ]);
+
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains('Running pre-publish hook tool/pre_publish.dart...'),
+        ]),
+      );
+      expect(
+          processRunner.recordedCalls,
+          containsAllInOrder(<ProcessCall>[
+            ProcessCall(
+                'dart',
+                const <String>[
+                  'pub',
+                  'get',
+                ],
+                package.directory.path),
+            ProcessCall(
+                'dart',
+                const <String>[
+                  'run',
+                  'tool/pre_publish.dart',
+                ],
+                package.directory.path),
+          ]));
+    });
+
+    test('causes command failure if it fails', () async {
+      final RepositoryPackage package = createFakePackage('foo', packagesDir,
+          isFlutter: true, examples: <String>[]);
+      package.prePublishScript.createSync(recursive: true);
+
+      processRunner.mockProcessesForExecutable['dart'] = <FakeProcessInfo>[
+        FakeProcessInfo(MockProcess(exitCode: 1),
+            <String>['run']), // run tool/pre_publish.dart
+      ];
+
+      Error? commandError;
+      final List<String> output =
+          await runCapturingPrint(commandRunner, <String>[
+        'publish',
+        '--packages=foo',
+      ], errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains('Pre-publish script failed.'),
+        ]),
+      );
+      expect(
+          processRunner.recordedCalls,
+          containsAllInOrder(<ProcessCall>[
+            ProcessCall(
+                getFlutterCommand(platform),
+                const <String>[
+                  'pub',
+                  'get',
+                ],
+                package.directory.path),
+            ProcessCall(
+                'dart',
+                const <String>[
+                  'run',
+                  'tool/pre_publish.dart',
+                ],
+                package.directory.path),
+          ]));
+    });
+  });
+
   group('Publishes package', () {
     test('while showing all output from pub publish to the user', () async {
       createFakePlugin('plugin1', packagesDir, examples: <String>[]);
