firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

doctor: make JDK validation message more descriptive (#157280)

Browse Source 
This PR attempts to improve clarity of androids section of `flutter
doctor -v` output by providing explicit information about which JDK is
being used and how to configure a different one if needed.

### Before

```console
• Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java
```

### After

1. When JDK is from Android Studio:

```console
    • Java binary at: /Users/users/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java
      This is the JDK bundled with latest Android Studio installation
      To manually set a custom JDK path, use: `flutter config --jdk-dir="path/to/jdk"`
```

2. When JDK is from JAVA_HOME env variable:

```console
    • Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java
      This JDK is specified by JAVA_HOME environment variable
      To manually set a custom JDK path, use: `flutter config --jdk-dir="path/to/jdk"`
```

3. When path to JDK is set in flutter config:

```console
    • Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java
      This JDK was found in system PATH
      To change current JDK, run: `flutter config --jdk-dir="path/to/jdk"`
```
4. When java binary is found in PATH (as fallback)

```console
    • Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java
      This JDK is specified in Flutter configuration
      To manually set a custom JDK path, use: `flutter config --jdk-dir="path/to/jdk"`
```

### Motivation

I think it's described in
https://github.com/flutter/flutter/issues/153156#issuecomment-2336814991.

TLDR; many developers struggle with Java-related issues and more verbose
doctor's output will (presumably) improve DX in that part.


fixes #153156


## Pre-launch Checklist

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

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
This commit is contained in:
Mikhail Novoseltsev 2024-11-23 07:27:18 +07:00 committed by GitHub
parent 5efd759085
commit 4b46b80661
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 218 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
packages/flutter_tools/lib/src/android/android_workflow.dart
View File

@ -105,7 +105,12 @@ class AndroidValidator extends DoctorValidator {
messages.add(ValidationMessage.error(_userMessages.androidMissingJdk));
return false;
}
messages.add(ValidationMessage(_userMessages.androidJdkLocation(_java!.binaryPath)));
messages.add(ValidationMessage(
_androidJdkLocationMessage(
_java!.binaryPath,
_java.javaSource,
),
));
if (!_java.canRun()) {
messages.add(ValidationMessage.error(_userMessages.androidCantRunJavaBinary(_java.binaryPath)));
return false;
@ -454,3 +459,26 @@ class AndroidLicenseValidator extends DoctorValidator {
);
}
}
String _androidJdkLocationMessage(String location, JavaSource source) {
final String setWithConfigBreadcrumb = switch (source) {
JavaSource.androidStudio || JavaSource.path || JavaSource.javaHome =>
'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`.',
JavaSource.flutterConfig =>
'To change the current JDK, run: `flutter config --jdk-dir="path/to/jdk"`.'
};
final String sourceMessagePart = switch (source) {
JavaSource.androidStudio =>
'This is the JDK bundled with the latest Android Studio installation on this machine.',
JavaSource.javaHome =>
'This JDK is specified by the JAVA_HOME environment variable.',
JavaSource.path =>
'This JDK was found in the system PATH.',
JavaSource.flutterConfig =>
'This JDK is specified in your Flutter configuration.',
};
return 'Java binary at: $location\n'
'$sourceMessagePart\n'
'$setWithConfigBreadcrumb';
}

39
packages/flutter_tools/lib/src/android/java.dart
View File

@ -15,11 +15,25 @@ import 'android_studio.dart';
const String _javaExecutable = 'java';
enum JavaSource {
/// JDK bundled with latest Android Studio installation.
androidStudio,
/// JDK specified by the system's JAVA_HOME environment variable.
javaHome,
/// JDK available through the system's PATH environment variable.
path,
/// JDK specified in Flutter's configuration.
flutterConfig,
}
typedef _JavaHomePathWithSource = ({String path, JavaSource source});
/// Represents an installation of Java.
class Java {
Java({
required this.javaHome,
required this.binaryPath,
required this.javaSource,
required Logger logger,
required FileSystem fileSystem,
required OperatingSystemUtils os,
@ -65,7 +79,7 @@ class Java {
platform: platform,
processManager: processManager
);
final String? home = _findJavaHome(
final _JavaHomePathWithSource? home = _findJavaHome(
config: config,
logger: logger,
androidStudio: androidStudio,
@ -73,7 +87,7 @@ class Java {
);
final String? binary = _findJavaBinary(
logger: logger,
javaHome: home,
javaHome: home?.path,
fileSystem: fileSystem,
operatingSystemUtils: os,
platform: platform
@ -83,9 +97,14 @@ class Java {
return null;
}
// If javaHome == null and binary is not null, it means that
// binary obtained from PATH as fallback.
final JavaSource javaSource = home?.source ?? JavaSource.path;
return Java(
javaHome: home,
javaHome: home?.path,
binaryPath: binary,
javaSource: javaSource,
logger: logger,
fileSystem: fileSystem,
os: os,
@ -110,6 +129,12 @@ class Java {
/// to this class instead.
final String binaryPath;
/// Indicates the source from where the Java runtime was located.
///
/// This information is useful for debugging and logging purposes to track
/// which source was used to locate the Java runtime environment.
final JavaSource javaSource;
final Logger _logger;
final FileSystem _fileSystem;
final OperatingSystemUtils _os;
@ -192,7 +217,7 @@ class Java {
}
}
String? _findJavaHome({
_JavaHomePathWithSource? _findJavaHome({
required Config config,
required Logger logger,
required AndroidStudio? androidStudio,
@ -200,17 +225,17 @@ String? _findJavaHome({
}) {
final Object? configured = config.getValue('jdk-dir');
if (configured != null) {
return configured as String;
return (path: configured as String, source: JavaSource.flutterConfig);
}
final String? androidStudioJavaPath = androidStudio?.javaPath;
if (androidStudioJavaPath != null) {
return androidStudioJavaPath;
return (path: androidStudioJavaPath, source: JavaSource.androidStudio);
}
final String? javaHomeEnv = platform.environment[Java.javaHomeEnvironmentVariable];
if (javaHomeEnv != null) {
return javaHomeEnv;
return (path: javaHomeEnv, source: JavaSource.javaHome);
}
return null;
}

1
packages/flutter_tools/lib/src/base/user_messages.dart
View File

@ -115,7 +115,6 @@ class UserMessages {
'No Java Development Kit (JDK) found; You must have the environment '
'variable JAVA_HOME set and the java binary in your PATH. '
'You can download the JDK from https://www.oracle.com/technetwork/java/javase/downloads/.';
String androidJdkLocation(String location) => 'Java binary at: $location';
String get androidLicensesAll => 'All Android licenses accepted.';
String get androidLicensesSome => 'Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses';
String get androidLicensesNone => 'Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses';

140
packages/flutter_tools/test/general.shard/android/android_workflow_test.dart
View File

@ -637,6 +637,146 @@ Android sdkmanager tool was found, but failed to run
expect(processManager, hasNoRemainingExpectations);
expect(stdio.stderr.getAndClear(), contains('UnsupportedClassVersionError'));
});
testWithoutContext('Mentions that JDK is provided by latest Android Studio Installation', () async {
// Mock a pass through scenario to reach _checkJavaVersion()
sdk
..licensesAvailable = true
..platformToolsAvailable = true
..cmdlineToolsAvailable = true
..directory = fileSystem.directory('/foo/bar')
..sdkManagerPath = '/foo/bar/sdkmanager';
final ValidationResult validationResult = await AndroidValidator(
java: FakeJava(),
androidSdk: sdk,
logger: logger,
platform: FakePlatform(),
userMessages: UserMessages()
).validate();
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'This is the JDK bundled with the latest Android Studio installation on this machine.'
)
),
true,
);
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`.'
)
),
true,
);
});
testWithoutContext("Mentions that JDK is provided by user's JAVA_HOME environment variable", () async {
// Mock a pass through scenario to reach _checkJavaVersion()
sdk
..licensesAvailable = true
..platformToolsAvailable = true
..cmdlineToolsAvailable = true
..directory = fileSystem.directory('/foo/bar')
..sdkManagerPath = '/foo/bar/sdkmanager';
final ValidationResult validationResult = await AndroidValidator(
java: FakeJava(javaSource: JavaSource.javaHome),
androidSdk: sdk,
logger: logger,
platform: FakePlatform(),
userMessages: UserMessages()
).validate();
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'This JDK is specified by the JAVA_HOME environment variable.'
)
),
true,
);
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`'
)
),
true,
);
});
testWithoutContext('Mentions that path to Java binary is obtained from PATH', () async {
// Mock a pass through scenario to reach _checkJavaVersion()
sdk
..licensesAvailable = true
..platformToolsAvailable = true
..cmdlineToolsAvailable = true
..directory = fileSystem.directory('/foo/bar')
..sdkManagerPath = '/foo/bar/sdkmanager';
final ValidationResult validationResult = await AndroidValidator(
java: FakeJava(javaSource: JavaSource.path),
androidSdk: sdk,
logger: logger,
platform: FakePlatform(),
userMessages: UserMessages()
).validate();
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'This JDK was found in the system PATH.'
)
),
true,
);
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`.'
)
),
true,
);
});
testWithoutContext('Mentions that JDK is provided by Flutter config', () async {
// Mock a pass through scenario to reach _checkJavaVersion()
sdk
..licensesAvailable = true
..platformToolsAvailable = true
..cmdlineToolsAvailable = true
..directory = fileSystem.directory('/foo/bar')
..sdkManagerPath = '/foo/bar/sdkmanager';
final ValidationResult validationResult = await AndroidValidator(
java: FakeJava(javaSource: JavaSource.flutterConfig),
androidSdk: sdk,
logger: logger,
platform: FakePlatform(),
userMessages: UserMessages()
).validate();
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'This JDK is specified in your Flutter configuration.'
)
),
true,
);
expect(
validationResult.messages.any(
(ValidationMessage message) => message.message.contains(
'To change the current JDK, run: `flutter config --jdk-dir="path/to/jdk"`.'
)
),
true,
);
});
}
class FakeAndroidSdk extends Fake implements AndroidSdk {

13
packages/flutter_tools/test/general.shard/android/java_test.dart
View File

@ -44,6 +44,7 @@ void main() {
final AndroidStudio androidStudio = _FakeAndroidStudioWithJdk();
final String androidStudioBundledJdkHome = androidStudio.javaPath!;
final String expectedJavaBinaryPath = fs.path.join(androidStudioBundledJdkHome, 'bin', 'java');
const JavaSource expectedJavaHomeSource = JavaSource.androidStudio;
processManager.addCommand(FakeCommand(
command: <String>[
@ -70,12 +71,14 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
expect(java.version!.toString(), 'OpenJDK Runtime Environment Zulu19.32+15-CA (build 19.0.2+7)');
expect(java.version, equals(Version(19, 0, 2)));
expect(java.javaSource, expectedJavaHomeSource);
});
testWithoutContext('finds JAVA_HOME if it is set and the JDK bundled with Android Studio could not be found', () {
final AndroidStudio androidStudio = _FakeAndroidStudioWithoutJdk();
const String javaHome = '/java/home';
final String expectedJavaBinaryPath = fs.path.join(javaHome, 'bin', 'java');
const JavaSource expectedJavaHomeSource = JavaSource.javaHome;
final Java java = Java.find(
config: config,
@ -90,11 +93,14 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
expect(java.javaHome, javaHome);
expect(java.binaryPath, expectedJavaBinaryPath);
expect(java.javaSource, expectedJavaHomeSource);
});
testWithoutContext('returns the java binary found on PATH if no other can be found', () {
final AndroidStudio androidStudio = _FakeAndroidStudioWithoutJdk();
final OperatingSystemUtils os = _FakeOperatingSystemUtilsWithJava(fileSystem);
const JavaSource expectedJavaHomeSource = JavaSource.path;
processManager.addCommand(
const FakeCommand(
@ -114,6 +120,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
expect(java.javaHome, isNull);
expect(java.binaryPath, os.which('java')!.path);
expect(java.javaSource, expectedJavaHomeSource);
});
testWithoutContext('returns null if no java could be found', () {
@ -138,6 +145,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
testWithoutContext('finds and prefers JDK found at config item "jdk-dir" if it is set', () {
const String configuredJdkPath = '/jdk';
config.setValue('jdk-dir', configuredJdkPath);
JavaSource expectedJavaHomeSource = JavaSource.flutterConfig;
processManager.addCommand(
const FakeCommand(
@ -164,9 +172,12 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
expect(java, isNotNull);
expect(java!.javaHome, configuredJdkPath);
expect(java.binaryPath, fs.path.join(configuredJdkPath, 'bin', 'java'));
expect(java.javaSource, expectedJavaHomeSource);
config.removeValue('jdk-dir');
expectedJavaHomeSource = JavaSource.androidStudio;
java = Java.find(
config: config,
androidStudio: androidStudio,
@ -180,6 +191,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
assert(androidStudio.javaPath != configuredJdkPath);
expect(java!.javaHome, androidStudio.javaPath);
expect(java.binaryPath, fs.path.join(androidStudio.javaPath!, 'bin', 'java'));
expect(java.javaSource, expectedJavaHomeSource);
});
});
@ -196,6 +208,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
processManager: processManager,
binaryPath: 'javaHome/bin/java',
javaHome: 'javaHome',
javaSource: JavaSource.javaHome,
);
});

4
packages/flutter_tools/test/src/fakes.dart
View File

@ -670,6 +670,7 @@ class FakeAndroidStudio extends Fake implements AndroidStudio {
class FakeJava extends Fake implements Java {
FakeJava({
this.javaHome = '/android-studio/jbr',
this.javaSource = JavaSource.androidStudio,
String binary = '/android-studio/jbr/bin/java',
Version? version,
bool canRun = true,
@ -687,6 +688,9 @@ class FakeJava extends Fake implements Java {
@override
String binaryPath;
@override
JavaSource javaSource;
final Map<String, String> _environment;
final bool _canRun;