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

Improve the way we register providers for reflection #42073

Merged

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Jul 23, 2024

Instead of registering all constructors we only need to register the nullary constructor.

Furthermore, we need to register the provider() method for reflective lookup to avoid MissingReflectionRegistrationErrors at run-time when using -H:+ThrowMissingRegistrationErrors or --exact-reachability-metadata.

The said constructor and method are accessed in ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.

Relates to #41995

@zakkak zakkak requested a review from gsmet July 23, 2024 10:11
@zakkak zakkak requested a review from geoand July 23, 2024 10:29
@zakkak zakkak added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 23, 2024
@quarkus-bot

This comment has been minimized.

Instead of registering all constructors we only need to register the
nullary constructor.

Furthermore, we need to register the `provider()` method for reflective
lookup to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

The said constructor and method are accessed in
ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.

Relates to quarkusio#41995
@zakkak zakkak force-pushed the 2024-07-23-rework-providers-registration branch from 4486fa2 to 862bebb Compare July 23, 2024 21:21
@zakkak
Copy link
Contributor Author

zakkak commented Jul 23, 2024

Resolved conflicts.

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 24, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 862bebb.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 integration-tests/kotlin

io.quarkus.kotlin.maven.it.KotlinRemoteDevModeIT.testThatTheApplicationIsReloadedOnKotlinChange - History

  • org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase was not fulfilled within 1 minutes. - java.lang.RuntimeException
java.lang.RuntimeException: org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase was not fulfilled within 1 minutes.
	at io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase.runAndCheck(RunAndCheckWithAgentMojoTestBase.java:86)
	at io.quarkus.kotlin.maven.it.KotlinRemoteDevModeIT.testThatTheApplicationIsReloadedOnKotlinChange(KotlinRemoteDevModeIT.java:23)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)

@gsmet gsmet merged commit ec8e234 into quarkusio:main Jul 24, 2024
52 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 24, 2024
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Jul 24, 2024
@zakkak zakkak deleted the 2024-07-23-rework-providers-registration branch July 24, 2024 07:51
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.

3 participants