Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pigeon] Support other hosts in generated file CI checks #5944

Merged
merged 16 commits into from
Jan 20, 2024
Merged

[pigeon] Support other hosts in generated file CI checks #5944

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
bb14fc7
Temp hack for local testing
stuartmorgan Jan 19, 2024
cb16cfd
Include swift for testing
stuartmorgan Jan 19, 2024
f2ad42a
Add explicit language selection for formatting
stuartmorgan Jan 19, 2024
3872d99
Per-platform language split
stuartmorgan Jan 19, 2024
cbdf3e9
Fully wire up
stuartmorgan Jan 19, 2024
28c2e4a
Disable for now
stuartmorgan Jan 19, 2024
262da18
Remove local hack
stuartmorgan Jan 19, 2024
b501fc7
Enable Swift check
stuartmorgan Jan 20, 2024
8d6bcb3
Merge branch 'main' into pigeon-format-check
stuartmorgan Jan 20, 2024
e136357
Remove unconditional --no-swift
stuartmorgan Jan 20, 2024
aa1158a
Add .h for ObjC
stuartmorgan Jan 20, 2024
ae2c565
Merge remote-tracking branch 'upstream/main' into pigeon-format-check
stuartmorgan Jan 20, 2024
c505762
Remove generated code filter from Swift formatting
stuartmorgan Jan 20, 2024
f875c84
Autoformat generated output
stuartmorgan Jan 20, 2024
afaaf0d
Autoformat quick_actions
stuartmorgan Jan 20, 2024
ecce48d
Merge remote-tracking branch 'upstream/main' into pigeon-format-check
stuartmorgan Jan 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 71 additions & 69 deletions packages/pigeon/tool/run_tests.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ library;

import 'dart:io';

import 'package:collection/collection.dart';
import 'package:path/path.dart' as p;

import 'shared/generation.dart';
Expand All @@ -35,62 +36,55 @@ void _validateTestCoverage(List<List<String>> shards) {
}

Future<void> _validateGeneratedTestFiles() async {
final String baseDir = p.dirname(p.dirname(Platform.script.toFilePath()));
final String repositoryRoot = p.dirname(p.dirname(baseDir));
final String relativePigeonPath = p.relative(baseDir, from: repositoryRoot);

print('Validating generated files:');
print(' Generating test output...');
final int generateExitCode = await generateTestPigeons(baseDir: baseDir);
if (generateExitCode != 0) {
print('Generation failed; see above for errors.');
exit(generateExitCode);
}

print(' Formatting output...');
final int formatExitCode =
await formatAllFiles(repositoryRoot: repositoryRoot);
if (formatExitCode != 0) {
print('Formatting failed; see above for errors.');
exit(formatExitCode);
}
await _validateGeneratedFiles(
(String baseDir) => generateTestPigeons(baseDir: baseDir),
generationMessage: 'Generating test output',
incorrectFilesMessage:
'The following files are not updated, or not formatted correctly:',
);
}

print(' Checking for changes...');
final List<String> modifiedFiles = await _modifiedFiles(
repositoryRoot: repositoryRoot, relativePigeonPath: relativePigeonPath);
Future<void> _validateGeneratedExampleFiles() async {
await _validateGeneratedFiles(
(String _) => generateExamplePigeons(),
generationMessage: 'Generating example output',
incorrectFilesMessage:
'Either messages.dart and messages_test.dart have non-matching definitions or\n'
'the following files are not updated, or not formatted correctly:',
);
}

if (modifiedFiles.isEmpty) {
Future<void> _validateGeneratedFiles(
Future<int> Function(String baseDirectory) generator, {
required String generationMessage,
required String incorrectFilesMessage,
}) async {
// Generated file validation is split by platform, both to avoid duplication
// of work, and to avoid issues if different CI configurations have different
// setups (e.g., different clang-format versions or no clang-format at all).
final Set<GeneratorLanguage> languagesToValidate;
if (Platform.isLinux) {
languagesToValidate = <GeneratorLanguage>{
GeneratorLanguage.cpp,
GeneratorLanguage.dart,
GeneratorLanguage.java,
GeneratorLanguage.kotlin,
GeneratorLanguage.objc,
};
} else if (Platform.isMacOS) {
languagesToValidate = <GeneratorLanguage>{
GeneratorLanguage.swift,
};
} else {
return;
}

print('The following files are not updated, or not formatted correctly:');
modifiedFiles.map((String line) => ' $line').forEach(print);

print('\nTo fix run "dart run tool/generate.dart --format" from the pigeon/ '
'directory, or apply the diff with the command below.\n');

final ProcessResult diffResult = await Process.run(
'git',
<String>['diff', relativePigeonPath],
workingDirectory: repositoryRoot,
);
if (diffResult.exitCode != 0) {
print('Unable to determine diff.');
exit(1);
}
print('patch -p1 <<DONE');
print(diffResult.stdout);
print('DONE');
exit(1);
}

Future<void> _validateGeneratedExampleFiles() async {
final String baseDir = p.dirname(p.dirname(Platform.script.toFilePath()));
final String repositoryRoot = p.dirname(p.dirname(baseDir));
final String relativePigeonPath = p.relative(baseDir, from: repositoryRoot);

print('Validating generated files:');
print(' Generating example output...');
print(' $generationMessage...');

final int generateExitCode = await generateExamplePigeons();

Expand All @@ -100,8 +94,8 @@ Future<void> _validateGeneratedExampleFiles() async {
}

print(' Formatting output...');
final int formatExitCode =
await formatAllFiles(repositoryRoot: repositoryRoot);
final int formatExitCode = await formatAllFiles(
repositoryRoot: repositoryRoot, languages: languagesToValidate);
if (formatExitCode != 0) {
print('Formatting failed; see above for errors.');
exit(formatExitCode);
Expand All @@ -110,22 +104,26 @@ Future<void> _validateGeneratedExampleFiles() async {
print(' Checking for changes...');
final List<String> modifiedFiles = await _modifiedFiles(
repositoryRoot: repositoryRoot, relativePigeonPath: relativePigeonPath);

if (modifiedFiles.isEmpty) {
final Set<String> extensions = languagesToValidate
.map((GeneratorLanguage lang) => _extensionsForLanguage(lang))
.flattened
.toSet();
final Iterable<String> filteredFiles = modifiedFiles.where((String path) =>
extensions.contains(p.extension(path).replaceFirst('.', '')));

if (filteredFiles.isEmpty) {
return;
}

print(
'Either messages.dart and messages_test.dart have non-matching definitions or');
print('the following files are not updated, or not formatted correctly:');
modifiedFiles.map((String line) => ' $line').forEach(print);
print(incorrectFilesMessage);
filteredFiles.map((String line) => ' $line').forEach(print);

print('\nTo fix run "dart run tool/generate.dart --format" from the pigeon/ '
'directory, or apply the diff with the command below.\n');

final ProcessResult diffResult = await Process.run(
'git',
<String>['diff', relativePigeonPath],
<String>['diff', ...filteredFiles],
workingDirectory: repositoryRoot,
);
if (diffResult.exitCode != 0) {
Expand All @@ -138,6 +136,17 @@ Future<void> _validateGeneratedExampleFiles() async {
exit(1);
}

Set<String> _extensionsForLanguage(GeneratorLanguage language) {
return switch (language) {
GeneratorLanguage.cpp => <String>{'cc', 'cpp', 'h'},
GeneratorLanguage.dart => <String>{'dart'},
GeneratorLanguage.java => <String>{'java'},
GeneratorLanguage.kotlin => <String>{'kt'},
GeneratorLanguage.swift => <String>{'swift'},
GeneratorLanguage.objc => <String>{'m', 'mm'},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how are we handling .h files for objc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! They should be listed here; having duplicates will be fine since.

};
}

Future<List<String>> _modifiedFiles(
{required String repositoryRoot,
required String relativePigeonPath}) async {
Expand Down Expand Up @@ -204,20 +213,13 @@ Future<void> main(List<String> args) async {
],
]);

// Ensure that all generated files are up to date. This is run only on Linux
// both to avoid duplication of work, and to avoid issues if different CI
// configurations have different setups (e.g., different clang-format versions
// or no clang-format at all).
if (Platform.isLinux) {
// Only run on master, since Dart format can change between versions.
// TODO(stuartmorgan): Make a more generic way to run this check only on
// master; this currently won't work for anything but Cirrus.
if (Platform.environment['CHANNEL'] == 'stable') {
print('Skipping generated file validation on stable.');
} else {
await _validateGeneratedTestFiles();
await _validateGeneratedExampleFiles();
}
// Ensure that all generated files are up to date.
// Only run on master, since Dart format can change between versions.
if (Platform.environment['CHANNEL'] == 'stable') {
print('Skipping generated file validation on stable.');
} else {
await _validateGeneratedTestFiles();
await _validateGeneratedExampleFiles();
}

final List<String> testsToRun;
Expand Down
65 changes: 48 additions & 17 deletions packages/pigeon/tool/shared/generation.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import 'package:pigeon/pigeon.dart';

import 'process_utils.dart';

enum GeneratorLanguages {
enum GeneratorLanguage {
cpp,
dart,
java,
kotlin,
objc,
Expand All @@ -20,8 +21,8 @@ enum GeneratorLanguages {

// A map of pigeons/ files to the languages that they can't yet be generated
// for due to limitations of that generator.
const Map<String, Set<GeneratorLanguages>> _unsupportedFiles =
<String, Set<GeneratorLanguages>>{};
const Map<String, Set<GeneratorLanguage>> _unsupportedFiles =
<String, Set<GeneratorLanguage>>{};

String _snakeToPascalCase(String snake) {
final List<String> parts = snake.split('_');
Expand Down Expand Up @@ -77,28 +78,28 @@ Future<int> generateTestPigeons({required String baseDir}) async {

for (final String input in inputs) {
final String pascalCaseName = _snakeToPascalCase(input);
final Set<GeneratorLanguages> skipLanguages =
_unsupportedFiles[input] ?? <GeneratorLanguages>{};
final Set<GeneratorLanguage> skipLanguages =
_unsupportedFiles[input] ?? <GeneratorLanguage>{};

// Generate the default language test plugin output.
int generateCode = await runPigeon(
input: './pigeons/$input.dart',
dartOut: '$sharedDartOutputBase/lib/src/generated/$input.gen.dart',
// Android
kotlinOut: skipLanguages.contains(GeneratorLanguages.kotlin)
kotlinOut: skipLanguages.contains(GeneratorLanguage.kotlin)
? null
: '$outputBase/android/src/main/kotlin/com/example/test_plugin/$pascalCaseName.gen.kt',
kotlinPackage: 'com.example.test_plugin',
kotlinErrorClassName: '${pascalCaseName}Error',
// iOS
swiftOut: skipLanguages.contains(GeneratorLanguages.swift)
swiftOut: skipLanguages.contains(GeneratorLanguage.swift)
? null
: '$outputBase/ios/Classes/$pascalCaseName.gen.swift',
// Windows
cppHeaderOut: skipLanguages.contains(GeneratorLanguages.cpp)
cppHeaderOut: skipLanguages.contains(GeneratorLanguage.cpp)
? null
: '$outputBase/windows/pigeon/$input.gen.h',
cppSourceOut: skipLanguages.contains(GeneratorLanguages.cpp)
cppSourceOut: skipLanguages.contains(GeneratorLanguage.cpp)
? null
: '$outputBase/windows/pigeon/$input.gen.cpp',
cppNamespace: '${input}_pigeontest',
Expand All @@ -114,7 +115,7 @@ Future<int> generateTestPigeons({required String baseDir}) async {
// single invocation.
generateCode = await runPigeon(
input: './pigeons/$input.dart',
swiftOut: skipLanguages.contains(GeneratorLanguages.swift)
swiftOut: skipLanguages.contains(GeneratorLanguage.swift)
? null
: '$outputBase/macos/Classes/$pascalCaseName.gen.swift',
suppressVersion: true,
Expand All @@ -130,16 +131,16 @@ Future<int> generateTestPigeons({required String baseDir}) async {
// Android
// This doesn't use the '.gen' suffix since Java has strict file naming
// rules.
javaOut: skipLanguages.contains(GeneratorLanguages.java)
javaOut: skipLanguages.contains(GeneratorLanguage.java)
? null
: '$alternateOutputBase/android/src/main/java/com/example/'
'alternate_language_test_plugin/${_javaFilenameForName(input)}.java',
javaPackage: 'com.example.alternate_language_test_plugin',
// iOS
objcHeaderOut: skipLanguages.contains(GeneratorLanguages.objc)
objcHeaderOut: skipLanguages.contains(GeneratorLanguage.objc)
? null
: '$alternateOutputBase/ios/Classes/$pascalCaseName.gen.h',
objcSourceOut: skipLanguages.contains(GeneratorLanguages.objc)
objcSourceOut: skipLanguages.contains(GeneratorLanguage.objc)
? null
: '$alternateOutputBase/ios/Classes/$pascalCaseName.gen.m',
suppressVersion: true,
Expand All @@ -154,10 +155,10 @@ Future<int> generateTestPigeons({required String baseDir}) async {
// single invocation.
generateCode = await runPigeon(
input: './pigeons/$input.dart',
objcHeaderOut: skipLanguages.contains(GeneratorLanguages.objc)
objcHeaderOut: skipLanguages.contains(GeneratorLanguage.objc)
? null
: '$alternateOutputBase/macos/Classes/$pascalCaseName.gen.h',
objcSourceOut: skipLanguages.contains(GeneratorLanguages.objc)
objcSourceOut: skipLanguages.contains(GeneratorLanguage.objc)
? null
: '$alternateOutputBase/macos/Classes/$pascalCaseName.gen.m',
suppressVersion: true,
Expand Down Expand Up @@ -234,16 +235,46 @@ Future<int> runPigeon({
/// This is intended for formatting generated output, but since there's no
/// way to filter to specific files in with the repo tooling it runs over the
/// entire package.
Future<int> formatAllFiles({required String repositoryRoot}) {
Future<int> formatAllFiles({
required String repositoryRoot,
Set<GeneratorLanguage> languages = const <GeneratorLanguage>{
GeneratorLanguage.cpp,
GeneratorLanguage.dart,
GeneratorLanguage.java,
GeneratorLanguage.kotlin,
GeneratorLanguage.objc,
GeneratorLanguage.swift,
},
}) {
final String dartCommand = Platform.isWindows ? 'dart.exe' : 'dart';
return runProcess(
dartCommand,
<String>[
'run',
'script/tool/bin/flutter_plugin_tools.dart',
'format',
'--no-swift',
'--packages=pigeon',
if (languages.contains(GeneratorLanguage.cpp) ||
languages.contains(GeneratorLanguage.objc))
'--clang-format'
else
'--no-clang-format',
if (languages.contains(GeneratorLanguage.java))
'--java'
else
'--no-java',
if (languages.contains(GeneratorLanguage.dart))
'--dart'
else
'--no-dart',
if (languages.contains(GeneratorLanguage.kotlin))
'--kotlin'
else
'--no-kotlin',
if (languages.contains(GeneratorLanguage.swift))
'--swift'
else
'--no-swift',
],
workingDirectory: repositoryRoot,
streamOutput: false,
Expand Down