-
Notifications
You must be signed in to change notification settings - Fork 204
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
Support runtime JDK config #1387
Conversation
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 minor code style comments. Otherwise looks great! Thank you for implementing this new feature 😄
"""/a/A.scala | ||
|object A { | ||
| def main(args: Array[String]): Unit = { | ||
| assert("goodbye" == sys.props("test")) |
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.
It might be tricky to implement, but how hard would it be to assert that the runtime property is not available at compile-time?
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.
It would be difficult to test, because we fork compilation only for Java, and under certain conditions only (ie. the Java home is different from the current Java). I guess we'd need a Java annotation processor that would check for the property?
f03f701
to
bbf43b9
Compare
Previously, Bloop would support only a single JDK configuration per project, which would apply to both compile-time and runtime. However, some tools (e.g. Pants) support defining a different JDK configuration for compile-time and runtime. To be able to export faithfully builds from these tools, Bloop needs to support this kind of configuration as well. This commit adds a new field, `runtimeConfig`, to JVM projects' `platform` configuration, which has the same schema as the existing `config`. The `config` is used to configure the compile-time configuration, whereas `runtimeConfig` is used for runtime. When `runtimeConfig` doesn't exist, Bloop falls back to `config`.
bbf43b9
to
266245b
Compare
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.
LGTM!
Previously, Bloop would support only a single JDK configuration per
project, which would apply to both compile-time and runtime. However,
some tools (e.g. Pants) support defining a different JDK configuration
for compile-time and runtime.
To be able to export faithfully builds from these tools, Bloop needs to
support this kind of configuration as well. This commit adds a new
field,
runtimeConfig
, to JVM projects'platform
configuration, whichhas the same schema as the existing
config
.The
config
is used to configure the compile-time configuration,whereas
runtimeConfig
is used for runtime. WhenruntimeConfig
doesn't exist, Bloop falls back to
config
.