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.

Sign up for GitHub

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

Jump to bottom

Bloop compile to correct JVM version #217

Merged
merged 3 commits into from
Oct 28, 2021
Merged

Bloop compile to correct JVM version #217

merged 3 commits into from
Oct 28, 2021

Conversation

tpasternak
Copy link
Contributor

No description provided.

@tpasternak tpasternak force-pushed the bloop-compiles-against-right-jvm branch from cbacb77 to 94d283c Compare October 14, 2021 15:54
@tpasternak
Copy link
Contributor Author

I'd like to somehow cleaup and organize this code now. I have no idea what's the right way.

Should I just extend acceptBloopVersion field function in BloopRifleScala in that way, it checks also bloop's jvm and restarts it if it's invalid?

Should I download a newer jvm via couriser lib and spawn bloop with it in that case in Operations.startServer?

@alexarchambault
Copy link
Contributor

alexarchambault commented Oct 15, 2021 •
edited
Loading

Should I just extend acceptBloopVersion field function in BloopRifleScala in that way, it checks also bloop's jvm and restarts it if it's invalid?

Something along those lines, yes. Maybe by adding a field like acceptJvm: Option[String => Boolean], that would return true if the passed JVM version is valid, or false to switch JVM (switching to using the java command provided by BloopRifleConfig.javaPath, so there should be no new need to initiate downloading a JVM, it's already done in SharedOptions.bloopRifleConfig()).

@tpasternak tpasternak force-pushed the bloop-compiles-against-right-jvm branch 2 times, most recently from ca2c15e to 4ca8668 Compare October 21, 2021 15:58
ckipp01 added a commit to ckipp01/scala-cli that referenced this pull request Oct 22, 2021
@ckipp01
fix: stick to a tcp connection for Bloop on FreeBSD.
0613dba 
I'll be honest, I haven't really dug into the JNI/JNA issues, so I'm
unsure if there is a better more appropriate to get domain sockets to
work on FreeBSD in this scenario, but when testing it with --bloop-bsp-protocol
being set to tcp it worked. So this is just a quick fix to ensure that the
default for FreeBSD for now is tcp.

Closes VirtusLab#217
@ckipp01 ckipp01 mentioned this pull request Oct 22, 2021
Merged
@tpasternak tpasternak force-pushed the bloop-compiles-against-right-jvm branch from 2b0b45f to 474572f Compare October 26, 2021 12:17
@tpasternak
Copy link
Contributor Author

Still known limitation: no way to enforce JVM
If you set JVM 11, it is guaranteed that the bloop will be able to compile Java 11 API code, but there's no way to strictly set JVM version/path

@tpasternak tpasternak force-pushed the bloop-compiles-against-right-jvm branch from 738d02d to ec68d63 Compare October 26, 2021 14:40
@tpasternak tpasternak changed the title [wip] Bloop compile to correct JVM version Bloop compile to correct JVM version Oct 26, 2021
@tpasternak tpasternak marked this pull request as ready for review October 26, 2021 14:50
romanowski
romanowski requested changes Oct 26, 2021
View reviewed changes
modules/bloop-rifle/src/main/scala/scala/build/bloop/BloopServer.scala Outdated Show resolved Hide resolved
modules/bloop-rifle/src/main/scala/scala/build/bloop/VersionOps.scala Outdated Show resolved Hide resolved
modules/bloop-rifle/src/main/scala/scala/build/bloop/VersionOps.scala Outdated Show resolved Hide resolved
modules/bloop-rifle/src/main/scala/scala/build/blooprifle/BloopRifle.scala Outdated Show resolved Hide resolved
modules/bloop-rifle/src/main/scala/scala/build/blooprifle/BloopRifle.scala Outdated Show resolved Hide resolved
modules/bloop-rifle/src/main/scala/scala/build/blooprifle/BloopRifle.scala Outdated Show resolved Hide resolved
modules/bloop-rifle/src/main/scala/scala/build/blooprifle/BloopRifle.scala Outdated Show resolved Hide resolved
project/deps.sc Outdated Show resolved Hide resolved
@tpasternak tpasternak marked this pull request as draft October 26, 2021 16:49
@tpasternak tpasternak changed the title Bloop compile to correct JVM version [wip] Bloop compile to correct JVM version Oct 26, 2021
@tpasternak tpasternak assigned tpasternak and unassigned tpasternak Oct 26, 2021
@tpasternak tpasternak force-pushed the bloop-compiles-against-right-jvm branch from fe1cd83 to 3edc01e Compare October 27, 2021 18:49
@tpasternak
Ensure Bloop is capable of compiling code using certain JVM api
52cc2ce 
For example the code may call java.util.Optional.isEmpty, which
came in JVM11. If bloop has been spawned under JDK8, it uses JDK8's
`javac` command which does not allow using this function.

On the other hand execution `scala-cli --jvm 11`, if bloop is newer
could lead to a false positive compilation result. Therefore we now
use release flags in scalac/javac to manipulate api restrictions.
@tpasternak tpasternak force-pushed the bloop-compiles-against-right-jvm branch from 3edc01e to 52cc2ce Compare October 27, 2021 20:16
@tpasternak tpasternak changed the title [wip] Bloop compile to correct JVM version Bloop compile to correct JVM version Oct 28, 2021
@tpasternak tpasternak marked this pull request as ready for review October 28, 2021 09:46
@romanowski romanowski merged commit baa9825 into VirtusLab:master Oct 28, 2021
@romanowski romanowski mentioned this pull request Nov 8, 2021
Closed
@tpasternak tpasternak mentioned this pull request Nov 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanowski romanowski romanowski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tpasternak @alexarchambault @romanowski