Correctly detect the project runtime based on the active language server #124

asmodeus812 · 2024-07-02T11:40:39Z

The goal of this pr is to correctly resolve the runtime home directory of the jdk that a specific project is using, after that the JAVA_HOME env variable is set to that, and the appropriate maven or gradle commands can be run.

This has to be done when we call maven or gradle. The reason this is needed is pretty simple, noone has just one single runtime installed on their system, and most people work with multiple java projects configured/developed to work with different runtime versions.

For example say a user has installed the latest jdk & setup his JAVA_HOME and PATH to point to jdk21, however the user would like to run/compile the tests for an older java 8, 11 or 17 spring project.

The JAVA_HOME variable has to be set to the correct runtime for the project before any relevant maven or gradle commands are run otherwise, for more simple projects you might get by and compile it, but for more complex projects which might be using deprecated or straight up removed features between major jdk versions, you will either fail to compile the code or you will encounter a runtime exception while running the code.

By default maven always picks up JAVA_HOME, you can also configure this in the pom itself, (by adding additional toolchain and compiler plugins) by setting the path to the runtime in the compiler plugin (for your local machine) gradle is more versatile it still reads JAVA_HOME, however you can start it with -D argument or you can configure the path to the runtime in build.gradle.

Both maven and gradle have toolchain plugins which allow them to specify the java version/runtime for the project separate from the java stack on user's PATH, manually, proivde absolute path to the desired runtime on your local machine, that the project has to be run with, however in practice noone does this, locally, since most projects are microservices, they rely on pod's env.

You can see why this is kind of needed, while it is possible to provide the path to the runtime for a project, and have it be separate from JAVA_HOME (i.e the java stack which is on your PATH)

would not be deployeable/portable in a cluster
it is not usually done, since it requires user input, locally modifying the pom
it is specific for the user's machine, people install them in different places

The solution in this plugin is to provide the runtimes just once, either as env variables JAVA_HOME_XX, or in your lua config, not having to modify the pom for each project, locally, separately, having to add additional maven/gradle plugins to the project just to be able to specify this etc, and you would certainly forget to do that.

TL;DR - Nothing guarantees you that the java stack on user's PATH is the one a specific project requires to be compiled / depends on.

lua/neotest-java/build_tool/gradle.lua

rcasia · 2024-07-05T23:30:00Z

lua/neotest-java/command/runtime.lua

+		return
+	end
+
+	local config = client.config.settings.java or {}


This is giving me this error. I suspect it is because I don't use any lsp settings.
It also happens when I have no lsp client active.

In general, neotest-java should not relay in LSPs, they should be just a complement.

 Warn 01:25:56 AM notify.warn Neotest neotest-java: .../REPOS/neotest-java/lua/neotest-java/command/runtime.lua:19: attempt to index local 'client' (a nil value) stack traceback: /home/rico/REPOS/neotest/lua/neotest/client/init.lua:89: in function '__index' .../REPOS/neotest-java/lua/neotest-java/command/runtime.lua:19: in function 'extract_runtime' .../REPOS/neotest-java/lua/neotest-java/command/runtime.lua:57: in function 'runtime' ...REPOS/neotest-java/lua/neotest-java/build_tool/maven.lua:80: in function 'get_dependencies_classpath' ...REPOS/neotest-java/lua/neotest-java/build_tool/maven.lua:97: in function 'prepare_classpath' ...-java/lua/neotest-java/command/junit_command_builder.lua:128: in function 'build' ...EPOS/neotest-java/lua/neotest-java/core/spec_builder.lua:88: in function 'build_spec' /home/rico/REPOS/neotest/lua/neotest/client/runner.lua:76: in function '_run_tree' /home/rico/REPOS/neotest/lua/neotest/client/runner.lua:65: in function [C]: in function 'xpcall' /home/rico/REPOS/neotest/lua/neotest/client/init.lua:84: in function 'run_tree' /home/rico/REPOS/neotest/lua/neotest/consumers/run.lua:73: in function 'func' /home/rico/REPOS/nvim-nio/lua/nio/tasks.lua:173: in function

lua/neotest-java/command/runtime.lua

lua/neotest-java/lsp/init.lua

rcasia · 2024-07-06T00:05:34Z

lua/neotest-java/lsp/init.lua

+-- table that holds the language server settings, this is mostly done for interfacing with coc.nvim, since native neovim lsp clients hold their settings in the clients table
+local SETTINGS = {}
+
+local function execute_command(command, bufnr)


I would separate this two conditional branches into two different functions as their logic is unrelated and they are long blocks of code.

lua/neotest-java/lsp/init.lua

Add a fallback way of extracting the runtime Allow the user to actually specify the runtime

asmodeus812 · 2024-07-11T06:50:44Z

@rcasia could you take a look again, there are a few standing questions, regarding piping, extracting the bufnr and what do we do with gradle's runtime

rcasia · 2024-07-14T18:29:52Z

lua/neotest-java/command/runtime.lua

I would not do this job here as the lsp should know what java executable to use using vscode.java.resolveJavaExecutable like in here: https://github.com/mfussenegger/nvim-jdtls/blob/6bfd1591583b02e742fc3a2f43393c4ea3b6d3c7/lua/jdtls/util.lua#L70

rcasia · 2024-07-14T18:42:00Z

README.md

@@ -66,13 +66,21 @@ require("neotest").setup({
  adapters = {
    require("neotest-java")({
        ignore_wrapper = false, -- whether to ignore maven/gradle wrapper
+        java_runtimes = {


At least nvim-jdtls and coc-java already do this work. This seems like duplicate work as lsp could be already has this setting. Or am I missing something?

rcasia · 2024-07-14T18:45:28Z

lua/neotest-java/util/read_xml_tag.lua

-	if type(parsed) == "table" then
-		return nil
-	end
+	-- why ? there are tags which can hold multiple tags / array of tags ?


I just wanted to retturn a non-table value, but feel free to use it to retrieve tables aswell.

rcasia · 2024-07-14T19:06:16Z

With those changes it looks alright and ready to merge.

asmodeus812 · 2024-07-15T06:12:29Z

With those changes it looks alright and ready to merge.

Not yet, almost. There are a few things to iron out.

rcasia · 2024-10-06T16:29:59Z

I will close this PR as #153 will fix the issue.

Thanks, this PR actually helped to get close to a solution.

asmodeus812 · 2024-10-07T14:34:50Z

@rcasia what about porting this to support coc, as the original pr ?

rcasia · 2024-10-07T18:10:45Z

Responded in #75 (comment)

Correctly detect the project runtime based on the active language server

d04059e

asmodeus812 force-pushed the feature/detect-project-runtimes branch from 1346e28 to d04059e Compare July 2, 2024 11:45

rcasia requested changes Jul 6, 2024

View reviewed changes

asmodeus812 added 7 commits July 6, 2024 07:51

Fix up some general issues

5f7ab21

Add a fallback way of extracting the runtime Allow the user to actually specify the runtime

Fix review notes

47fdcd7

Fix some more review notes and left overs

37f83cd

Remove left over statements

3969556

Move some comments around and restore run in gradle

1796fdd

Split maven and gradle runtime extraction

0540124

Add runtime config and more logging

3ec8190

rcasia linked an issue Jul 11, 2024 that may be closed by this pull request

Use correct JDK runtime for projects #75

Open

rcasia requested changes Jul 14, 2024

View reviewed changes

asmodeus812 mentioned this pull request Jul 15, 2024

Add optional env variables to spawn nvim-neotest/neotest#438

Open

Minor changes and improvements around run command

15cf518

rcasia closed this Oct 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly detect the project runtime based on the active language server #124

Correctly detect the project runtime based on the active language server #124

asmodeus812 commented Jul 2, 2024 •

edited

Loading

rcasia Jul 5, 2024

rcasia Jul 6, 2024

asmodeus812 commented Jul 11, 2024

rcasia Jul 14, 2024

rcasia Jul 14, 2024

rcasia Jul 14, 2024

rcasia commented Jul 14, 2024

asmodeus812 commented Jul 15, 2024

rcasia commented Oct 6, 2024

asmodeus812 commented Oct 7, 2024

rcasia commented Oct 7, 2024

Correctly detect the project runtime based on the active language server #124

Correctly detect the project runtime based on the active language server #124

Conversation

asmodeus812 commented Jul 2, 2024 • edited Loading

rcasia Jul 5, 2024

Choose a reason for hiding this comment

rcasia Jul 6, 2024

Choose a reason for hiding this comment

asmodeus812 commented Jul 11, 2024

rcasia Jul 14, 2024

Choose a reason for hiding this comment

rcasia Jul 14, 2024

Choose a reason for hiding this comment

rcasia Jul 14, 2024

Choose a reason for hiding this comment

rcasia commented Jul 14, 2024

asmodeus812 commented Jul 15, 2024

rcasia commented Oct 6, 2024

asmodeus812 commented Oct 7, 2024

rcasia commented Oct 7, 2024

asmodeus812 commented Jul 2, 2024 •

edited

Loading