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.

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

Conversation

stuartmorgan
Copy link
Contributor

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.

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +124 to +126
// 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()) {
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.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 11, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 11, 2024
Copy link
Contributor

auto-submit bot commented Nov 11, 2024

auto label is removed for flutter/packages/8052, due to - The status or check suite Mac_arm64 macos_platform_tests master - packages has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan
Copy link
Contributor Author

Since this only affects repo_checks and format, and those tasks/steps have completed and passed, I'm going to land this without waiting for (and de-flaking) the rest of the tests, to unblock the roller.

@stuartmorgan stuartmorgan merged commit df0f423 into flutter:main Nov 11, 2024
68 of 76 checks passed
@stuartmorgan stuartmorgan deleted the tool-pub-get-before-format branch November 11, 2024 19:55
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 13, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 13, 2024
flutter/packages@72356fd...26e123a

2024-11-13 [email protected] [camera_windows] Set device media type for video preview explicitly (flutter/packages#7447)
2024-11-13 [email protected] [go_router] Add support for relative routes (flutter/packages#6825)
2024-11-13 [email protected] [vector_graphics_compiler] fix a renamed method parameter lint (flutter/packages#8070)
2024-11-12 [email protected] [in_app_purchase] Add expiration date to Transaction (flutter/packages#8030)
2024-11-12 [email protected] [various] Clean up contributing guides (flutter/packages#8032)
2024-11-12 [email protected] [ci] Remove web renderer option from tools. (flutter/packages#8055)
2024-11-12 [email protected] [url_launcher] Update Pigeon version for Linux (flutter/packages#8065)
2024-11-12 [email protected] [go_router] Add support for preloading branches of StatefulShellRoute (revised solution) (flutter/packages#6467)
2024-11-12 [email protected] [pigeon] Make Linux type declarations public (flutter/packages#8040)
2024-11-11 [email protected] Roll Flutter from 73546b3 to c8510f2 (30 revisions) (flutter/packages#8042)
2024-11-11 [email protected] Use dependabot multi-directory configuration for Android package updates (flutter/packages#8048)
2024-11-11 [email protected] [tools] Run `pub get` before `format` (flutter/packages#8052)
2024-11-11 [email protected] [file_selector] Fix Linux cancel regression (flutter/packages#8051)
2024-11-09 [email protected] [shared_preferences] Fix confusing language in README (flutter/packages#8049)
2024-11-08 [email protected] Use dependabot multi-directory configuration for Android example gradle updates (flutter/packages#8036)
2024-11-08 [email protected] [animations] Remove `.flutter-plugins` reference from example app (flutter/packages#8002)
2024-11-08 [email protected] Group dependabot github-action update PRs, delete dead docker updates (flutter/packages#8044)
2024-11-08 [email protected] [vector_graphics_compiler] fix-null-exception (flutter/packages#8006)
2024-11-08 [email protected] [tools] Format Dart per-package (flutter/packages#8043)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants