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

[tools] Run pub get before format #8052

Merged
merged 3 commits into from
Nov 11, 2024
Merged

[tools] Run pub get before format #8052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5716dac
Ensure that pub get is run in the format command
stuartmorgan Nov 11, 2024
080846f
Fetch before formatting
stuartmorgan Nov 11, 2024
3e5ccef
Ignore web generated plugin registrant
stuartmorgan Nov 11, 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
7 changes: 7 additions & 0 deletions .ci/targets/repo_checks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ tasks:
- name: prepare tool
script: .ci/scripts/prepare_tool.sh
infra_step: true # Note infra steps failing prevents "always" from running.
# format requires that 'pub get' has been run to determine the language
# version, so the tool will auto-run it if necessary. Run it manually first
# so that the network requests are in a separate infra step.
- name: download Dart deps
script: .ci/scripts/tool_runner.sh
args: ["fetch-deps"]
infra_step: true
- name: tool unit tests
script: .ci/scripts/plugin_tools_tests.sh
- name: tool format
Expand Down
10 changes: 10 additions & 0 deletions script/tool/lib/src/format_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:meta/meta.dart';
import 'common/core.dart';
import 'common/output_utils.dart';
import 'common/package_looping_command.dart';
import 'common/pub_utils.dart';
import 'common/repository_package.dart';

/// In theory this should be 8191, but in practice that was still resulting in
Expand Down Expand Up @@ -120,6 +121,15 @@ class FormatCommand extends PackageLoopingCommand {
relativeTo: package.directory,
);
if (getBoolArg(_dartArg)) {
// Ensure that .dart_tool exists, since without it `dart` doesn't know
// the lanugage version, so the formatter may give different output.
if (!package.directory.childDirectory('.dart_tool').existsSync()) {
Comment on lines +124 to +126
Copy link
Member

Choose a reason for hiding this comment

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

Should we warn through the console in case this step needs to be performed?

That would let us know if something like flutter clean gets added by mistake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We pass stdout/stderr through in runPubGet, so it's super obvious in the console when this is happening.

if (!await runPubGet(package, processRunner, super.platform)) {
printError('Unable to fetch dependencies.');
return PackageResult.fail(<String>['unable to fetch dependencies']);
}
}

await _formatDart(files, workingDir: package.directory);
}
// Success or failure is determined overall in completeRun, since most code
Expand Down
1 change: 1 addition & 0 deletions script/tool/lib/src/license_check_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const Set<String> _ignoreBasenameList = <String>{
'flutter_export_environment',
'GeneratedPluginRegistrant',
'generated_plugin_registrant',
'web_plugin_registrant',
};

// File suffixes that otherwise match _codeFileExtensions to ignore.
Expand Down
89 changes: 71 additions & 18 deletions script/tool/test/format_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ void main() {
runner.addCommand(analyzeCommand);
});

/// Creates the .dart_tool directory for [package] to simulate (as much as
/// this command requires) `pub get` having been run.
void fakePubGet(RepositoryPackage package) {
package.directory.childDirectory('.dart_tool').createSync();
}

/// Returns a modified version of a list of [relativePaths] that are relative
/// to [package] to instead be relative to [packagesDir].
List<String> getPackagesDirRelativePaths(
Expand Down Expand Up @@ -92,6 +98,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format']);

Expand All @@ -117,6 +124,7 @@ void main() {
unformattedFile,
],
);
fakePubGet(plugin);

final p.Context posixContext = p.posix;
childFileWithSubcomponents(
Expand All @@ -140,7 +148,9 @@ void main() {
'lib/src/b.dart',
'lib/src/c.dart',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['dart'] = <FakeProcessInfo>[
FakeProcessInfo(MockProcess(exitCode: 1), <String>['format'])
Expand All @@ -163,7 +173,9 @@ void main() {
const List<String> files = <String>[
'lib/a.dart',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-dart']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -179,6 +191,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format']);

Expand All @@ -203,7 +216,9 @@ void main() {
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
'android/src/main/java/io/flutter/plugins/a_plugin/b.java',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['java'] = <FakeProcessInfo>[
FakeProcessInfo(MockProcess(exitCode: 1), <String>['-version'])
Expand All @@ -229,7 +244,9 @@ void main() {
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
'android/src/main/java/io/flutter/plugins/a_plugin/b.java',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['java'] = <FakeProcessInfo>[
FakeProcessInfo(
Expand Down Expand Up @@ -260,6 +277,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(
runner, <String>['format', '--java-path=/path/to/java']);
Expand All @@ -284,7 +302,9 @@ void main() {
const List<String> files = <String>[
'android/src/main/java/io/flutter/plugins/a_plugin/a.java',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-java']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -304,6 +324,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format']);

Expand All @@ -328,7 +349,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['clang-format'] =
<FakeProcessInfo>[FakeProcessInfo(MockProcess(exitCode: 1))];
Expand Down Expand Up @@ -357,6 +380,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['clang-format'] =
<FakeProcessInfo>[FakeProcessInfo(MockProcess(exitCode: 1))];
Expand Down Expand Up @@ -396,6 +420,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner,
<String>['format', '--clang-format-path=/path/to/clang-format']);
Expand All @@ -421,7 +446,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['clang-format'] =
<FakeProcessInfo>[
Expand All @@ -448,7 +475,9 @@ void main() {
const List<String> files = <String>[
'linux/foo_plugin.cc',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-clang-format']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -465,6 +494,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format']);

Expand All @@ -488,7 +518,9 @@ void main() {
'android/src/main/kotlin/io/flutter/plugins/a_plugin/a.kt',
'android/src/main/kotlin/io/flutter/plugins/a_plugin/b.kt',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['java'] = <FakeProcessInfo>[
FakeProcessInfo(
Expand All @@ -513,7 +545,9 @@ void main() {
const List<String> files = <String>[
'android/src/main/kotlin/io/flutter/plugins/a_plugin/a.kt',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-kotlin']);
expect(processRunner.recordedCalls, orderedEquals(<ProcessCall>[]));
Expand All @@ -530,6 +564,7 @@ void main() {
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>[
'format',
Expand Down Expand Up @@ -567,11 +602,12 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin(
final RepositoryPackage plugin = createFakePlugin(
'a_plugin',
packagesDir,
extraFiles: files,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format', '--no-swift']);

Expand All @@ -583,7 +619,9 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
Expand All @@ -609,7 +647,9 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
Expand Down Expand Up @@ -643,7 +683,9 @@ void main() {
const List<String> files = <String>[
'macos/foo.swift',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['swift-format'] =
<FakeProcessInfo>[
Expand Down Expand Up @@ -694,6 +736,7 @@ void main() {
...javaFiles,
],
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format']);

Expand Down Expand Up @@ -732,6 +775,7 @@ void main() {
'example/macos/Flutter/GeneratedPluginRegistrant.swift',
],
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>[
'format',
Expand Down Expand Up @@ -773,7 +817,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

const String changedFilePath = 'packages/a_plugin/linux/foo_plugin.cc';
processRunner.mockProcessesForExecutable['git'] = <FakeProcessInfo>[
Expand Down Expand Up @@ -825,7 +871,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

processRunner.mockProcessesForExecutable['git'] = <FakeProcessInfo>[
FakeProcessInfo(MockProcess(exitCode: 1), <String>['ls-files'])
Expand All @@ -850,7 +898,9 @@ void main() {
'linux/foo_plugin.cc',
'macos/Classes/Foo.h',
];
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
final RepositoryPackage plugin =
createFakePlugin('a_plugin', packagesDir, extraFiles: files);
fakePubGet(plugin);

const String changedFilePath = 'packages/a_plugin/linux/foo_plugin.cc';
processRunner.mockProcessesForExecutable['git'] = <FakeProcessInfo>[
Expand Down Expand Up @@ -892,6 +942,7 @@ void main() {
packagesDir,
extraFiles: <String>[...batch1, extraFile],
);
fakePubGet(package);

await runCapturingPrint(runner, <String>['format']);

Expand Down Expand Up @@ -922,11 +973,12 @@ void main() {
// Make the file list one file longer than would fit in a Windows batch.
final List<String> batch = get99CharacterPathExtraFiles(batchSize + 1);

createFakePlugin(
final RepositoryPackage plugin = createFakePlugin(
pluginName,
packagesDir,
extraFiles: batch,
);
fakePubGet(plugin);

await runCapturingPrint(runner, <String>['format']);

Expand All @@ -947,6 +999,7 @@ void main() {
packagesDir,
extraFiles: <String>[...batch1, extraFile],
);
fakePubGet(package);

await runCapturingPrint(runner, <String>['format']);

Expand Down
1 change: 1 addition & 0 deletions script/tool/test/license_check_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ void main() {
'GeneratedPluginRegistrant.m',
'generated_plugin_registrant.cc',
'generated_plugin_registrant.cpp',
'web_plugin_registrant.dart',
// Ignored path suffixes.
'foo.g.dart',
'foo.mocks.dart',
Expand Down