Internal refactorings to prepare for Pants support#1145
Merged
olafurpg merged 40 commits intoscalameta:masterfrom Dec 5, 2019
Merged
Internal refactorings to prepare for Pants support#1145olafurpg merged 40 commits intoscalameta:masterfrom
olafurpg merged 40 commits intoscalameta:masterfrom
Conversation
added 7 commits
December 4, 2019 15:56
Previously, the super shell had a glitch when running `slow/test` because we ran two parallel sbt states at the same time.
This module is necessary to convert between Scala/Java futures.
Even if file watchers seems a bit more reliable on Linux, they're unreliable and possible sources of flaky test failures.
added 22 commits
December 4, 2019 16:54
Previously, the mtags symbol index only supported indexing jars. Now, it also supports source directories. This feature will be used by Pants where we don't only have `*-sources.jar` but also local directories containing source code of dependencies.
Previously, Metals would crash when reading an invalid `*-sources.jar` file (for example because it's actually an HTML redirect file). It's not possible to catch these crashes with `NonFatal` because `ZipError` is fatal, so this exception needs special handling.
Previously, it was only possible to run `bloopInstall` steps via a system process. Now, with this refactoring it's possible to run a `bloopInstall` command as a normal function call. The system process logic for build tools like sbt, Maven, Gradle has been abstracted over in a new `BloopPluginBuildTool` helper.
This class is useful outside of tests.
The file hashing feature in the directory watcher makes file watching very expensive to start in large directories. See http://scalameta.org/metals/blog/2019/01/22/bloom-filters.html#indexing-time for more details on how expensive this step is. We can safely disable the file hashing feature because it's fine if we get duplicate file watching notifications. Our mtags indexer is fast enough that it's not a big deal if we end up tokenizing the same file twice.
Previously, Metals assumed that the source directories returned by BSP were good approximations for what files we want to file watch. This model works great for build tools like sbt, Maven where the build are structured so that most of the source code lives under a "source directory". This model doesn't work great in build tools like Pants where we have lots of small targets that have individual source files and no directories. With "source roots", build tools like Pants can declare what directories should for example be file watched without us trying to infer from a long list of individual source files.
Previously, the file watcher supported file watching individual files but it did so in a very expensive way creating a new Thread for each watched file. In one large workspace, the file watcher spent 7 minutes getting started. Now, the file watcher only watches directories and build tools that use individual files can use "source roots" if they want to declare what directories to watch.
Previously, this class only supported completing a `cancel=true` value. Now, it's possible to complete with `cancel=false`
Previously, build tools didn't have a way for example to manually register "source root" directories.
Previously, build tools were not able to tell Metals how to handle "no build target" errors. Now, build tools like Pants can register a handler for this situation and gracefully recover from it by calling out to Pants to for example re-expand source globs.
This commit should have been included with the Pants PR but it ended up coming a bit earlier.
Experiments show that this data structure can grow up to 700mb for large workspaces. In comparison, all the other indexes use <50mb combined!
added 7 commits
December 4, 2019 16:54
This method allows build tools to for example register "source roots"
For large workspaces with large classpaths this results in faster indexing.
Previously, the indexing pipeline would crash if a `*-sources.jar` did not have valid jar contents.
gabro
reviewed
Dec 4, 2019
metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala
Show resolved
Hide resolved
mzarnowski
reviewed
Dec 5, 2019
metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala
Outdated
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/FileWatcher.scala
Outdated
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
Show resolved
Hide resolved
mtags/src/main/scala/scala/meta/internal/mtags/MtagsEnrichments.scala
Outdated
Show resolved
Hide resolved
mtags/src/main/scala/scala/meta/internal/mtags/MtagsEnrichments.scala
Outdated
Show resolved
Hide resolved
mtags/src/main/scala/scala/meta/internal/mtags/MtagsEnrichments.scala
Outdated
Show resolved
Hide resolved
tgodzik
approved these changes
Dec 5, 2019
| file.toURI.toString | ||
| } | ||
| def isBuild: Boolean = | ||
| file.filename.startsWith("BUILD") |
Contributor
There was a problem hiding this comment.
Nevermind, found Marek's comment 😅
mzarnowski
approved these changes
Dec 5, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To make it easier to code review the changes in #935 adding Pants support, this PR adds all the necessary non-Pants related changes to unblock that PR. See individual commits for explanations.