redhat-developer-tooling / developer-platform-install

Single Installer for all Red Hat Development Tools and more. Download it form Red Hat Developers Portal web site - http://developers.redhat.com/products/devsuite/overview/.
Apache License 2.0
25 stars 29 forks source link

Select Open JDK 8 to install if required and Java 9+ detected #1386

Closed dgolovin closed 6 years ago

dgolovin commented 6 years ago

requires https://github.com/redhat-developer-tooling/developer-platform-install/pull/1384 and https://github.com/redhat-developer-tooling/developer-platform-install/pull/1385 to work.

codecov-io commented 6 years ago

Codecov Report

Merging #1386 into master will decrease coverage by 0.5%. The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1386      +/-   ##
==========================================
- Coverage    92.7%   92.19%   -0.51%     
==========================================
  Files          47       47              
  Lines        4891     4970      +79     
  Branches      549      862     +313     
==========================================
+ Hits         4534     4582      +48     
- Misses        357      359       +2     
- Partials        0       29      +29
Impacted Files Coverage Δ
browser/pages/selection/controller.js 85.39% <0%> (-0.98%) :arrow_down:
browser/model/jdk-install.js 94.65% <100%> (+0.11%) :arrow_up:
browser/model/devstudio-autoinstall.js 97.29% <0%> (-2.71%) :arrow_down:
browser/pages/install/controller.js 91.86% <0%> (-1.75%) :arrow_down:
browser/model/helpers/downloader.js 95.53% <0%> (-1.68%) :arrow_down:
browser/model/che.js 95% <0%> (-1.62%) :arrow_down:
browser/services/platform.js 98.14% <0%> (-1.48%) :arrow_down:
browser/pages/account/controller.js 98.61% <0%> (-1.39%) :arrow_down:
browser/model/cdk.js 97.6% <0%> (-1.16%) :arrow_down:
browser/model/devstudio-p2-zip.js 64.4% <0%> (-1.12%) :arrow_down:
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a86d098...588f408. Read the comment docs.

jrichter1 commented 6 years ago

This actually lets you attempt to re-install openjdk msi. Because the msi puts java on the end of the path, the pre-existing jdk9+ will still take precedence when detected. So when you install the msi, the next run of the installer lets you do it again, which does virtually nothing (only one installation may exist). In fact, it forces you to "install" jdk again if you select something that depends on it.

But it does still use the original openjdk location when installing dependent items into a new folder along with openjdk, so it actually works.

dgolovin commented 6 years ago

@jrichter1 that is all true, because devsuite installer just takes first element detected on path and goes with it. This is the less intrusive fix to do right now. IMO we should merge it.

jrichter1 commented 6 years ago

I guess that's the price to pay for installing java 9. I'm not going to stall this.