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

Unify build tool integrations with Metals #895

Closed
jvican opened this issue May 15, 2019 · 1 comment · Fixed by #942
Closed

Unify build tool integrations with Metals #895

jvican opened this issue May 15, 2019 · 1 comment · Fixed by #942
Assignees
Labels
feature gradle integrations maven Anything concerning Maven integration sbt

Comments

@jvican
Copy link
Contributor

jvican commented May 15, 2019

State of the art

Integrating with Metals requires several changes in any build to get the full IDE experience (find all references, go to definition, rename, etc). These include:

  1. Adding semanticdb to every project definition and configuring it based on its Scala version. Dealing with semanticdb compiler plugin options such as target and source roots is also necessary.
  2. Disabling -Xfatal-warnings so that warnings turned as errors don't cripple IDE features and enabling -Yrangepos.
  3. Setting bloopExportJarClassifiers to true so that library dependencies have source artifacts that "Go to Definition" can access.

These changes are done per build tool. To support sbt, Metals has an sbt plugin that depends on sbt-bloop and configures the options above. To support Maven and Gradle, we are inlining these modifications in the plugin sources instead.

However, this has several disadvantages. Sbt is the best supported build tool and the "Import Build" experience is still:

  1. Slow. A plugin is added to the global sbt directory which cleans previous caches and forces a new reload every time a build is imported (which affects sbt shells running in users' terminals).
  2. Fragile. Adding a global sbt plugin creates problems around cache cleaning, corrupted settings and conflicts with other sbt plugins.

To those disadvantages, we need to add the complexity of modifying scalac options of build targets in build tools such as Maven and mill, where no easy way to modify the build settings is provided.

Proposed solution

Me and @olafurpg have been working on an proposal to improve on the status quo and provide a better built-in experience, while eliminating the above drawbacks.

The idea is to centralize the modifications required by Metals in bloop itself instead of spreading them across all plugins. The solution will work as follows:

  1. Metals adds the sbt-bloop plugin to project/metals.sbt and gitignores it.
  2. Metals triggers sbt bloopInstall, the whole build is exported.
  3. Metals connects to Bloop via BSP and communicates which semanticdb jars it should use for every supported Scala version.
  4. Bloop will take those semanticdb jars and apply the build target modifications to its in-memory representation. It's important these changes are not written back to the .bloop configuration files, but that instead we create some infrastructure to apply these changes every time there's a change in a configuration file under .bloop/.
  5. Bloop will save the choice for semanticdb artifacts in a workspace settings file under .bloop. Next time that the server is restarted, bloop will detect that setting is set and transform the build targets appropriately. This avoids invalidating compilation caches.

Advantages

  1. A single source of truth, one "unique view" of the build projects. Right now, there are two views, the one generated by Metals and the one generated by a single user bloopInstall.
  2. Works for any build tool, out-of-the-box! Sbt, gradle, maven, mill, fury; all of them are supported.
  3. Simpler than before, Metals can get rid of a bunch of build-tool-specific code.
  4. Fast. sbt-bloop is added in the build by default and caches are not invalidated every time we "Import Build" from VS Code.
  5. Robust, we don't change anything in the build definition so sbt keeps working as before and users don't need to deal with Metals/Bloop errors or conflicts with existing plugins.

Disadvantages

  1. Less modular design. Everything will be now centralized in the bloop server.

Remarks

Comparing performance of bloop and sbt is not as easy as before. People might be comparing wrong timings in performance if they check compilation of bloop after Metals has been connecting and an sbt shell (Bloop would do more job as -Yrangepos and the semanticdb plugin increase compilation time).

This is kind of already happening at the moment when users export the build with Metals and then compile the projects with bloop and sbt independently, but it's worth pointing out again as other solutions could have worked around this problem.

I'm hopeful we can partially address this issue by creating a benchmarking guide and providing benchmark code to compare compilation timings across sbt and bloop (ensuring they use the same scalac options, etc).

Required changes in bloop

These are the concrete changes we would need to add in bloop.

  1. Add a mechanism to persist options per workspace under $WORKSPACE/.bloop/. We can create a new JSON file workspace-settings.bloop.json or workspace-settings.json where we store these options.
  2. Add a generic mechanism to simulate -Xfatal-warnings but without forcing a whole compilation to fail. Compilations with warnings would succeed (we would correctly save the class files) but we would turn warnings into errors and fail with a non-zero exit code. The client will think that the compilation failed indeed, but bloop will instead reuse it and just update the code if the warnings are removed.
  3. Implement a new abstraction to map build targets whenever they are loaded from disk and transform their model. Add the infrastructure to set a transformer whenever Metals connects to it. By default, the identity is used.

/cc @tgodzik This might be of interest to you Tomasz since you've been mostly working on this area and are familiar with the current pain points. What do you think of the proposed solution?

@jvican
Copy link
Contributor Author

jvican commented May 28, 2019

@tgodzik I'm assigning you to the ticket based on our discussion last week. Feel free to ask any questions or take a stab at it whenever you can 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature gradle integrations maven Anything concerning Maven integration sbt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants