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
Bugfix/java checks #1228
Bugfix/java checks #1228
Conversation
3310dc7
to
e70e078
Compare
Codecov Report
@@ Coverage Diff @@
## master #1228 +/- ##
==========================================
- Coverage 72.02% 72.00% -0.02%
==========================================
Files 21 21
Lines 1698 1697 -1
==========================================
- Hits 1223 1222 -1
Misses 475 475
Continue to review full report at Codecov.
|
e70e078
to
fbd0865
Compare
3f4ad25
to
6daa175
Compare
6daa175
to
98cd058
Compare
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.
Great!
* fix: Java version parsing if java executable prints out additional information with --version * fix: Ensure JAVA_HOME path comes first in the PATH environment * refactor: Removed redundent code in favour of keeping a change introduced from another PR
Platforms affected
Android
Motivation and Context
Fixes #1221
Fixes #1203(Addressed by #1220)Description
This addresses two flaws in Java checking.
If some Java options are present,Addressed by fix(requirements check): use regex to get java version from javac output #1220javac -version
will print out additional lines of information. A regex is used to find thejava(c) x.x.x
line for proper version checking.JAVA_HOME
is set, we would only set the path ifjavac
wasn't findable, and when we do, we set the path at the end of thePATH
chain. To be smarter, ifJAVA_HOME
is set, we always want to set theJAVA_HOME
bin path to the front of thePATH
chain so that we can guarantee the proper java executable to be used.Testing
Added new test for the
javac -version
issue. All existing tests passes. Manual testing for theJAVA_HOME
issue.Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)