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

Don't give a local compiler when disallowed #1762

Merged
merged 1 commit into from
Jul 20, 2022

Conversation

Duhemm
Copy link
Collaborator

@Duhemm Duhemm commented Jul 19, 2022

Previously, Bloop's CompilerCache could return instances of
JavaCompiler relying on using the local JDK Java compiler API
("unforked" compilation) in cases where the javac options included
options expected to be passed to the runtime system (options starting
with -J, eg. -J-Dfoo=bar).

This happened, because the compiler instances are cached and the path to
the javac binary was used as cache key. This means that if 2 projects
A and B share the same path to the javac binary, but only project B
has javac options that should be passed to the runtime system, then
Bloop could use a local compiler to compile project B, because the cache
would have been populated with the local compiler used to compile
project A. See the test cases included in this commit.

This commit fixes this issue by encoding in the cache key whether a
local compiler can be used.

Previously, Bloop's `CompilerCache` could return instances of
`JavaCompiler` relying on using the local JDK Java compiler API
("unforked" compilation) in cases where the javac options included
options expected to be passed to the runtime system (options starting
with `-J`, eg. `-J-Dfoo=bar`).

This happened, because the compiler instances are cached and the path to
the `javac` binary was used as cache key. This means that if 2 projects
A and B share the same path to the `javac` binary, but only project B
has javac options that should be passed to the runtime system, then
Bloop could use a local compiler to compile project B, because the cache
would have been populated with the local compiler used to compile
project A. See the test cases included in this commit.

This commit fixes this issue by encoding in the cache key whether a
local compiler can be used.
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tgodzik tgodzik merged commit 02877e3 into scalacenter:main Jul 20, 2022
@Duhemm Duhemm deleted the forked-compiler branch July 26, 2022 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants