-
Notifications
You must be signed in to change notification settings - Fork 135
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
Workaround to IDEA-301084 #2368
Conversation
See JetBrains/intellij-community#2135 for more details. Even if JetBrains takes this PR, it'll be months before it's common that all devs have a fixed version (it's common for devs to be a version or two behind) so it seems sensible to just workaround in baseline for hte next year or two.
Generate changelog in
|
*/ | ||
private static void enablePreviewOnCompileTask( | ||
Provider<ChosenJavaVersion> provider, CompileOptions compileOptions) { | ||
if (provider.get().enablePreview()) { |
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.
This is eager and not lazy, obviously. @iamdanfox would this be a problem?
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.
The downside of this 'eager' approach is:
If the first line of your build.gradle says apply plugin: 'com.palantir.baseline-java-version'
and then your second line says: javaVersions { runtime = '19_PREVIEW' }
, then we will already have configured this compileOptions
task and not added the --enable-preview
compiler arg.
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.
Yeah this should be lazy. You can use getCompilerArgumentProviders().add
to give an argument provider that can return a list of size 0 or 1 to be lazy.
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.
@CRogers the compilerArgumentProviders approach is what's already on develop, but sadly when you use IntelliJ's native integration, they literally check javaCompileTask.options.compilerArgs.contains("--enable-preview")
which does not invoke my beloved command line argument provider.
James has a fix up here: JetBrains/intellij-community#2135
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.
Yeah I probably should have read that first 🤦🏻
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.
ok folks. I've updated this PR to actually use both methods (lazy and eager). Dan and I think that this will most likely solve the problem in all practical cases because in any case where code is in subprojects, the javaVersions closure will have been set on the root and so evaluated before the java library declaration, avoiding the problem.
However, just in case laziness is an issue (e.g. you have only a root project) this will maintain the fact that --enable-preview is set, with the only downside being it's most likely set twice. This should be a no-op in all cases, should guarantee no semantic changes from present except avoiding the IntelliJ bug.
Released 4.160.0 |
Unfortunately this seems to not work internally in our normal multi-project repos:
|
###### _excavator_ is a bot for automating changes across repositories. Changes produced by the roomba/latest-baseline-oss check. # Release Notes ## 4.154.0 | Type | Description | Link | | ---- | ----------- | ---- | | Improvement | The JUnits reports plugin is no longer applied by default. Test reports now use the standard output locations from Gradle conventions. | palantir/gradle-baseline#2355 | ## 4.155.0 _Automated release, no documented user facing changes_ ## 4.156.0 | Type | Description | Link | | ---- | ----------- | ---- | | Fix | Fix BaselineJavaVersion checkstyle configuration on gradle < 7.5 | palantir/gradle-baseline#2360 | ## 4.157.0 | Type | Description | Link | | ---- | ----------- | ---- | | Improvement | Make task initialization lazier in the `junit-reports` plugin. | palantir/gradle-baseline#2364 | ## 4.158.0 | Type | Description | Link | | ---- | ----------- | ---- | | Fix | Make the `checkUnusedDependencies` tasks added by `baseline-exact-dependencies` compatible with Gradle's configure-on-demand feature. | palantir/gradle-baseline#2363 | ## 4.159.0 | Type | Description | Link | | ---- | ----------- | ---- | | Improvement | Add an errorprone check and typed annotation for Javax -> Jakarta<br><br>There is a certain class of very problematic cases whereby if you have<br>a method such as the following:<br><br>```<br>myJerseyResource.register(/* this is of type Object */ object);<br>```<br><br>Then if you supply a resource which includes any `javax.ws.rs`<br>annotations on it, then those will not be registered if your Jersey<br>version is 3.x or later (and you'll only find this out at runtime).<br><br>The opposite is also true if you try to supply resources annotated<br>with `jakarta.ws.rs` to Jersey 2.x.<br><br>To address this, this commit attempts to add an errorprone check<br>which lets implementors add an annotation `@ForbidJavax` to methods<br>which have been knowingly migrated to Jakarta EE9 and cannot<br>accept legacy javax types. | palantir/gradle-baseline#2366 | ## 4.160.0 | Type | Description | Link | | ---- | ----------- | ---- | | Fix | Workaround to IDEA-301084 | palantir/gradle-baseline#2368 | ## 4.161.0 | Type | Description | Link | | ---- | ----------- | ---- | | Fix | Reverts a change introduced to baseline-java-version 4.160.0, which was causing failures on multi-project builds. | palantir/gradle-baseline#2369 | To enable or disable this check, please contact the maintainers of Excavator.
See JetBrains/intellij-community#2135 for
more details. Even if JetBrains takes this PR, it'll be months
before it's common that our devs have a fixed version (it's common
for devs to be a version or two behind) so it seems sensible to
just workaround in baseline for hte next year or two.