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

Restart Bloop automatically if newer version is needed #1256

Merged
merged 1 commit into from Jan 6, 2020

Conversation

@tgodzik
Copy link
Collaborator

tgodzik commented Jan 6, 2020

Previously, when detected that running build server is using an old Bloop version, we would just display a warning without explaining exactly what to do.

Now, we suggest to restart the Bloop server if Metals uses a newer version and this restart can be requested by the user.

Fixes #1190

@tgodzik tgodzik force-pushed the tgodzik:restart-bloop-server branch from f33a16c to 88a3192 Jan 6, 2020
@tgodzik tgodzik requested a review from olafurpg Jan 6, 2020
System.err,
Shell.default
)
val result = cli.run(Array("ng-stop")) == 0

This comment has been minimized.

Copy link
@olafurpg

olafurpg Jan 6, 2020

Member

What is the difference between ngstop and exit?

This comment has been minimized.

Copy link
@tgodzik

tgodzik Jan 6, 2020

Author Collaborator

I think that is the underlying nailgun command, might be better to use exit. I was testing it previously and it didn't seem to work with exit, but that might have been a different thing.

This comment has been minimized.

Copy link
@tgodzik

tgodzik Jan 6, 2020

Author Collaborator

Switched to exit since it works with it and that is a Bloop command.

…loop version, we would just display a warning without

explaning exactly what to do.
Now, we suggest to restart the Bloop server if Metals uses a newer version and this restart can be requested by the user.
@tgodzik tgodzik force-pushed the tgodzik:restart-bloop-server branch from 88a3192 to 5221982 Jan 6, 2020
(minimumVersion.split('.'), version.split('.')) match {

def splitVersion(v: String) =
v.replaceAll("(-|\\+).+$", "").split('.')

This comment has been minimized.

Copy link
@tgodzik

tgodzik Jan 6, 2020

Author Collaborator

Fixed it to work with 1.4.0-RC etc. and snapshot versions.

@@ -78,7 +78,7 @@ final class TestDebugger(
def shutdown: Future[Unit] = {
for {
_ <- terminated.future
_ <- debugger.shutdown
_ <- debugger.shutdown(60)

This comment has been minimized.

Copy link
@tgodzik

tgodzik Jan 6, 2020

Author Collaborator

The test on windows were timing out and they usually are slower.

@gabro
gabro approved these changes Jan 6, 2020
Copy link
Member

gabro left a comment

LGTM 👏

@ckipp01
ckipp01 approved these changes Jan 6, 2020
Copy link
Member

ckipp01 left a comment

LGTM 👍

Copy link
Member

olafurpg left a comment

LGTM 👍 Thank you @tgodzik for fixing this unblocking the last issue for the coming mega-release 😄

@@ -1653,6 +1658,26 @@ class MetalsLanguageServer(
.foreach(_ => languageClient.refreshModel())
}

private def checkRunningBloopVersion(bspServerVersion: String) = {

This comment has been minimized.

Copy link
@olafurpg

olafurpg Jan 6, 2020

Member

nit: should we move this into a separate file?

This comment has been minimized.

Copy link
@tgodzik

tgodzik Jan 6, 2020

Author Collaborator

I think we should do it as a larger refactor of the file. I can see a number of similar methods that might be put there.

@tgodzik tgodzik merged commit 80efe9d into scalameta:master Jan 6, 2020
12 checks passed
12 checks passed
windows-latest jdk-11 unit tests
Details
macOS-latest jdk-11 unit tests
Details
ubuntu-latest jdk-8 unit tests
Details
ubuntu-latest jdk-11 unit tests
Details
Sbt integration
Details
Maven integration
Details
Gradle integration
Details
Mill integration
Details
Pants integration
Details
LSP integration tests
Details
Scala cross tests
Details
Scalafmt/Scalafix/Docs
Details
@tgodzik tgodzik deleted the tgodzik:restart-bloop-server branch Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.