-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Fix incremental compilation with JDK16 using toolchains #16066
Merged
Merged
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b525a17
Add a test compiling Java using a range of JDKs
rieske 627d7ee
Merge branch 'master' into vv/jdk16/toolchains
rieske 6788132
Bust the cache to rerun test on CI
rieske 56c89f3
Fix class visibility for incremental compiler using JDK16 toolchain
rieske 06f3afb
Extend toolchain compilation test coverage to JDK17
rieske fb8f8d7
Do not use private compiler APIs in incremental compile task
rieske 997da0b
Add integration test for incremental compilation with anonymous classes
rieske 5d1ac35
Merge branch 'master' into vv/jdk16/toolchains
rieske f6e9975
Normalize all nested type elements to binary names
rieske ef5a115
Merge branch 'master' into vv/jdk16/toolchains
rieske b04b5ab
Update deprecation assertions in samples
rieske 34e5a64
Pull out javac task type check to incremental compiler plugin constru…
rieske 4bd0230
Expand test coverage for Java incremental compiler plugin
rieske 03def01
Move anonymous classes incremental compilation tests to Java base tes…
rieske File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,6 +78,7 @@ | |
import javax.annotation.Nullable; | ||
import javax.inject.Inject; | ||
import java.io.File; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.concurrent.Callable; | ||
|
||
|
@@ -254,7 +255,7 @@ private <T extends CompileSpec> Compiler<T> createToolchainCompiler() { | |
|
||
private Provider<JavaCompiler> getCompilerTool() { | ||
JavaToolchainSpec explicitToolchain = determineExplicitToolchain(); | ||
if(explicitToolchain == null) { | ||
if (explicitToolchain == null) { | ||
if(javaCompiler.isPresent()) { | ||
return this.javaCompiler; | ||
} else { | ||
|
@@ -307,7 +308,6 @@ DefaultJavaCompileSpec createSpec() { | |
List<File> sourcesRoots = CompilationSourceDirs.inferSourceRoots((FileTreeInternal) getStableSources().getAsFileTree()); | ||
JavaModuleDetector javaModuleDetector = getJavaModuleDetector(); | ||
boolean isModule = JavaModuleDetector.isModuleSource(modularity.getInferModulePath().get(), sourcesRoots); | ||
boolean toolchainCompatibleWithJava8 = isToolchainCompatibleWithJava8(); | ||
|
||
final DefaultJavaCompileSpec spec = createBaseSpec(); | ||
|
||
|
@@ -323,7 +323,7 @@ DefaultJavaCompileSpec createSpec() { | |
configureCompatibilityOptions(spec); | ||
spec.setSourcesRoots(sourcesRoots); | ||
|
||
if (!toolchainCompatibleWithJava8) { | ||
if (!isToolchainCompatibleWithJava8()) { | ||
spec.getCompileOptions().setHeaderOutputDirectory(null); | ||
} | ||
return spec; | ||
|
@@ -333,6 +333,10 @@ private boolean isToolchainCompatibleWithJava8() { | |
return getCompilerTool().get().getMetadata().getLanguageVersion().canCompileOrRun(8); | ||
} | ||
|
||
private boolean isToolchainCompatibleWithJava16() { | ||
return getCompilerTool().get().getMetadata().getLanguageVersion().canCompileOrRun(16); | ||
} | ||
|
||
@Input | ||
JavaVersion getJavaVersion() { | ||
return JavaVersion.toVersion(getCompilerTool().get().getMetadata().getLanguageVersion().asInt()); | ||
|
@@ -349,6 +353,10 @@ private DefaultJavaCompileSpec createBaseSpec() { | |
private void applyToolchain(ForkOptions forkOptions) { | ||
final JavaInstallationMetadata metadata = getToolchain(); | ||
forkOptions.setJavaHome(metadata.getInstallationPath().getAsFile()); | ||
if (isToolchainCompatibleWithJava16()) { | ||
// used by the incremental compiler plugin, see org.gradle.internal.compiler.java.ClassNameCollector | ||
forkOptions.getJvmArgs().addAll(Arrays.asList("--add-opens", "jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might be worthing looking to actually fixing this without the add opens. I pushed a small WIP here that I played around with, we might not need this workaround for this. WDYT? |
||
} | ||
} | ||
|
||
@Nullable | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Maybe use
#javaVersion
to see which iterations are skipped?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.
We get this information without explicitly embedding the variable in the test header (since Spock 2 I believe): https://ge.gradle.org/s/vhyeam6vqgtqi/tests