-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[tools] use Java class for all java-searching behavior #127354
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be outside the scope of the pr but I think _java should provide a Version object so that this type of conversion does not happen in multiple places slightly differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to use the Version object here by using the Version.withText constructor to preserve the full version string (e.g. 'openjdk 19.0.2 2023-01-17').
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment still true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment appears to be true given https://github.com/flutter/flutter/blob/e8f5a51f79fc5dd2f8de38b4d9f504d09dca8197/packages/flutter_tools/lib/src/flutter_cache.dart#L411-L413, though I am not sure if it should. I've created #127848 to track as a follow-up and added a TODO comment linking to it
…lutter#127975) flutter/engine@ee53c04...8e67cc3 2023-05-31 [email protected] Roll Skia from f475d4f5e080 to cb883f64681b (6 revisions) (flutter/engine#42452) 2023-05-31 [email protected] Fix bugprone-unchecked-optional-access errors in image_generator_apng (flutter/engine#42450) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…lutter#127979) flutter/engine@8e67cc3...b9860dc 2023-05-31 [email protected] Replace use of Skia's private GrRectanizer with a copy of the equivalent code (flutter/engine#42430) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes #124252, finishing work on the umbrella tracking issue, #126126.
Essentially, after this PR, no (non-test) code should be be referencing/invoking the java home or binary paths.
Pre-launch Checklist
///).