scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.08k stars 328 forks source link

user.dir system property should not be specified if it user defined in the launch config #2888

Open spangaer opened 3 years ago

spangaer commented 3 years ago

Describe the bug

When I specify a -Duser.dir=, myself in VSCode launch.json it gets overridden by one metals seems to inject.

"jvmOptions": [
        "-Duser.dir=C:\\me\\dev\\p4\\jede.ESKO-CHARLIE.cloud-core.16.0\\deploy\\dev-jede-eu\\"
      ]

VisualVM read-out:

-Duser.dir=C:\me\dev\p4\jede.ESKO-CHARLIE.cloud-core.16.0\deploy\dev-jede-eu\
-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,quiet=n
-Duser.dir=C:\me\dev\p4\ecc-gaia_next\ecc-gaia

Installation:

tgodzik commented 3 years ago

Thanks for reporting! This seems to be intentional change done in https://github.com/scalacenter/bloop/pull/1175 to follow the semantics of the build tool.

Working directory is currently set as it is in sbt. It can be changed currently with:

fork in run := true
baseDirectory in run := file("/path/to/working/directory/")

and reimported.

I think we should be also able to change that Bloop doesn't add user dir property when it's already specified in the java or alternatively change the order so that default property value is overriden. The actual code is here:

https://github.com/scalacenter/bloop/blob/3eab1f8079cfaf228ec68740daadf5854070c89b/frontend/src/main/scala/bloop/exec/JvmProcessForker.scala#L100

spangaer commented 3 years ago

One or the other would indeed be kinder than current behavior.

I guess those settings in sbt helping out comes to no surprise, but re-importing for changing that is sort of a though one given that I'm working with source deps to 2 projects ATM, so sbt bloopInstall takes while to complete.

tgodzik commented 3 years ago

One or the other would indeed be kinder than current behavior.

I guess those settings in sbt helping out comes to no surprise, but re-importing for changing that is sort of a though one given that I'm working with source deps to 2 projects ATM, so sbt bloopInstall takes while to complete.

Sure, it's not ideal at all, I think we should be able to make the approach a bit more flexible.