-
Notifications
You must be signed in to change notification settings - Fork 28k
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
Add Java-Gradle-AGP validation to flutter analyze #123916
Changes from 59 commits
a07f7ee
7b4f418
2b95eb5
477734e
5a8005a
e05aa96
53e89a0
af3c647
7e643a6
6b4ed1b
96cae3d
5e08847
b263dfc
1a5e9e1
4749a95
fff074e
2df3691
3815b0c
21b8887
87ea610
7bc2f4b
565dd58
51903c4
a84e4b6
c2df14e
453889c
07213fb
860b861
8d379d6
54d1b99
6ce48fe
2641a53
ad94e9b
3a8dc54
9295069
f9d61ba
cd20ff2
0e45f6f
5bc570c
b3823a5
0fb85f4
0c66529
2484666
dab39a0
42092ee
77db98e
69f7fb9
221f2e6
ab0e2e6
6d0268d
ee8ccb6
5b10e19
09adddc
09c2c97
fa7da03
7f50a6f
76aa064
64c7b8e
9f03e54
033cd4b
a749ad8
7ba52c0
ae705bf
6e69b85
da7b866
e0572ac
3165cbf
deb320f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,6 +2,8 @@ | |||||
// Use of this source code is governed by a BSD-style license that can be | ||||||
// found in the LICENSE file. | ||||||
|
||||||
import 'package:meta/meta.dart'; | ||||||
|
||||||
import '../base/common.dart'; | ||||||
import '../base/file_system.dart'; | ||||||
import '../base/os.dart'; | ||||||
|
@@ -409,6 +411,57 @@ class AndroidSdk { | |||||
return null; | ||||||
} | ||||||
|
||||||
// Returns the version of java in the format \d(.\d)+(.\d)+ | ||||||
// Returns null if version not found. | ||||||
String? getJavaVersion({ | ||||||
required AndroidStudio? androidStudio, | ||||||
required FileSystem fileSystem, | ||||||
required OperatingSystemUtils operatingSystemUtils, | ||||||
required Platform platform, | ||||||
required ProcessUtils processUtils, | ||||||
}) { | ||||||
final String? javaBinary = findJavaBinary( | ||||||
androidStudio: androidStudio, | ||||||
fileSystem: fileSystem, | ||||||
operatingSystemUtils: operatingSystemUtils, | ||||||
platform: platform, | ||||||
); | ||||||
if (javaBinary == null) { | ||||||
globals.printTrace('Could not find java binary to get version.'); | ||||||
return null; | ||||||
} | ||||||
final RunResult result = processUtils.runSync( | ||||||
<String>[javaBinary, '--version'], | ||||||
environment: sdkManagerEnv, | ||||||
); | ||||||
if (result.exitCode != 0) { | ||||||
globals.printTrace( | ||||||
'java --version failed: exitCode: ${result.exitCode} stdout: ${result.stdout} stderr: ${result.stderr}'); | ||||||
return null; | ||||||
} | ||||||
return parseJavaVersion(result.stdout); | ||||||
} | ||||||
|
||||||
// Extracts jdk version from the output of java --version. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. Also, I prefer "JDK" to "jdk" :P There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed but I doubt I will get all the places updated. |
||||||
@visibleForTesting | ||||||
static String? parseJavaVersion(String rawVersionOutput) { | ||||||
// The contents that matter come in the format '11.0.18' or '1.8.0_202'. | ||||||
final RegExp jdkVersionRegex = RegExp(r'\d+\.\d+(\.\d+(?:_\d+)?)?'); | ||||||
final Iterable<RegExpMatch> matches = | ||||||
jdkVersionRegex.allMatches(rawVersionOutput); | ||||||
if (matches.isEmpty) { | ||||||
christopherfujino marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
globals.logger.printWarning(_formatJavaVersionWarning(rawVersionOutput)); | ||||||
return null; | ||||||
} | ||||||
final String? versionString = matches.first.group(0); | ||||||
if (versionString == null || versionString.split('_').isEmpty) { | ||||||
globals.logger.printWarning(_formatJavaVersionWarning(rawVersionOutput)); | ||||||
return null; | ||||||
} | ||||||
// Trim away _d+ from versions 1.8 and below. | ||||||
return versionString.split('_').first; | ||||||
} | ||||||
|
||||||
/// First try Java bundled with Android Studio, then sniff JAVA_HOME, then fallback to PATH. | ||||||
static String? findJavaBinary({ | ||||||
required AndroidStudio? androidStudio, | ||||||
|
@@ -417,36 +470,64 @@ class AndroidSdk { | |||||
required Platform platform, | ||||||
}) { | ||||||
if (androidStudio?.javaPath != null) { | ||||||
globals.printTrace("Using Android Studio's java."); | ||||||
return fileSystem.path.join(androidStudio!.javaPath!, 'bin', 'java'); | ||||||
} | ||||||
|
||||||
final String? javaHomeEnv = platform.environment[_javaHomeEnvironmentVariable]; | ||||||
final String? javaHomeEnv = | ||||||
platform.environment[_javaHomeEnvironmentVariable]; | ||||||
if (javaHomeEnv != null) { | ||||||
// Trust JAVA_HOME. | ||||||
globals.printTrace('Using JAVA_HOME.'); | ||||||
return fileSystem.path.join(javaHomeEnv, 'bin', 'java'); | ||||||
} | ||||||
|
||||||
// MacOS specific logic to avoid popping up a dialog window. | ||||||
// See: http://stackoverflow.com/questions/14292698/how-do-i-check-if-the-java-jdk-is-installed-on-mac. | ||||||
if (platform.isMacOS) { | ||||||
try { | ||||||
final String javaHomeOutput = globals.processUtils.runSync( | ||||||
<String>['/usr/libexec/java_home', '-v', '1.8'], | ||||||
throwOnError: true, | ||||||
hideStdout: true, | ||||||
).stdout.trim(); | ||||||
// -v Filter versions (as if JAVA_VERSION had been set in the environment). | ||||||
// It is unlikley that filtering to java version 1.8 is the right | ||||||
// decision here. That said, trying this on a mac shows the same jdk | ||||||
// path no matter what input is passed. | ||||||
final String javaHomeOutput = globals.processUtils | ||||||
.runSync( | ||||||
<String>['/usr/libexec/java_home', '-v', '1.8'], | ||||||
throwOnError: true, | ||||||
hideStdout: true, | ||||||
) | ||||||
.stdout | ||||||
.trim(); | ||||||
if (javaHomeOutput.isNotEmpty) { | ||||||
final String javaHome = javaHomeOutput.split('\n').last.trim(); | ||||||
globals.printTrace('Using mac JAVA_HOME.'); | ||||||
return fileSystem.path.join(javaHome, 'bin', 'java'); | ||||||
} | ||||||
} on Exception { /* ignore */ } | ||||||
} on Exception {/* ignore */} | ||||||
} | ||||||
|
||||||
// Fallback to PATH based lookup. | ||||||
return operatingSystemUtils.which(_javaExecutable)?.path; | ||||||
final String? pathJava = operatingSystemUtils.which(_javaExecutable)?.path; | ||||||
if (pathJava != null) { | ||||||
globals.printTrace('Using path java.'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tiny nit ("path java" took me a few seconds to parse&understand than "java from PATH")
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good readability suggestion. Fixed. |
||||||
} else { | ||||||
globals.printTrace('Could not find java path.'); | ||||||
} | ||||||
return pathJava; | ||||||
} | ||||||
|
||||||
// Returns a user visible String that says the tool failed to parse | ||||||
// the version of java along with the output. | ||||||
static String _formatJavaVersionWarning(String javaVersionRaw) { | ||||||
return 'Could not parse java version from: \n' | ||||||
'$javaVersionRaw \n' | ||||||
'If there is a version please look for an existing bug ' | ||||||
'https://github.com/flutter/flutter/issues/' | ||||||
' and if one does not exist file a new issue.'; | ||||||
} | ||||||
|
||||||
Map<String, String>? _sdkManagerEnv; | ||||||
|
||||||
/// Returns an environment with the Java folder added to PATH for use in calling | ||||||
/// Java-based Android SDK commands such as sdkmanager and avdmanager. | ||||||
Map<String, String> get sdkManagerEnv { | ||||||
|
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.
Shouldn't these be
///
?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.
good catch fixed.