search

com-lihaoyi / mill

Your shiny new Java/Scala build tool!
https://mill-build.com/
MIT License
1.99k stars 303 forks source link

BSP's dependencyModules do not return transitive libraries #3165

Open tgodzik opened 1 month ago

tgodzik commented 1 month ago

While investigating https://github.com/scalameta/metals/issues/6419 I realized that dependencyModules returns only the Scala 3 library for a simple Scala 3 project. It's missing the Scala 2.13 library.

My guess is that no transitive dependencies are being returned.

I can work around it by not requesting dependencyModules from Mill BSP server, but I wanted to mention it before doing a workaround.

lefou commented 1 month ago

You are right, there is no resolution step in the current implementation. It's probably an oversight.

P.S. I've planned to touch that code as part of a fix for https://github.com/com-lihaoyi/mill/issues/3148, but that will no happen in the near next days.

tgodzik commented 1 month ago

There is no hurry, unless this will take more than a month I don't think I need to add a workaround. Thanks!

tgodzik commented 1 month ago

Or did I misunderstand and it would be better to add it for now?

lefou commented 1 month ago

The fix in the right place, e.g. call resolveDeps, should be easier than any workaround.

https://github.com/com-lihaoyi/mill/blob/27ff770f797a677e013e0242a365ff60cafc8271/bsp/worker/src/mill/bsp/worker/MillBuildServer.scala#L402-L431

megri commented 1 month ago

Any updates on this? Perhaps I can help (I reported the original issue here: https://github.com/scalameta/metals/issues/6419)

lefou commented 1 month ago

@megri Go ahead, I haven't started yet.

megri commented 3 weeks ago

The fix in the right place, e.g. call resolveDeps, should be easier than any workaround.

https://github.com/com-lihaoyi/mill/blob/27ff770f797a677e013e0242a365ff60cafc8271/bsp/worker/src/mill/bsp/worker/MillBuildServer.scala#L402-L431

Perhaps I'm misunderstanding the proposed solution but wouldn't it be weird to call m.resolveDeps in this method? That is, shouldn't these dependencies be resolved elsewhere?

lefou commented 2 weeks ago

Perhaps I'm misunderstanding the proposed solution but wouldn't it be weird to call m.resolveDeps in this method? That is, shouldn't these dependencies be resolved elsewhere?

@megri There is some resolution needed, that's for sure. Of course we already have a target which resolves all ivy dependencies used for compilation (JavaModule.resolvedIvyDeps), but it is only returning some jars, not the full Maven coordinates, which are clearly needed for the dependencyModules request. So we have multiple options here:

  1. Refactor JavaModule to also return the coordinates for each jar, which isn't trivial since there is no API in coursier to get that easily. You either get some resolved metadata without the jar location or some jar location without the metadata. At least from what I have discovered about the API.

  2. Just run the resolution locally in this BSP request (by doing something like what resolvedIvyDeps does). Since coursier has a cache and is fast, it should not impose much overhead. Otherwise, we have the same limitations regarding the coursier API, so it's probably easier to resolve and assemble the dependency data locally and only refactor after we have found a nice way to assemble it.