Clarify ValidationErrorBuildItem/ArtifactResultBuildItem/EmptyBuildItem #27427

Sgitario · 2022-08-23T07:12:02Z

Use ValidationErrorBuildItem wherever we currently use @Produce(EmptyBuildItem.class) in Quarkus core
Update the error message for build steps that don't produce anything to remove the mention of @Produce(EmptyBuildItem.class)
Add a check to make the build fail whenever a build step uses @Produce(EmptyBuildItem.class) (or maybe more generally when it consumes/produces an abstract class, since that simply cannot work?)
Improve the javadoc of ArtifactBuildItem
Update the documentation ("Writing extensions") to mention ArtifactBuildItem and explain when it's used.
Update the documentation ("Writing extensions") to mention ValidationErrorBuildItem and explain when it's used; include a link to https://quarkus.io/guides/cdi-integration#use-case-i-need-to-validate-the-deployment .
Maybe, update the error message for build steps that don't produce anything to mention ArtifactBuildItem/ValidationErrorBuildItem

quarkus-bot · 2022-08-23T07:12:20Z

Sgitario · 2022-08-23T07:13:19Z

@yrodiere I think I've already done all the bullets from the description, but the one about to update the javadocs of ArtifactResultBuildItem because I was not sure what else to describe there.

docs/src/main/asciidoc/writing-extensions.adoc

yrodiere

Thanks for taking care of this!

It looks good to me, except for a little something in the documentation, see below.

but the one about to update the javadocs of ArtifactResultBuildItem because I was not sure what else to describe there

I don't know either, to be honest. It looks fine to me as it is. That item in the to-do list was referring to the discussion between @stuartwdouglas and @mkouba. Unless they chime in to tell us what kind of improvement they wanted in this javadoc, I'd leave it alone.

docs/src/main/asciidoc/writing-extensions.adoc

Sgitario · 2022-08-23T12:46:04Z

I've just updated the section you mentioned. Let me know if it looks better now.

yrodiere

Ah, but... I didn't mean we should remove all the documentation your added, just that one specific change! The sections about ValidationErrorBuildItem/ArtifactResultBuildItem were great...

I hope you can still find your commit in git reflog?

docs/src/main/asciidoc/writing-extensions.adoc

Sgitario · 2022-08-23T19:27:37Z

Ah, but... I didn't mean we should remove all the documentation your added, just that one specific change! The sections about ValidationErrorBuildItem/ArtifactResultBuildItem were great...

I hope you can still find your commit in git reflog?

ah ok! I've recovered it back :)

Sgitario · 2022-08-24T06:33:29Z

...s/deployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java

@@ -85,14 +85,15 @@ AdditionalBeanBuildItem registerRestLinksProviderProducer() {
    }

    @BuildStep
-    @Produce(EmptyBuildItem.class)
+    @Produce(ArtifactResultBuildItem.class)


I could not use the ValidationErrorBuildItem here because it was causing cycle detected issues. Using ArtifactResultBuildItem did work here.

yrodiere

LGTM, thanks!

docs/src/main/asciidoc/writing-extensions.adoc

mkouba

Looks good.

Fix quarkusio#27314

Requested changes have been addressed

quarkus-bot · 2022-08-26T18:12:23Z

Failing Jobs - Building `0876202`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✔️	JVM Tests - JDK 17
✖	JVM Tests - JDK 18	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 18 #

- Failing: integration-tests/micrometer-prometheus

📦 integration-tests/micrometer-prometheus

✖ io.quarkus.it.micrometer.prometheus.ClientRequestTest.testClientRequests line 36 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

yrodiere · 2022-08-29T06:42:53Z

The failing test is unrelated; merging. Thanks!

gsmet · 2022-09-05T13:52:34Z

@Sgitario I won't backport this because Kogito released a version with the EmptyBuildItem: kiegroup/kogito-runtimes@587672e .

Any chance you could prepare a commit for kogito-runtimes that they could include in the next version? And backport if they can.

yrodiere · 2022-09-05T13:58:58Z

Any chance you could prepare a commit for kogito-runtimes

To clarify, the commit needs to replace @Produce(EmptyBuildItem.class) (which doesn't work and will lead to an exception after we backport this PR) with @Produce(ArtifactResultBuildItem.class) (which does work).

In next versions of Quarkus, it won't allow using `@Produce(EmptyBuildItem.class)` (an exception will be thrown). More information about this change can be found here: quarkusio/quarkus#27427 Moreover, for the use case you were using `@Produce(EmptyBuildItem.class)`, validations, we should use `ValidationErrorBuildItem`. I didn't do it because it would require adding the dependency `quarkus-arc-deployment`, so I replaced it with `@Produce(ArtifactResultBuildItem.class)` which is guaranteed that the build step will be executed.

Sgitario · 2022-09-06T06:02:10Z

@Sgitario I won't backport this because Kogito released a version with the EmptyBuildItem: kiegroup/kogito-runtimes@587672e .

Any chance you could prepare a commit for kogito-runtimes that they could include in the next version? And backport if they can.

Done: https://github.com/kiegroup/kogito-runtimes/pull/2442

gsmet · 2022-09-06T18:57:55Z

Thanks @Sgitario !

In next versions of Quarkus, it won't allow using `@Produce(EmptyBuildItem.class)` (an exception will be thrown). More information about this change can be found here: quarkusio/quarkus#27427 Moreover, for the use case you were using `@Produce(EmptyBuildItem.class)`, validations, we should use `ValidationErrorBuildItem`. I didn't do it because it would require adding the dependency `quarkus-arc-deployment`, so I replaced it with `@Produce(ArtifactResultBuildItem.class)` which is guaranteed that the build step will be executed. Note that the `process-svg` addon won't validate the check anymore as it was not expected (according to Christiano)

* Replace EmptyBuildItem with ArtifactResultBuildItem In next versions of Quarkus, it won't allow using `@Produce(EmptyBuildItem.class)` (an exception will be thrown). More information about this change can be found here: quarkusio/quarkus#27427 Moreover, for the use case you were using `@Produce(EmptyBuildItem.class)`, validations, we should use `ValidationErrorBuildItem`. I didn't do it because it would require adding the dependency `quarkus-arc-deployment`, so I replaced it with `@Produce(ArtifactResultBuildItem.class)` which is guaranteed that the build step will be executed. Note that the `process-svg` addon won't validate the check anymore as it was not expected (according to Christiano) * Remove messaging required capabilities Co-authored-by: Cristiano Nicolai <570894+cristianonicolai@users.noreply.github.com>

Sgitario requested review from yrodiere and mkouba August 23, 2022 07:12

quarkus-bot bot added area/core area/documentation area/hibernate-orm Hibernate ORM area/persistence area/resteasy-reactive labels Aug 23, 2022

quarkus-bot bot added this to To do in Quarkus Documentation Aug 23, 2022

gsmet previously requested changes Aug 23, 2022

View reviewed changes

docs/src/main/asciidoc/writing-extensions.adoc Outdated Show resolved Hide resolved

Quarkus Documentation automation moved this from To do to Review pending Aug 23, 2022

yrodiere requested changes Aug 23, 2022

View reviewed changes

docs/src/main/asciidoc/writing-extensions.adoc Outdated Show resolved Hide resolved

Sgitario force-pushed the improve_empty_build_item branch from f67c3ef to 2398681 Compare August 23, 2022 12:45

Sgitario requested review from yrodiere and gsmet August 23, 2022 12:45

yrodiere reviewed Aug 23, 2022

View reviewed changes

docs/src/main/asciidoc/writing-extensions.adoc Outdated Show resolved Hide resolved

gsmet added the triage/backport? label Aug 23, 2022

This comment has been minimized.

Sign in to view

Sgitario force-pushed the improve_empty_build_item branch from 87d097d to b58cb65 Compare August 23, 2022 19:27

Sgitario requested a review from yrodiere August 23, 2022 19:27

This comment has been minimized.

Sign in to view

Sgitario force-pushed the improve_empty_build_item branch from b58cb65 to 317b3e1 Compare August 24, 2022 06:32

Sgitario commented Aug 24, 2022

View reviewed changes

yrodiere approved these changes Aug 24, 2022

View reviewed changes

yrodiere added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 24, 2022

mkouba reviewed Aug 24, 2022

View reviewed changes

docs/src/main/asciidoc/writing-extensions.adoc Outdated Show resolved Hide resolved

Sgitario force-pushed the improve_empty_build_item branch from 317b3e1 to aa376ff Compare August 24, 2022 09:45

Sgitario requested a review from mkouba August 24, 2022 09:45

yrodiere approved these changes Aug 24, 2022

View reviewed changes

mkouba approved these changes Aug 24, 2022

View reviewed changes

Clarify ValidationErrorBuildItem/ArtifactResultBuildItem/EmptyBuildItem

0876202

Fix quarkusio#27314

Sgitario force-pushed the improve_empty_build_item branch from aa376ff to 0876202 Compare August 26, 2022 08:24

Quarkus Documentation automation moved this from Review pending to Reviewer approved Aug 26, 2022

This comment has been minimized.

Sign in to view

yrodiere merged commit ec68e68 into quarkusio:main Aug 29, 2022

Quarkus Documentation automation moved this from Reviewer approved to Done Aug 29, 2022

quarkus-bot bot added this to the 2.13 - main milestone Aug 29, 2022

quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Aug 29, 2022

Sgitario deleted the improve_empty_build_item branch August 29, 2022 06:43

Sgitario mentioned this pull request Sep 6, 2022

Replace EmptyBuildItem with ArtifactResultBuildItem apache/incubator-kie-kogito-runtimes#2442

Merged

8 tasks

gsmet removed the triage/backport? label Sep 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify ValidationErrorBuildItem/ArtifactResultBuildItem/EmptyBuildItem #27427

Clarify ValidationErrorBuildItem/ArtifactResultBuildItem/EmptyBuildItem #27427

Sgitario commented Aug 23, 2022

quarkus-bot bot commented Aug 23, 2022

Sgitario commented Aug 23, 2022

yrodiere left a comment

Sgitario commented Aug 23, 2022

yrodiere left a comment

This comment has been minimized.

Sgitario commented Aug 23, 2022

This comment has been minimized.

This comment has been minimized.

Sgitario Aug 24, 2022

yrodiere left a comment

mkouba left a comment

This comment has been minimized.

quarkus-bot bot commented Aug 26, 2022

yrodiere commented Aug 29, 2022

gsmet commented Sep 5, 2022

yrodiere commented Sep 5, 2022 •

edited

Sgitario commented Sep 6, 2022

gsmet commented Sep 6, 2022

Clarify ValidationErrorBuildItem/ArtifactResultBuildItem/EmptyBuildItem #27427

Clarify ValidationErrorBuildItem/ArtifactResultBuildItem/EmptyBuildItem #27427

Conversation

Sgitario commented Aug 23, 2022

quarkus-bot bot commented Aug 23, 2022

Sgitario commented Aug 23, 2022

yrodiere left a comment

Choose a reason for hiding this comment

Sgitario commented Aug 23, 2022

yrodiere left a comment

Choose a reason for hiding this comment

This comment has been minimized.

Sgitario commented Aug 23, 2022

This comment has been minimized.

This comment has been minimized.

Sgitario Aug 24, 2022

Choose a reason for hiding this comment

yrodiere left a comment

Choose a reason for hiding this comment

mkouba left a comment

Choose a reason for hiding this comment

This comment has been minimized.

quarkus-bot bot commented Aug 26, 2022

Failing Jobs - Building 0876202

Failures

⚙️ JVM Tests - JDK 18 #

📦 integration-tests/micrometer-prometheus

yrodiere commented Aug 29, 2022

gsmet commented Sep 5, 2022

yrodiere commented Sep 5, 2022 • edited

Sgitario commented Sep 6, 2022

gsmet commented Sep 6, 2022

Failing Jobs - Building `0876202`

yrodiere commented Sep 5, 2022 •

edited