Use existing metals instance instead of starting new on goto file outside of workspace root

[ValdemarGr]

Describe the bug

In a project with multiple workspaces where one or more child projects are not under the same .bsp folder, metals or the editor prompts to start a new server if a file is opened from in a folder that does not share the same workspace root, even if the current metals instance contains that file in a build target.

  parent/
    .bsp/...
    src/main/scala/parent/Parent.scala

  child/
    src/main/scala/child/Child.scala

What should happen: Parent's bsp server should handle both parent and child projects.

What happens: Opening Parent.scala and then opening Child.scala causes a prompt to appear, asking if Child.scala's project should be imported.

Being able to have this folder structure is especially useful when compiling third party code from source.

I am unsure if this is an issue is related to the metals server or the LSP client? I use nvim-metals.

I suppose one of the BSP choices (maybe even the default?) should be to continue on the currently running BSP instance, in the case that the opened file is part of the already running metals instance's sources list.

Maybe I am missing something essential, since I am also developing the BSP server such that there is an additional axis of potential fault on my part, but here is the full trace, doctor and semanticdb of the child: https://gist.github.com/ValdemarGr/ca1d4b3e5695da07c1e51c4dfe5753d6

The output was generated from the project in the example folder in this project, launched with the .bsp/mezel-local.json BSP configuration which uses a locally built server sbt assembly.

Expected behavior

No response

Operating system

Linux

Editor/Extension

Nvim (nvim-metals)

Version of Metals

v1.3.5

Extra context or search terms

No response

[ValdemarGr]

I think this might be a false report. I wiped my .metals folder and I cannot seem to reproduce it anymore.

[ValdemarGr]

Okay, I think that the reason that it worked in the first place with local third-party projects is because I have had a generated .bloop and .metals folder in the third-party project's folder which I've imported. I tried removing all bsp configurations from non-root projects and the issue occurs consistently.

Can it be confirmed whether or not this is a feature in metals I am not meeting the requirements for, or something the editor LSP client should handle?

[tgodzik]

The source files in the stack trace look weird a bit, they are located in the bazel cache: file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc

Are you opening the deps in the cache? Otherwise we might not be able to see if those are the same ones as in the child project.

Specifically sources result points to the cache:

      "target": {
        "uri": "@hxl//:hxl"
      },
      "sources": [
        {
          "uri": "file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl/modules/core/src/main/scala/hxl/DSKey.scala",
          "kind": 1,
          "generated": false
        },
        {
          "uri": "file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl/modules/core/src/main/scala/hxl/DataSource.scala",
          "kind": 1,
          "generated": false
        },
        {
          "uri": "file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl/modules/core/src/main/scala/hxl/Hxl.scala",
          "kind": 1,
          "generated": false
        },
        {
          "uri": "file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl/modules/core/src/main/scala/hxl/Requests.scala",
          "kind": 1,
          "generated": false
        }
      ],
      "roots": [
        "file:///home/valde/git/mezel/"
      ]
    }

[ValdemarGr]

Are you opening the deps in the cache? Otherwise we might not be able to see if those are the same ones as in the child project.

Yes.

If the third-party sources are fetched from network they'll be in the bazel cache folder. However, the buildTarget is pointing to the correct baseDirectory.

      "id": {
        "uri": "@hxl//:hxl"
      },
      "displayName": "@hxl//:hxl",
      "baseDirectory": "file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl/",

Are there any restrictions on the path of child project (shared root or similar)?

I think I might have been ambiguous with my example folder structure, this is what I meant.

/some/path/parent/
  .git/...
  .bsp/...
  src/main/scala/parent/Parent.scala

/some/other/path/child/
  .git/...
  src/main/scala/child/Child.scala

I can depend on the resulting child project's resulting .jar (I think this is what bazel-bsp does), but that would mean that the user would have to wait until the project was fully compiled until they could goto definition.

[tgodzik]

We do try to support that case and there was a change as recent as https://github.com/scalameta/metals/pull/6563

Metals should not start a new connection if we detect that a current build server already works for that, but maybe there is still some issue. In particular, we do save in .metals the list of projects this project handles (to make sure we don't start two connection when initiating both at the same time). Maybe something about that logic is off? Do you see a json file being written in .metals?

[ValdemarGr]

Other than the bsp trace, there's a settings.json.

{"project-ref":["../../.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl"]}

I am not sure how you detect project handles, but the one in the settings file is relative but buildTarget is absolute.

[tgodzik]

relative should be fine in this case (though will be a problem for windows)

Let's add some debug messages so that it's easier to see on your side https://github.com/scalameta/metals/pull/6675

-Dmetals.loglevel=debug in metals.serverProperties will turn those logs on

[ValdemarGr]

I tried your branch and followed the contributors guide for how to test locally. So I used vscode instead of my usual editor neovim.

The feature seems to work fine in vscode, but doesn't in neovim. Is the LSP client supposed to support this feature? It is a while ago the neovim plugin has been updated https://github.com/scalameta/nvim-metals

[tgodzik]

Honestly, I have no idea, you can create the lsp trace in .metals/lsp.trace.json and see if didChangeWorkspaceFolders are being sent or what is the differences between stack trace produced when using VS Code and nvim

@ckipp01 might know more about nvim part.

[ValdemarGr]

Thanks for the quick responses.

Are there any flags I can give metals so it doesn't truncate the trace files? As soon as I goto definition the file is truncated since it thinks it is a new project.

[tgodzik]

Nothing currently :thinking: We always truncate the files to avoid having infinately growing files. We can add a flag here no problem. I will add it to the same PR

[tgodzik]

@ValdemarGr actually this is weird, we don't truncate the file when the same Metals instance is running. And if it's a new project, wouldn't it be a totally new directory with .metals being created there?

[ValdemarGr]

  valde@nixos  ~  ps -eo args | grep metals | cut -c1-400
tail -n 100 -f /home/valde/git/gateway/.metals/metals.log
/nix/store/00qkmlz3vcydw9si4k3lzsrslpqbcprr-openjdk-21.0.3+9/bin/java -XX:+UseG1GC -XX:+UseStringDeduplication -Xss4m -Xms100m -Dmetals.client=nvim-lsp -Dmetals.verbose=true -Dmetals.askToReconnect=false -Dmetals.loglevel=debug -Dmetals.build-server-ping-interval=10h -cp /nix/store/wzqas28amsw2k5112gqs05r9c62r13q0-metals-deps-1.3.5/share/java/ammonite-runner_2.13-0.4.0.jar:/nix/store/wzqas28amsw2k
/nix/store/00qkmlz3vcydw9si4k3lzsrslpqbcprr-openjdk-21.0.3+9/bin/java -XX:+UseG1GC -XX:+UseStringDeduplication -Xss4m -Xms100m -Dmetals.client=nvim-lsp -Dmetals.verbose=true -Dmetals.askToReconnect=false -Dmetals.loglevel=debug -Dmetals.build-server-ping-interval=10h -cp /nix/store/wzqas28amsw2k5112gqs05r9c62r13q0-metals-deps-1.3.5/share/java/ammonite-runner_2.13-0.4.0.jar:/nix/store/wzqas28amsw2k
grep --color=auto --exclude-dir=.bzr --exclude-dir=CVS --exclude-dir=.git --exclude-dir=.hg --exclude-dir=.svn --exclude-dir=.idea --exclude-dir=.tox metals

✘ valde@nixos  ~  ps -A | grep java | awk '{ print $1 }' | xargs -I{} pwdx {}   
94956: /home/valde/git/mezel
95521: /home/valde/git/mezel
95611: /home/valde/git/mezel
95632: /home/valde/git/mezel
96140: /home/valde/git/mezel
99659: /home/valde/git/mezel

It starts a new process, but the PWD is the same.

[tgodzik]

You could just do tail -f .metals/lsp.trace.json > backup.json

[ValdemarGr]

It looks like the LSP client asks to initialize after getting the textDocument/definition response. backup.json

Edit; include what I think might be the relevant part:

[Trace - 01:18:02 PM] Received request 'textDocument/definition - (7)'
Params: {
  "textDocument": {
    "uri": "file:///home/valde/git/mezel/example/src/main/scala/example/Main.scala"
  },
  "position": {
    "line": 6,
    "character": 4
  }
}

[Trace - 01:18:02 PM] Sending response 'textDocument/definition - (7)'. Processing request took 108ms
Result: [
  {
    "uri": "file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl/modules/core/src/main/scala/hxl/Hxl.scala",
    "range": {
      "start": {
        "line": 43,
        "character": 7
      },
      "end": {
        "line": 43,
        "character": 10
      }
    }
  }
]

[Trace - 01:18:02 PM] Received request 'initialize - (1)'
Params: {
  "workDoneToken": "1",
  "processId": 115398,
  "rootPath": "/home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl",
  "rootUri": "file:///home/valde/.cache/bazel/_bazel_valde/mezel-b38cc3c8af6e82d3fafc60d8b18e56bc/external/hxl",
  "initializationOptions": {

[tgodzik]

Looks like a completely new Metals server is started in the new directory, which is why it has no knowledge of the previous BSP server. This doesn't look like something we can fix in Metals, wouldn't it be a neovim logic?

[ValdemarGr]

It might be related to https://github.com/scalameta/nvim-metals/issues/685 And https://github.com/scalameta/nvim-metals/issues/671

[ValdemarGr]

This seems to fix it https://github.com/scalameta/nvim-metals/issues/671#issuecomment-2194575956 .

I suppose it would be ideal if the default semantics were the same as in vscode?

[tgodzik]

I suppose it would be ideal if the default semantics were the same as in vscode?

I guess that depends, but in this case sure. I have very limited understanding of the neovim plugin though.

[ckipp01]

I'm chiming in a bit late here, but nvim-metals works a bit different with multiple root projects since in neovim the idea of a "root" doesn't really exist. We sort of artificially determine where it should be in a single place. However when you have two different roots in a single "workspace", things get tricky. Therefore you'll want to use the *vim.lsp.buf.add_workspace_folder()* to manually add the other root. Then nvim-metals will be able to correctly work with both roots.