Skip to content

Conversation

@andrewkolos
Copy link
Contributor

cherry-picks changes from #139614 onto the beta channel

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.

…run (flutter#139614)

## Summary
Fixes flutter#139180, where `flutter create` could crash if the `java` binary the tool found cannot be run.

## Context

At startup, the tool searches for a Java installation[^1]. Unless the located installation is from [an Android Studio installation](https://github.com/flutter/flutter/blob/e1967ecabf014bf93d1731fde6a6547b06ca9c33/packages/flutter_tools/lib/src/android/android_studio.dart#L163), the tool does not verify that the binary is runnable. For more, see flutter#139613, which tracks this inconsistency in behavior. 

This means that in the scenario where

1) the user does not have Android Studio installed or the java binary found within cannot be run **and**
2) the user has a) `flutter config --jdk-dir` set, b) `JAVA_HOME` set in their environment, **or** c) `java` on their system path **and**
3) the java binary we think we found during cannot be run (or `java --version` fails), **then**

the user running `flutter create` with Android enabled will hit a tool crash.

## Change

`Java.version` should return null if version checking fails for any reason. [This is documented behavior](https://github.com/flutter/flutter/blob/48f57621ade657b0c20ba53d513de4c3cd563abd/packages/flutter_tools/lib/src/android/java.dart#L136). Therefore, we'll update the implementation to first verify that the binary is runnable. If it isn't, it will return `null`.

[^1]: We find java by calling the static `Java.find`, see: https://github.com/flutter/flutter/blob/48187028c11ca8ca10e0179705d25553e1fe2c14/packages/flutter_tools/lib/src/context_runner.dart#L271
[^2]: This PR doesn't change this, as this would be too dangerous to cherry-pick into stable.
@andrewkolos andrewkolos added the tool Affects the "flutter" command-line tool. See also t: labels. label Dec 8, 2023
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 12, 2023
@auto-submit auto-submit bot merged commit f5a2ca0 into flutter:flutter-3.18-candidate.0 Dec 12, 2023
@andrewkolos andrewkolos deleted the cp-beta-flutter-create-java-crash branch April 29, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants