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

Mill 0.10.9 BSP unable to initialize with nightly Metals #2123

Closed
ckipp01 opened this issue Nov 14, 2022 · 1 comment · Fixed by #2124
Closed

Mill 0.10.9 BSP unable to initialize with nightly Metals #2123

ckipp01 opened this issue Nov 14, 2022 · 1 comment · Fixed by #2124
Labels
bug The issue represents an bug
Milestone

Comments

@ckipp01
Copy link
Collaborator

ckipp01 commented Nov 14, 2022

So I think some combination of these prs are causing things to break or behave in weird ways when using Mill as a build server with the nightly version of Metals. Mainly, the BSP server doesn't respond at all.

To illustrate I'll start with a fresh project.

g8 com-lihaoyi/mill-scala-hello.g8

Everything working

So I'll start with the following:

  • Metals version: 0.11.9 (brings in 0.4.2 of millw and sticks it in a temp dir)
  • Mill 0.10.9

Steps taken

  1. Open the project
  2. Answer "Not now" to build import
  3. Trigger a "Switch Build Server" and select mill-bsp
  4. Everything works as expected

.bsp/mill-bsp.json

{
  "name": "mill-bsp",
  "argv": [
    "/Users/ckipp/.cache/mill/download/0.10.9",
    "--bsp",
    "--disable-ticker",
    "--color",
    "false",
    "--jobs",
    "1"
  ],
  "millVersion": "0.10.9",
  "bspVersion": "2.0.0",
  "languages": [
    "scala",
    "java"
  ]
}

jps output is what I'd expect

❯ jps
9601 MillServerMain
9658 Jps
9628 MillClientMain
9565 metals

Stuff not working

Now I'll bump the Metals version to the latest nightly (0.11.9+131-601042ab-SNAPSHOT) which brings in the new placement and version of millw, and I'll fully clean the workspace.

Steps taken

  1. Open the project
  2. Answer "Not now" to build import
  3. Trigger a "Switch Build Server" and select mill-bsp
  4. Wait and see that Metals times out waiting for the BSP connection

.bsp/mill-bsp.json

{
  "name": "mill-bsp",
  "argv": [
    "/Users/ckipp/Documents/scala-workspace/mill-bsp-test/.metals/millw",
    "--bsp",
    "--disable-ticker",
    "--color",
    "false",
    "--jobs",
    "1"
  ],
  "millVersion": "0.10.9",
  "bspVersion": "2.0.0",
  "languages": [
    "scala",
    "java"
  ]
}

There is a mill-bsp.trace file created, but with nothing inside of it and no other files created by Mill.

When I look at the BSP trace file from Metals all I see is the following:

[Trace - 08:59:46 am] Sending request 'build/initialize - (1)'
Params: {
  "rootUri": "file:///Users/ckipp/Documents/scala-workspace/mill-bsp-test/",
  "displayName": "Metals",
  "version": "0.11.9+131-601042ab-SNAPSHOT",
  "bspVersion": "2.1.0-M3",
  "capabilities": {
    "languageIds": [
      "scala",
      "java"
    ]
  },
  "data": {
    "javaSemanticdbVersion": "0.7.4",
    "semanticdbVersion": "4.6.0",
    "supportedScalaVersions": [
      "2.13.10",
      "2.12.17",
      "2.12.16",
      "2.12.15",
      "2.12.14",
      "2.12.13",
      "2.12.12",
      "2.13.5",
      "2.13.6",
      "2.13.7",
      "2.13.8",
      "2.13.9",
      "2.11.12",
      "2.12.9",
      "2.12.10",
      "2.12.11",
      "2.13.1",
      "2.13.2",
      "2.13.3",
      "2.13.4"
    ]
  }
}

So the request is sent, but never responds. Looking at the output of jps I do see that Mill has started:

❯ jps
10118 MillClientMain
10109 metals
10126 MillServerMain
10127 Jps

Other research

I also tried reverting the place that Metals stores the millw back to a tmp dir and that still doesn't work. For example the BSP entry in this case looks like:

{
  "name": "mill-bsp",
  "argv": [
    "/var/folders/fq/nx_jsnyd6550xp03czx898d40000gn/T/metals17064985890367699422/millw",
    "--bsp",
    "--disable-ticker",
    "--color",
    "false",
    "--jobs",
    "1"
  ],
  "millVersion": "0.10.9",
  "bspVersion": "2.0.0",
  "languages": [
    "scala",
    "java"
  ]
}

And the same behavior of an initialize being sent, but not responded to.

Conclusions

From what I can tell it just seems problematic that the BSP entry is pointing to the millw file. I'm not 100% sure how to dig in further to see why Mill isn't responding here or why pointing towards millw is an issue. Any ideas or anything else I can provide?

@lefou lefou added the bug The issue represents an bug label Nov 14, 2022
@lefou
Copy link
Member

lefou commented Nov 14, 2022

Thanks for the report!

It took me a while to see what the issue is, but I think, as we now pass -D as the first option, Mill is no longer correctly switching to the BSP mode. For this, the --bsp options needs to come as a first option. I just checked with other options, which need to be given as the first one like --repl or --interactice, and indeed, they no longer work.

This is actually an issue with the bundled mill script in Mill, as well as with millw 0.4.4.

ckipp01 added a commit to ckipp01/metals that referenced this issue Nov 14, 2022
When I opened com-lihaoyi/mill#2123 it made me
wonder why we didn't hit on this in our tests, but then I realized we
were actually never testing the initialization with Mill server. This
adds some more tests to ensure we're not only testing the generation of
the BSP config file, but also connection to the build server.
lefou added a commit to lefou/millw that referenced this issue Nov 15, 2022
lefou added a commit that referenced this issue Nov 15, 2022
Also respect an already set DEFAULT_MILL_VERSION variable.

Adapted from lefou/millw#42

Fix #2123

Pull request: #2124
@lefou lefou added this to the after 0.10.9 milestone Nov 15, 2022
ckipp01 added a commit to scalameta/metals that referenced this issue Nov 18, 2022
* refactor: ensure we're testing the connection with Mill

When I opened com-lihaoyi/mill#2123 it made me
wonder why we didn't hit on this in our tests, but then I realized we
were actually never testing the initialization with Mill server. This
adds some more tests to ensure we're not only testing the generation of
the BSP config file, but also connection to the build server.

* deps: update millw to 0.4.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue represents an bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants