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

BSP import: don't add Scala library to runtime classpath in Scala SDK #596

Closed
wants to merge 1 commit into from

Conversation

lrytz
Copy link

@lrytz lrytz commented Sep 8, 2021

When importing a project through bsp, the compiler classpath
(scala-library, scala-reflect, scala-compiler) obtained through
bsp should only be used for the "Compiler classpath" section of the
Scala SDK.

Before this change, the classpath was also added to the module's
<CLASSES> ("Standard library" section of the Scala SDK in the UI).
This is

  • slightly incorrect, because it adds the scala-compiler jar to the
    module's classpath
  • not needed, because the scala-library is added to the module's
    classpath as a separate library

scala/scala-dev#786 (comment)

When importing a project through bsp, the compiler classpath
(scala-library, scala-reflect, scala-compiler) obtained through
bsp should only be used for the "Compiler classpath" section of the
Scala SDK.

Before this change, the classpath was also added to the module's
`<CLASSES>` ("Standard library" section of the Scala SDK in the UI).
This is
  - slightly incorrect, because it adds the scala-compiler jar to the
    module's classpath
  - not needed, because the scala-library is added to the module's
    classpath as a separate library

scala/scala-dev#786 (comment)
@jastice jastice self-requested a review September 8, 2021 13:38
@jastice jastice self-assigned this Sep 8, 2021
Copy link
Member

@jastice jastice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to check for unintended side effects, but it seems fine :)

@jastice
Copy link
Member

jastice commented Sep 21, 2021

Merging was causing troubles so I applied this change manually. Thank you!

86c9f39

@jastice jastice closed this Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants