Skip to content
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

Run CodeLens does not respect Bloop-configured JVM #5780

Closed
harpocrates opened this issue Oct 18, 2023 · 2 comments · Fixed by #5781
Closed

Run CodeLens does not respect Bloop-configured JVM #5780

harpocrates opened this issue Oct 18, 2023 · 2 comments · Fixed by #5781
Milestone

Comments

@harpocrates
Copy link

In my project's Bloop config, I have:

        "platform": {
            "name": "jvm",
            "config": {
                "home": "C:\\Users\\Alec\\MyJdks\\jdk\\jdk-11.0.16.1+1",
                "options": [
                ]
            },
            "mainClass": [
            ],
            "resources": [
            ]
        },

However, when I click "run" in the CodeLens, I see that metals-vscode is using the version of Java that corresponds to JAVA_HOME instead.

Expected behavior

I'd expect the CodeLens to run the main method using the Java version configured in the corresponding Bloop projects JVM configuration (eg. as bloop run would do).

Installation:

  • Operating system: reproduced on Windows, though likely platform independent
  • VSCode version: 1.81.1
  • VSCode extension version: 1.24.5
  • Metals version: 1.0.0
@tgodzik
Copy link
Contributor

tgodzik commented Oct 19, 2023

Thanks for reporting! You should be able to change the metals.javaHome setting to get the proper Java version. We don't have currently a way to get the Java home from Bloop config (aside from parsing json, which I would rather avoid). In the future, that setting will be the project Java Home and Metals will just use its' own separate (sometimes) JVM . That project java home would be exported during import to bloop config. So it should be the same.

I will leave this issue open, since we might be able to figure out how to solve this better in the future.

@tgodzik tgodzik transferred this issue from scalameta/metals-vscode Oct 23, 2023
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 23, 2023
I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test 🤔

Fixes scalameta#5780
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 23, 2023
I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test 🤔

Fixes scalameta#5780
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 23, 2023
I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test 🤔

Fixes scalameta#5780
@tgodzik
Copy link
Contributor

tgodzik commented Oct 23, 2023

Well, this seem to be actually quite easy to do and I totally forgot

tgodzik added a commit to tgodzik/metals that referenced this issue Oct 24, 2023
~~I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test~~

Decided to add test as the logic became a bit more complex.

Fixes scalameta#5780
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 24, 2023
~~I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test~~

Decided to add test as the logic became a bit more complex.

Fixes scalameta#5780
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 24, 2023
~~I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test~~

Decided to add test as the logic became a bit more complex.

Fixes scalameta#5780
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 24, 2023
~~I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test~~

Decided to add test as the logic became a bit more complex.

Fixes scalameta#5780
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 24, 2023
~~I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test~~

Decided to add test as the logic became a bit more complex.

Fixes scalameta#5780
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 25, 2023
~~I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test~~

Decided to add test as the logic became a bit more complex.

Fixes scalameta#5780
tgodzik added a commit that referenced this issue Oct 26, 2023
~~I am hesistant to add a test case, since that would require downloading separate JDK for the sake of one test~~

Decided to add test as the logic became a bit more complex.

Fixes #5780
@tgodzik tgodzik added this to the Metals v1.1.1 milestone Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants