-
Notifications
You must be signed in to change notification settings - Fork 128
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
Warn if jvm api cannot be used #447
Warn if jvm api cannot be used #447
Conversation
7f9b56d
to
482a670
Compare
If a JVM is requested via `--jvm` command, or via `using java-home` directive, the code is still compiled against Bloop's host jvm. If it's too old, the user should be warned.
482a670
to
a5534e9
Compare
e67dcbf
to
9f43d15
Compare
val bloopJvmV = options.javaOptions.bloopJvmVersion | ||
val javaHome = options.javaHome() | ||
if (bloopJvmV.exists(javaHome.value.version > _.value)) { | ||
logger.log(List(scala.build.errors.Diagnostic( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT
logger.log(List(scala.build.errors.Diagnostic( | |
logger.log(List(Diagnostic( |
val red = Console.RED | ||
val yellow = Console.YELLOW | ||
val reset = Console.RESET | ||
val prefix = | ||
if (severity == Severity.Error) s"[${red}error$reset] " | ||
else s"[${yellow}warn$reset] " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT Factor those as private fields in object ConsoleBloopBuildClient
?
if (severity == Severity.Error) s"[${red}error$reset] " | ||
else s"[${yellow}warn$reset] " | ||
val msgIt = message.linesIterator | ||
out.println(s"${prefix}bloop:" + (if (msgIt.hasNext) " " + msgIt.next() else "")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a "bloop"
here?
for (line <- msgIt) | ||
out.println(prefix + line) | ||
|
||
for { pos <- positions } pos match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT Syntax
for { pos <- positions } pos match { | |
positions.foreach { |
scalaOptions = ScalaOptions(scalacOptions = scalacOptions) | ||
) | ||
|
||
val inputs = new Inputs(Nil, None, os.pwd, "project", false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT
val inputs = new Inputs(Nil, None, os.pwd, "project", false) | |
val inputs = Inputs(Nil, None, os.pwd, "project", false) |
(scalaCompilerOptions, res.right.get.javacOptions, logger.diagnostics) | ||
} | ||
|
||
def jvm(v: Int) = os.proc("cs", "java-home", "--jvm", s"zulu:$v").call().out.text().trim() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, we should use the same value as in the ITs (just "cs"
is a problem when the launcher is cs.bat
on Windows, but not when it's cs.exe
, IIRC).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Only left minor NIT comments.
modules/build/src/main/scala/scala/build/ConsoleBloopBuildClient.scala
Outdated
Show resolved
Hide resolved
modules/build/src/test/scala/scala/build/tests/BuildProjectTests.scala
Outdated
Show resolved
Hide resolved
modules/build/src/test/scala/scala/build/tests/BuildProjectTests.scala
Outdated
Show resolved
Hide resolved
Co-authored-by: Alexandre Archambault <alexarchambault@users.noreply.github.com>
No description provided.