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

Workaround to IDEA-301084 #2368

Merged
merged 3 commits into from
Sep 1, 2022
Merged

Workaround to IDEA-301084 #2368

merged 3 commits into from
Sep 1, 2022

Conversation

j-baker
Copy link
Contributor

@j-baker j-baker commented Sep 1, 2022

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.

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.
@changelog-app
Copy link

changelog-app bot commented Sep 1, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Workaround to IDEA-301084

Check the box to generate changelog(s)

  • Generate changelog entry

*/
private static void enablePreviewOnCompileTask(
Provider<ChosenJavaVersion> provider, CompileOptions compileOptions) {
if (provider.get().enablePreview()) {
Copy link
Contributor Author

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?

Copy link
Contributor

@iamdanfox iamdanfox Sep 1, 2022

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.

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor

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 🤦🏻

Copy link
Contributor Author

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.

@bulldozer-bot bulldozer-bot bot merged commit 4ebd7ca into develop Sep 1, 2022
@bulldozer-bot bulldozer-bot bot deleted the jbaker/compiler_args branch September 1, 2022 17:06
@svc-autorelease
Copy link
Collaborator

Released 4.160.0

@iamdanfox
Copy link
Contributor

Unfortunately this seems to not work internally in our normal multi-project repos:

Cannot query the value of this property because it has no value available.
	at org.gradle.api.internal.provider.AbstractMinimalProvider.get(AbstractMinimalProvider.java:85)
	at com.palantir.baseline.plugins.javaversions.BaselineJavaVersions.lambda$apply$0(BaselineJavaVersions.java:66)

iamdanfox added a commit that referenced this pull request Sep 2, 2022
bulldozer-bot bot pushed a commit that referenced this pull request Sep 2, 2022
Revert #2368 (Workaround to IDEA-301084)
bulldozer-bot bot pushed a commit to palantir/witchcraft-api that referenced this pull request Sep 2, 2022
###### _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.
This was referenced Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants