It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

Ah, I thought format didn't have tests, I now see that it does, will test, sec.

OK, I believe this should be ready for review.

Fixed to remove formatting changes. PTAL. Thanks!

Looks like linux is failing. I cant tell why maybe we do assume that java needs to be on the path for CI?

[2024-08-08 12:25:00.567034] ERROR: ERROR: Cannot run Java, skipping Java file formatting!

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8740146003425990369/+/u/test:_test:_Check_formatting__2_/stdout

I can look more shortly. I suspect the path may not exist for linux and it may need to use java. I can add a bit more logging and some minimal fallback behavior.

I had lost the "log the path" bits in my "remove formatting changes" update. Running the bots again should give us better data.

Turns out the paths inside openjdk differ per platform (based on searching for other uses of such in the repo). 🤦

OK, so this is the problem on linux:

ERROR: Cannot find Java (/b/s/w/ir/cache/builder/src/flutter/third_party/java/openjdk/bin/java). Did you run gclient sync? Skipping Java format check.

I might have gotten the location wrong, can look more later.

Hmm, looks like it's not always downloaded:

  'src/flutter/third_party/java/openjdk': {
     'packages': [
       {
        'package': 'flutter/java/openjdk/${{platform}}',
        'version': 'version:17'
       }
     ],
     'condition': 'download_android_deps',
     'dep_type': 'cipd',
   },

  # Checkout Android dependencies only on platforms where we build for Android targets.
  'download_android_deps': 'host_os == "mac" or (host_os == "linux" and host_cpu == "x64")',

Looks like this is ready (as far as the bots are concerned), but I'm in no rush to land. Looks like you CC'd matan, so I'll happily wait for additional reviews (or feel free to click squash and merge once reviewed).

Looks like this is ready (as far as the bots are concerned), but I'm in no rush to land. Looks like you CC'd matan, so I'll happily wait for additional reviews (or feel free to click squash and merge once reviewed).

I added matan because I thought you would need 2 contributors to approve but you are a contributor. Feel free to land and consider matan fyi.

I'll wait for the bots to green again (after I removed the prints) and then press squash. Thanks!

What does DBC mean? Would you like action from me? Thanks!

Oh, sorry. It means drive-by comment. As a follow-up, if you wouldn't mind doing the following, it would be helpful:

1. Add a new gclient var next to download_android_deps called something like download_jdk and default it to true.
2. In the json files under ci/builders, wherever you see download_android_deps set to false, also set download_jdk to false.

This will save on repo checkout time on the CI bots. Thanks!

This is still in my inbox (I've not forgotten), but it may be a while before I code again.

This seems to break engine build on Linux/Arm64

flutter/flutter#157576