Skip to content

Commit

Permalink
[tools] Run pub get before format (#8052)
Browse files Browse the repository at this point in the history 
The new Dart formatter needs to know the Dart language version of the
code it is formatting, and it reads that from a file in `.dart_tool`,
not `pubspec.yaml` directly. To avoid it failing to determine the
version and assuming the latest (which will almost always be wrong in
this repo):
- Adds a step to the `format` repo command to ensure that `pub get`
appears to have been run, and runs it if not, and
- To avoid `pub get` running in `format` in CI, adds a deps-fetching
step to the `repo_checks` task, as we have in other tasks that need to
`pub get`.

This should unblock the roll.
  • Loading branch information
@stuartmorgan
stuartmorgan authored Nov 11, 2024
1 parent d681e4e commit df0f423
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 18 deletions.
7 changes: 7 additions & 0 deletions .ci/targets/repo_checks.yaml
View file
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
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()) {
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
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
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
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

0 comments on commit df0f423

Please sign in to comment.