feat: Enabled the usage of existing JDK installations #1500

quintesse · 2022-10-31T16:53:18Z

Right now we have support for using (not managing!) SDKMAN and Scoop installations.

maxandersen · 2022-10-31T21:55:39Z

hmm - how is that supposed to work?
I'll some convincing to add this :)

How does version comparison work? Which variation of sdk and scoops installs do we look at/consider?

quintesse · 2022-10-31T23:59:37Z

It's just "in-order". JBang's JDKs come first. If a requested version is not available it will try the next provider, eg SDKMAN. If it exists it uses it, if not it will fall back to installing the JDK using JBang, as before. And there's no version comparison, a provider either has a version X (major, eg 10, 11, 14, etc) or it doesn't.

codecov · 2022-11-01T00:14:35Z

Codecov Report

Base: 0.00% // Head: 0.00% // No change to project coverage 👍

Coverage data is based on head (ab27a80) compared to base (ce15aae).
Patch coverage: 0.00% of modified lines in pull request are covered.

❗ Current head ab27a80 differs from pull request most recent head d845114. Consider uploading reports for the commit d845114 to get more accurate results

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #1500    +/-   ##
======================================
  Coverage   0.00%   0.00%            
======================================
  Files        110     121    +11     
  Lines       6984    7380   +396     
  Branches    1141    1199    +58     
======================================
- Misses      6984    7380   +396

Flag	Coverage Δ
Linux	`0.00% <0.00%> (ø)`
Windows	`0.00% <0.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/main/java/dev/jbang/Cache.java	`0.00% <0.00%> (ø)`
src/main/java/dev/jbang/Settings.java	`0.00% <0.00%> (ø)`
src/main/java/dev/jbang/catalog/Alias.java	`0.00% <0.00%> (ø)`
src/main/java/dev/jbang/catalog/Catalog.java	`0.00% <ø> (ø)`
src/main/java/dev/jbang/catalog/CatalogUtil.java	`0.00% <ø> (ø)`
src/main/java/dev/jbang/cli/Alias.java	`0.00% <ø> (ø)`
src/main/java/dev/jbang/cli/App.java	`0.00% <0.00%> (ø)`
src/main/java/dev/jbang/cli/BaseBuildCommand.java	`0.00% <0.00%> (ø)`
src/main/java/dev/jbang/cli/Build.java	`0.00% <0.00%> (ø)`
src/main/java/dev/jbang/cli/BuildMixin.java	`0.00% <ø> (ø)`
... and 27 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

quintesse · 2022-11-15T15:37:55Z

Ok @maxandersen , this now has a jdkproviders.enabled config option that you can set to true to tell JBang to use sdkman/scoop-installed JDKs. By default this is set to false. It would still be nice to see if we can add support for "alternatives" as well, although it might actually be easier to just look at a specific set of standard folders where Linux/Mac OSses tend to install their JDKs.

Edit: the config option explanation isn't up-to-date anymore. See below.

quintesse · 2022-12-13T01:48:01Z

Ok, all tests pass again. This should finally be ready for merging.

quintesse · 2022-12-13T01:52:18Z

The config option has now be turned into a hidden flag, --jdk-providers, on run and build that takes a comma-separated list of provider names (order matters). The currently supported names are: javahome, path, jbang, sdkman and scoop. By default the value for this flag is "javahome,path,jbang". You can of course still set the value using config options, but there are now named: run.jdkproviders and build.jdkproviders.

maxandersen · 2022-12-13T06:58:43Z

Should it not be javahome,path,jbang by default?

quintesse · 2022-12-13T11:12:28Z

Should it not be javahome,path,jbang by default?

Yes, it is, I just wrote it down wrong in the comment. I updated it.

maxandersen · 2022-12-15T03:10:54Z

trying this out and it does not seem to work for me: jbang jdk --jdk-providers=sdkman list gives:

[jbang] jbang version 0.101.0.13
[jbang] [ERROR] No providers could be initialized. Aborting.
dev.jbang.cli.ExitException: No providers could be initialized. Aborting.
	at dev.jbang.net.JdkManager.initProvidersByName(JdkManager.java:75)
	at dev.jbang.cli.JdkProvidersMixin.initJdkProviders(JdkProvidersMixin.java:17)
	at dev.jbang.cli.Jdk.list(Jdk.java:64)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at picocli.CommandLine.executeUserObject(CommandLine.java:1972)
	at picocli.CommandLine.access$1300(CommandLine.java:145)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2358)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2352)
	at dev.jbang.cli.JBang$3.handle(JBang.java:147)
	at dev.jbang.cli.JBang$3.handle(JBang.java:142)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
	at picocli.CommandLine$RunLast.execute(CommandLine.java:2316)
	at picocli.CommandLine.execute(CommandLine.java:2078)
	at dev.jbang.Main.main(Main.java:14)
[jbang] If you believe this a bug in jbang, open an issue at https://github.com/jbangdev/jbang/issues

wondering if it fails because i don't actually have any sdkman java folder yet?

Beyond that we should have some basic docs for how to use this.

maxandersen · 2022-12-15T03:12:07Z

also...should jbang jdk --jdk-providers=path list technically not list something ?

maxandersen · 2022-12-15T03:18:19Z

so yes, issue with sdkman was that i had no jdks installed yet. that shouldn't fail IMO.

That said now I installed 2 java 17's and I get:

jbang jdk --verbose --jdk-providers=sdkman list --show-details
[jbang] jbang version 0.101.0.13
Installed JDKs (<=default):
   17 (17.0.5, sdkman, 17.0.5-amzn-sdkman, /Users/manderse/.sdkman/candidates/java/17.0.5-amzn)
   17 (17.0.5, sdkman, 22.3.r17-grl-sdkman, /Users/manderse/.sdkman/candidates/java/22.3.r17-grl)

how do I as a user know which is picked when I use //JAVA 17 ?

maxandersen · 2022-12-15T03:22:17Z

jbang jdk install 17 complains about java 17 already being installed...surprised me. turns out its because sdkman installed it in path. Maybe have message state which java 17 it is considering.

maxandersen · 2022-12-15T03:31:03Z

this might be expected but seems inconsistent to me somehow:

jbang jdk list --show-details
Installed JDKs (<=default):
   8 (1.8.0_352, jbang, 8-jbang, /Users/manderse/.jbang/cache/jdks/8)
   11 (11.0.17, jbang, 11-jbang, /Users/manderse/.jbang/cache/jdks/11)
   17 (17.0.4.1, jbang, 17-jbang, /Users/manderse/.jbang/cache/jdks/17) <
❯ jbang jdk jdk-env
Unmatched argument at index 1: 'jdk-env'
Did you mean: java-env?
❯ jbang jdk java-env
export PATH="/Users/manderse/.jbang/currentjdk/bin:$PATH"
export JAVA_HOME="/Users/manderse/.jbang/currentjdk"
# Run this command to configure your shell:
# eval $(jbang jdk java-env)

why is default java-env using a path not listed in available JVMs?

maxandersen · 2022-12-15T03:45:56Z

another quirk:

jbang jdk --verbose --jdk-providers=path,sdkman java-env
[jbang] jbang version 0.101.0.13
[jbang] No default JDK set, use 'jbang jdk default <version>' to set one.
[jbang] [ERROR] JDK null is not installed
dev.jbang.cli.ExitException: JDK null is not installed
	at dev.jbang.cli.Jdk.javaEnv(Jdk.java:183)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at picocli.CommandLine.executeUserObject(CommandLine.java:1972)
	at picocli.CommandLine.access$1300(CommandLine.java:145)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2358)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2352)
	at dev.jbang.cli.JBang$3.handle(JBang.java:147)
	at dev.jbang.cli.JBang$3.handle(JBang.java:142)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
	at picocli.CommandLine$RunLast.execute(CommandLine.java:2316)
	at picocli.CommandLine.execute(CommandLine.java:2078)
	at dev.jbang.Main.main(Main.java:14)
[jbang] If you believe this a bug in jbang, open an issue at https://github.com/jbangdev/jbang/issues

might be because i have java11 in path but jbang seem to want a release file?

 jbang jdk --verbose --jdk-providers=path,sdkman list --show-details
[jbang] jbang version 0.101.0.13
[jbang] Unable to read 'release' file in path: /usr
Installed JDKs (<=default):
   17 (17.0.5, sdkman, 22.3.r17-grl-sdkman, /Users/manderse/.sdkman/candidates/java/22.3.r17-grl)

There's now a new "javahome" and a "path" JDK provider so the user can limit the JDK discovery to just `JAVA_HOME` or `PATH` if they want to.

This is just a minor thing but backslashes are not special characters on Windows so we shouldn't turn to quoting just because a CLI argument has some. Also removed `;` from the list of safe characters for powershell.

We could already override the cache folder location by using the `JBANG_CACHE_DIR` environment variable, but now it's also possible to to do so for specific cache classes by appending their name to the variable name, eg. setting `JBANG_CACHE_DIR_JDKS` would only override that specific folder.

Before if you would run the tests with a JDK older than 11 the tests would download the JDK 11 multiple times because the cache gets cleared between each test. We now use a single folder just for JDKs for the entire test run.

This is basically just to have a way to pass the value of a possible `--fresh` option to the builder, without it being as generic as "fresh". In this case the option `skipMetadataImport` will _only_ affect the importing of metadata from any previously created and cached JARs.

Code can now query JDK providers without triggering an installation. Added som emore inline docs.

Also `jdk list` now shows more detailed information when passed the new flag `--show-details`.

The providers are no longer handling default JDKs themselves. It's now a central feature of JBang itself again.

Added two new JDK providers: "current" and "default" which makes it possible to remove some hard-coded heuristics for finding the "most eligible" JDK that should be used if no specific version is given.

With the recent changes made the above mentioned commands no longer automatically installed missing JDKs, they now do so again.

maxandersen · 2023-01-14T00:24:50Z

tried install with sdkman:

jbang jdk --jdk-providers=sdkman --verbose java-env 12.0-sdkman
JAVA_HOME is set but does not seem to point to a valid Java JDK
[jbang] jbang version 0.101.0.25
[jbang] [ERROR] No providers could be initialized. Aborting.
dev.jbang.cli.ExitException: No providers could be initialized. Aborting.
	at dev.jbang.net.JdkManager.initProvidersByName(JdkManager.java:86)
	at dev.jbang.cli.JdkProvidersMixin.initJdkProviders(JdkProvidersMixin.java:17)
	at dev.jbang.cli.Jdk.javaEnv(Jdk.java:181)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at picocli.CommandLine.executeUserObject(CommandLine.java:1972)
	at picocli.CommandLine.access$1300(CommandLine.java:145)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2358)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2352)
	at dev.jbang.cli.JBang$3.handle(JBang.java:147)
	at dev.jbang.cli.JBang$3.handle(JBang.java:142)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
	at picocli.CommandLine$RunLast.execute(CommandLine.java:2316)
	at picocli.CommandLine.execute(CommandLine.java:2078)
	at dev.jbang.Main.main(Main.java:14)
[jbang] If you believe this a bug in jbang, open an issue at https://github.com/jbangdev/jbang/issues
❯ jbang jdk --jdk-providers=sdkman java-env 12.0-sdkman
JAVA_HOME is set but does not seem to point to a valid Java JDK
[jbang] [ERROR] No providers could be initialized. Aborting.
[jbang] Run with --verbose for more details

quintesse · 2023-01-14T01:02:18Z

tried install with sdkman:

And you were using the latest version? Because I get a different error:

$ jbang jdk --jdk-providers=sdkman --verbose java-env 123-sdkman
[jbang] jbang version 0.101.0.25
[jbang] [ERROR] JDK id is not available for installation: 123-sdkman

which is correct.

If I use an existing id it works fine:

$ jbang jdk --jdk-providers=sdkman --verbose java-env 11.0.12-open-sdkman
[jbang] jbang version 0.101.0.25
[jbang] Using JDK: 11 (11.0.12, 11.0.12-open-sdkman, /home/tschotan/.sdkman/candidates/java/11.0.12-open)
export PATH="/home/tschotan/.sdkman/candidates/java/11.0.12-open/bin:$PATH"
export JAVA_HOME="/home/tschotan/.sdkman/candidates/java/11.0.12-open"
# Run this command to configure your shell:
# eval $(jbang jdk java-env 11.0.12-open-sdkman)

maxandersen · 2023-01-14T13:57:51Z

Ah. It's not telling sdkman to install it. Gotcha.

Maybe respond with an error stating the id is not found but you can install generic java using numbers or otherwise see available jdks using jdk list?

So you can do things like `jdk home 11+` or `jdk install 12.0-sdkman`

quintesse · 2023-01-16T19:17:00Z

Ah. It's not telling sdkman to install it. Gotcha.

No, although that's definitely technically possible, the provider would "just" need to implement the install() method, but I decided that was just too much for now. So for any 3rd party providers the user need to use the appropriate 3rd party install mechanism.

Maybe respond with an error stating the id is not found but you can install generic java using numbers or otherwise see available jdks using jdk list?

Done.

maxandersen · 2023-01-17T00:00:39Z

although this does not set working java_home:

 jbang jdk --jdk-providers=path java-env
export PATH="/usr/bin:$PATH"
export JAVA_HOME="/usr"
# Run this command to configure your shell:
# eval $(jbang jdk java-env)

I'm going to merge it with that as a known limitation we can improve on - as its a rather obscure option atm :)

quintesse force-pushed the jdkproviders branch 2 times, most recently from 5f98b1a to b99310c Compare October 31, 2022 20:36

quintesse force-pushed the jdkproviders branch 2 times, most recently from a6a5f35 to 7ba59a4 Compare November 1, 2022 00:07

quintesse force-pushed the jdkproviders branch 3 times, most recently from 7e581ba to aa42394 Compare November 15, 2022 13:04

quintesse marked this pull request as ready for review November 15, 2022 14:19

quintesse force-pushed the jdkproviders branch 2 times, most recently from 533437c to c3394f1 Compare December 6, 2022 00:07

quintesse force-pushed the jdkproviders branch 2 times, most recently from faac707 to 5017424 Compare December 13, 2022 01:16

quintesse requested a review from maxandersen December 13, 2022 01:46

quintesse marked this pull request as draft December 13, 2022 13:23

quintesse force-pushed the jdkproviders branch from 5017424 to 1e8425f Compare December 14, 2022 22:53

quintesse force-pushed the jdkproviders branch from 1e8425f to 79defd0 Compare December 15, 2022 10:10

quintesse added 8 commits January 11, 2023 14:57

refactor: split EnvJdkProvider in two

24030bb

There's now a new "javahome" and a "path" JDK provider so the user can limit the JDK discovery to just `JAVA_HOME` or `PATH` if they want to.

chore: backslashes don't cause quoting anymore for CMD or PS

49451c0

This is just a minor thing but backslashes are not special characters on Windows so we shouldn't turn to quoting just because a CLI argument has some. Also removed `;` from the list of safe characters for powershell.

tests: don't excessively download JDKs

07ff418

Before if you would run the tests with a JDK older than 11 the tests would download the JDK 11 multiple times because the cache gets cleared between each test. We now use a single folder just for JDKs for the entire test run.

chore: final set of tests now work

1f94a28

Code can now query JDK providers without triggering an installation. Added som emore inline docs.

refactor: providers now return JDK lists instead of sets

5fe086e

Also `jdk list` now shows more detailed information when passed the new flag `--show-details`.

refactor: extracted default JDK handling

b6f159f

The providers are no longer handling default JDKs themselves. It's now a central feature of JBang itself again.

quintesse force-pushed the jdkproviders branch 2 times, most recently from 12e4e1c to b5c4e32 Compare January 11, 2023 22:27

quintesse marked this pull request as ready for review January 12, 2023 10:27

refactor: added new JDK providers

05013af

Added two new JDK providers: "current" and "default" which makes it possible to remove some hard-coded heuristics for finding the "most eligible" JDK that should be used if no specific version is given.

quintesse force-pushed the jdkproviders branch from b5c4e32 to 05013af Compare January 12, 2023 12:26

chore: jdk home and jdk java-env again install JDKs

6053c01

With the recent changes made the above mentioned commands no longer automatically installed missing JDKs, they now do so again.

quintesse force-pushed the jdkproviders branch 2 times, most recently from 7c21e19 to b4f2d4a Compare January 13, 2023 19:32

quintesse force-pushed the jdkproviders branch 3 times, most recently from 98b1bda to ab27a80 Compare January 16, 2023 14:51

feat: jdk commands now support extended versions and ids

d845114

So you can do things like `jdk home 11+` or `jdk install 12.0-sdkman`

quintesse force-pushed the jdkproviders branch from ab27a80 to d845114 Compare January 16, 2023 16:01

maxandersen merged commit 20b39af into jbangdev:main Jan 17, 2023

This was referenced Feb 14, 2023

Upgrade to Disco API v3 #1415

Closed

Is the JDK provider customizable? #316

Closed

quintesse mentioned this pull request Jun 6, 2023

JDK lookups should include sdkman installed JDKs #1476

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Enabled the usage of existing JDK installations #1500

feat: Enabled the usage of existing JDK installations #1500

quintesse commented Oct 31, 2022

maxandersen commented Oct 31, 2022

quintesse commented Oct 31, 2022

codecov bot commented Nov 1, 2022 •

edited

Loading

quintesse commented Nov 15, 2022 •

edited

Loading

quintesse commented Dec 13, 2022

quintesse commented Dec 13, 2022 •

edited

Loading

maxandersen commented Dec 13, 2022

quintesse commented Dec 13, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Jan 14, 2023

quintesse commented Jan 14, 2023

maxandersen commented Jan 14, 2023

quintesse commented Jan 16, 2023

maxandersen commented Jan 17, 2023

feat: Enabled the usage of existing JDK installations #1500

feat: Enabled the usage of existing JDK installations #1500

Conversation

quintesse commented Oct 31, 2022

maxandersen commented Oct 31, 2022

quintesse commented Oct 31, 2022

codecov bot commented Nov 1, 2022 • edited Loading

Codecov Report

quintesse commented Nov 15, 2022 • edited Loading

quintesse commented Dec 13, 2022

quintesse commented Dec 13, 2022 • edited Loading

maxandersen commented Dec 13, 2022

quintesse commented Dec 13, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Dec 15, 2022

maxandersen commented Jan 14, 2023

quintesse commented Jan 14, 2023

maxandersen commented Jan 14, 2023

quintesse commented Jan 16, 2023

maxandersen commented Jan 17, 2023

codecov bot commented Nov 1, 2022 •

edited

Loading

quintesse commented Nov 15, 2022 •

edited

Loading

quintesse commented Dec 13, 2022 •

edited

Loading