Skip to content

Commit

Permalink
[tool] Add a package-level pre-publish hook (#7156)
Browse files Browse the repository at this point in the history
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/flutter#150210
  • Loading branch information
stuartmorgan authored Jul 25, 2024
1 parent 94f8b2f commit 50238e7
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 7 deletions.
8 changes: 8 additions & 0 deletions script/tool/lib/src/common/repository_package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
15 changes: 8 additions & 7 deletions script/tool/lib/src/custom_test_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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>[],
Expand All @@ -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();
Expand Down
31 changes: 31 additions & 0 deletions script/tool/lib/src/publish_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
30 changes: 30 additions & 0 deletions script/tool/lib/src/publish_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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']);
}
Expand Down Expand Up @@ -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 '
Expand Down
82 changes: 82 additions & 0 deletions script/tool/test/publish_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
85 changes: 85 additions & 0 deletions script/tool/test/publish_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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>[]);
Expand Down

0 comments on commit 50238e7

Please sign in to comment.