Continue the bsp implementation #431
Merged
Conversation
This file contains 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
jvican
added
integrations
build server
Reuse the new infrastructure using Bloop loggers via Slf4jAdapter. On top of that, in case there's an error in a BSP test case, we report *all* the logs reported by both the client and the server to help diagnose problems.
Yay! Goodbye XML.
jvican
force-pushed
the
topic/prepare-bsp-for-intellij
branch
from
c3a73b1
to
49b40dc
Compare
|
The latest additions to this PR also implement |
In Windows, the order was different than in Linux and was picking up `root`, which has no sources and therefore can never compile and output 'Done compiling'.
Read gmethvin/directory-watcher#12 for all the context.
jvican
changed the title
(including the failed ones). This is what this commit does: 1. `ResultsCache` no longer stores the successful previous results. Now it also stores the `Compiler.Result`s, so that a state that has been compiled allows us to derive summaries and now how the compilation was. This includes being able to tell the user why a given project was not compiled (because it was blocked by other ones). This is important to return a `CompileReport` in bsp-land, but that's implemented in another PR. 2. The `Result` ADT is moved to backend, `Success` now takes the reporter too (to check how many error or warnings were outputted to the user's log), and a reporter is derived from the source infos of previous Zinc results. 3. The `Result` ADT no longer takes the project as a paramter, instead we get the project from the `toCompileTask` in `Tasks.scala`.
jvican
force-pushed
the
topic/prepare-bsp-for-intellij
branch
from
ddc363d
to
76968fe
Compare
Frontend is becoming bloated and reporter utils have no specific dependency in frontend, therefore we're moving them to backend.
jvican
force-pushed
the
topic/prepare-bsp-for-intellij
branch
from
cbb66db
to
0d5ffd5
Compare
BSP clients needed to escape the ansii color codes manually. In order to avoid this unnecessary work, we default to no ansii color codes for now. In the future, we will enrich the spec with a flag to toggle ansii color codes.
jvican
force-pushed
the
topic/prepare-bsp-for-intellij
branch
from
d2fc438
to
decd99b
Compare
rberenguel
reviewed
Apr 11, 2018
| @@ -30,4 +32,8 @@ object AbsolutePath { | |||
| def apply(path: String)(implicit cwd: AbsolutePath): AbsolutePath = apply(NioPaths.get(path))(cwd) | |||
| def apply(path: Path)(implicit cwd: AbsolutePath): AbsolutePath = | |||
| if (path.isAbsolute) new AbsolutePath(path) else cwd.resolve(path.toString) | |||
|
|
|||
| // Necessary to test wrong paths in tests... | |||
| private[bloop] def completelyUnsafe(path: String): AbsolutePath = | |||
There was a problem hiding this comment.
Wouldn't unsafe or toUnsafe sound a bit less scary?
There was a problem hiding this comment.
It's meant to be super scary, so that anyone but me uses it 😛 This makes an absolute path out of something it's not an absolute path.
There was a problem hiding this comment.
Haha got it, didn't think it was that scary
rberenguel
reviewed
Apr 11, 2018
| Right(CompileReport(items)) | ||
| def compile(params: CompileParams): BspResponse[CompileReport] = { | ||
| def compile(projects: Seq[ProjectMapping]): BspResponse[CompileReport] = { | ||
| // TODO(jvican): Naive approach, we need to implement batching here. |
There was a problem hiding this comment.
Batching in what sense? Isn't this fold kind of, going through a batch? Or am I misunderstanding the Run-thing?
There was a problem hiding this comment.
Batching in the sense that bloop needs to find out the smallest group of disjoint DAGs that includes a given list of nodes. This is for performance reasons, a very simple example is described in #217.
There was a problem hiding this comment.
Oh I see. The problem somehow sounds familiar...
See the Scaladoc in `Dag.reduce`. This will allow us to implement #217 efficiently.
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.
This pull request adds several compilation tests that check the correct
compilation of a target, as well as the potential errors showing up from an
invalid URI format.
It also improves the logging internals of BSP: now, logback is not used
anymore, and Bloop has its own slf4j logger adapter to wrap with scalalogging
and passes it in the lsp4s internals.
This pull request also fixes the handling of the root URI: before, it had to be
configuration directory
.bloop, but that was wrong since the root URI must bethe base directory of the workspace. Now the detection of the configuration
directory is done automatically by Bloop.
/cc @jastice