Skip to content

Conversation

@andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented May 22, 2023

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

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@andrewkolos andrewkolos added platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels. t: gradle "flutter build" and "flutter run" on Android labels May 22, 2023
@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems c: contributor-productivity Team-specific productivity, code health, technical debt. labels May 22, 2023
@andrewkolos andrewkolos marked this pull request as ready for review May 23, 2023 19:45
Copy link
Contributor

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.

Copy link
Contributor Author

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').

Copy link
Contributor

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?

Copy link
Contributor Author

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

@github-actions github-actions bot removed c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels May 30, 2023
andrewkolos and others added 15 commits May 31, 2023 14:24
@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label May 31, 2023
@github-actions github-actions bot removed the engine flutter/engine related. See also e: labels. label May 31, 2023
@andrewkolos andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 1, 2023
@auto-submit auto-submit bot merged commit 06e2b18 into flutter:master Jun 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2023
@andrewkolos andrewkolos deleted the java-finding branch June 15, 2023 01:02
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App platform-android Android applications specifically t: gradle "flutter build" and "flutter run" on Android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The logic used to find a JDK in various tool operations is unclear and confusing

3 participants