Upgrade to Jandex 3.1.5 #36105

Ladicek · 2023-09-22T16:35:25Z

https://smallrye.io/blog/jandex-3-1-5/

Ladicek · 2023-09-22T22:15:10Z

Hmm, tests failed in my fork (and likely also here) in quarkus-junit5-component, which is weird. I'll investigate on Monday. Just FYI.

Ladicek · 2023-09-25T09:53:10Z

Added one commit that fixes ArC's TypeAndQualifiers.hashCode(), CC @mkouba

The TypeAndQualifiers.equals() method is completely custom (because AnnotationInstance.equals() takes into account the target), but the TypeAndQualifiers.hashCode() method blindly delegates to AnnotationInstance (because AnnotationInstance.hashCode() used to ignore the target). That is wrong; AnnotationInstance.equals() and hashCode() must always be consistent, but there has never been a stated contract that the AnnotationInstance.hashCode() will stay consistent with a custom implementation of equality that ignores the target. If someone has custom AnnotationInstance equality, they should have a matching hash code (and the two must be consistent, of course).

Sanne · 2023-09-25T09:57:08Z

Was reading the release blog - there's one thing puzzling me: wouldn't the "target" always be the same for a given annotation? I assume it's referring to @java.lang.annotation.Target , so inherently bound to the identity of the annotation?

I would have thought that in this case skipping the target would have been a valid optimisation; wondering what I'm missing.

Ladicek · 2023-09-25T09:58:53Z

No, the AnnotationInstance.target is the AnnotationTarget of the annotation, or the thing that the annotation is present on. Class, method, field, type usage, things like that.

Sanne · 2023-09-25T10:01:29Z

Ah, the actual runtime instance! Thanks, that explains my confusion.
Great work :)

mkouba · 2023-09-25T10:54:16Z

...endent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/InjectionPointInfo.java

-                    if (!annotationsAreEqual(a1, a2)) {
+            for (AnnotationInstance a1 : s1) {
+                for (AnnotationInstance a2 : s2) {
+                    if (!annotationEquals(a1, a2)) {


And why don't we use AnnotationInstance.equivalentTo(AnnotationInstance) instead?

Because AnnotationInstance doesn't have a corresponding hash code function (yet -- see smallrye/jandex#333). Once AnnotationInstance has both equivalence and equivalence hash code, we remove both annotationEquals and annotationHashCode. Until then, I think it's better to keep both equals and hashCode completely self-sufficient.

In other words, if I switched to AnnotationInstance.equivalentTo(), I would be creating another problem of the same kind that I'm fixing here. Potential problem, that is. The annotationHashCode() implemented in this commit is consistent with AnnotationInstance.equivalentTo(), but there's no stated contract saying that. It would be another dependency on an implementation detail.

Well, the contract of AnnotationInstance#equivalentTo() is pretty clear so I don't see a dependency on an implementation detail. But let's keep it independent if you think it's better...

mkouba · 2023-09-25T11:12:01Z

Added one commit that fixes ArC's TypeAndQualifiers.hashCode(), CC @mkouba

The TypeAndQualifiers.equals() method is completely custom (because AnnotationInstance.equals() takes into account the target), but the TypeAndQualifiers.hashCode() method blindly delegates to AnnotationInstance (because AnnotationInstance.hashCode() used to ignore the target). That is wrong; AnnotationInstance.equals() and hashCode() must always be consistent, but there has never been a stated contract that the AnnotationInstance.hashCode() will stay consistent with a custom implementation of equality that ignores the target. If someone has custom AnnotationInstance equality, they should have a matching hash code (and the two must be consistent, of course).

Makes sense but I wonder if ArC is the only one who relied on this implementation detail of AnnotationInstance#hashCode() 🤔

Ladicek · 2023-09-25T11:21:20Z

That's a good question. Let's see if more tests fail :-)

Ladicek · 2023-09-26T09:10:02Z

The number of failures looks creepy, but I can't reproduce any of them locally and the failures look highly unrelated. Especially the failing Oracle DB tests show errors on the database side ("table or view does not exist", "sequence does not exist", "object SYSTEM.MYPROC is invalid"). Many other PRs fail with the same errors, see e.g. #36131, which suggests this is an infrastructure failure.

The `TypeAndQualifiers` compound is used as a key in hash maps, which means its equality and hash code must be consistent. Unfortunately, the implementation of `equals()` is completely custom, because it needs to ignore `AnnotationInstance.target`, yet the `hashCode()` implementation blindly delegates to `AnnotationInstance`. That used to work, because `AnnotationInstance.hashCode()` ignored the `target` before, but that has never been a stated contract. A completely custom implementation of equality should always be accompanied by a custom implementation of hash code. That's what this commit does.

quarkus-bot · 2023-10-03T17:12:58Z

Failing Jobs - Building `fcbd46d`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✔️	JVM Tests - JDK 17
✖	JVM Tests - JDK 17 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 20
✔️	Maven Tests - JDK 11
✖	Maven Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✖	Native Tests - Virtual Thread - Main	`Build`	⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 Windows #

- Failing: independent-projects/tools/analytics-common 
! Skipped: devtools/cli devtools/gradle/gradle-application-plugin devtools/maven and 219 more

📦 independent-projects/tools/analytics-common

✖ io.quarkus.analytics.AnalyticsServiceTest.sendAnalyticsTest line 160 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a io.quarkus.analytics.AnalyticsServiceTest Expected at least one request matching: {
  "url" : "/v1/track",

✖ io.quarkus.analytics.rest.RestClientFailTest.postIdentityServerTTLExceeded line 87 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a io.quarkus.analytics.rest.RestClientFailTest Expected at least one request matching: {
  "url" : "/v1/identify",

✖ io.quarkus.analytics.rest.RestClientTest.postIdentity line 102 - More details - Source on GitHub

java.util.concurrent.TimeoutException
	at java.base/java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1960)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2095)

⚙️ Maven Tests - JDK 11 Windows #

📦 integration-tests/maven

✖ io.quarkus.maven.it.JarRunnerIT.testPlatformPropertiesOverridenInApplicationProperties line 135 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.JarRunnerIT that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.UberJarQuarkusIntegrationTestIT.testUberJar line 13 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.QuarkusITBase that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.JarRunnerIT.testPlatformPropertiesOverridenInApplicationProperties line 135 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.JarRunnerIT that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.UberJarQuarkusIntegrationTestIT.testUberJar line 13 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.QuarkusITBase that uses io.quarkus.maven.it.verifier.MavenProcessInvocationResult was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

Sanne approved these changes Sep 22, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

Ladicek mentioned this pull request Sep 23, 2023

Bump io.smallrye:jandex-maven-plugin from 3.1.3 to 3.1.5 #36114

Closed

Sanne approved these changes Sep 25, 2023

View reviewed changes

mkouba reviewed Sep 25, 2023

View reviewed changes

mkouba approved these changes Sep 25, 2023

View reviewed changes

mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 25, 2023

This comment has been minimized.

Sign in to view

Ladicek force-pushed the jandex-3.1.5 branch from 3204cb6 to 3579cdc Compare October 2, 2023 07:14

This comment has been minimized.

Sign in to view

Ladicek added 2 commits October 3, 2023 12:04

Upgrade to Jandex 3.1.5

9d27563

Ladicek force-pushed the jandex-3.1.5 branch from 3579cdc to fcbd46d Compare October 3, 2023 11:26

gsmet merged commit eddc83e into quarkusio:main Oct 5, 2023

quarkus-bot bot added this to the 3.5 - main milestone Oct 5, 2023

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 5, 2023

Ladicek deleted the jandex-3.1.5 branch October 5, 2023 12:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Jandex 3.1.5 #36105

Upgrade to Jandex 3.1.5 #36105

Ladicek commented Sep 22, 2023

Ladicek commented Sep 22, 2023

This comment has been minimized.

Ladicek commented Sep 25, 2023

Sanne commented Sep 25, 2023

Ladicek commented Sep 25, 2023

Sanne commented Sep 25, 2023 •

edited

Loading

mkouba Sep 25, 2023

Ladicek Sep 25, 2023

mkouba Sep 25, 2023

mkouba commented Sep 25, 2023

Ladicek commented Sep 25, 2023

This comment has been minimized.

Ladicek commented Sep 26, 2023

This comment has been minimized.

quarkus-bot bot commented Oct 3, 2023

Upgrade to Jandex 3.1.5 #36105

Upgrade to Jandex 3.1.5 #36105

Conversation

Ladicek commented Sep 22, 2023

Ladicek commented Sep 22, 2023

This comment has been minimized.

Ladicek commented Sep 25, 2023

Sanne commented Sep 25, 2023

Ladicek commented Sep 25, 2023

Sanne commented Sep 25, 2023 • edited Loading

mkouba Sep 25, 2023

Choose a reason for hiding this comment

Ladicek Sep 25, 2023

Choose a reason for hiding this comment

mkouba Sep 25, 2023

Choose a reason for hiding this comment

mkouba commented Sep 25, 2023

Ladicek commented Sep 25, 2023

This comment has been minimized.

Ladicek commented Sep 26, 2023

This comment has been minimized.

quarkus-bot bot commented Oct 3, 2023

Failing Jobs - Building fcbd46d

Failures

⚙️ JVM Tests - JDK 17 Windows #

📦 independent-projects/tools/analytics-common

⚙️ Maven Tests - JDK 11 Windows #

📦 integration-tests/maven

Sanne commented Sep 25, 2023 •

edited

Loading

Failing Jobs - Building `fcbd46d`