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

@Produce(EmptyBuildItem.class) not working #27314

Closed
Sgitario opened this issue Aug 16, 2022 · 36 comments · Fixed by #27427
Closed

@Produce(EmptyBuildItem.class) not working #27314

Sgitario opened this issue Aug 16, 2022 · 36 comments · Fixed by #27427
Labels
kind/bug Something isn't working
Milestone

Comments

@Sgitario
Copy link
Contributor

Describe the bug

As suggested in a comment, when a build step is not producing any build item, we can annotate with @Produce(EmptyBuildItem.class) and the build step is going to be invoked always. See line

+ " Use @Produce(EmptyBuildItem.class) if you want to always execute this step.");
.

However, this is not working for me. See reproducer.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

  1. git clone https://github.com/quarkiverse/quarkus-helm
  2. cd quarkus-helm
  3. git branch avoid_both_extensions
  4. mvn clean install

The build fails because the resources are not generated (if @produce(EmptyBuildItem.class) would work, the resources would be generated (see main branch).

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@Sgitario Sgitario added the kind/bug Something isn't working label Aug 16, 2022
@Sgitario
Copy link
Contributor Author

cc @gsmet @mkouba

@mkouba
Copy link
Contributor

mkouba commented Aug 16, 2022

Hm, I'm still not sure what's going on but I noticed that your build step is annotated with @BuildStep(onlyIf = { HelmEnabled.class, IsNormal.class }) -> IsNormal.class means that it should not be executed in the dev mode or during tests...

@Sgitario
Copy link
Contributor Author

Hm, I'm still not sure what's going on but I noticed that your build step is annotated with @BuildStep(onlyIf = { HelmEnabled.class, IsNormal.class }) -> IsNormal.class means that it should not be executed in the dev mode or during tests...

Yes, that's intended. I only want to be executed during the build.

@mkouba
Copy link
Contributor

mkouba commented Aug 17, 2022

Ok, so I believe that the error message in the BuildStepBuilder is misleading because the EmptyBuildItem was not designed to work this way. The docs is clear that "Empty build items are final (usually empty) classes which extend io.quarkus.builder.item.EmptyBuildItem.", i.e. @Produce(EmptyBuildItem.class) does not a cause build step to be executed.

@yrodiere I think that the sentence "Use @Produce(EmptyBuildItem.class) if you want to always execute this step." should be removed. We should also throw an exception if @Produce(EmptyBuildItem.class) is used. Finally, we could introduce a new build item that extends EmptyBuildItem.class and is always consumed so that it could be used the way @Produce(EmptyBuildItem.class) is currently described.

@yrodiere
Copy link
Member

yrodiere commented Aug 17, 2022

@mkouba Ok, it seems I misunderstood that part, indeed (and I'm not the only one as I got the advice to use @Produces(EmptyBuildItem.class) from someone else).

Your plan makes sense, but I have a question first: is there any reason to forbid using @Produces(EmptyBuildItem.class) and to force people to use a concrete classes extending EmptyBuildItem? Since we know we'll never, ever instantiate that class, what should it matter that the class is abstract? Instead of adding code to throw an exception when we encounter @Produces(EmptyBuildItem.class), couldn't we just add code to support @Produces(EmptyBuildItem.class)?

EDIT: I guess one argument to forbid using @Produces(EmptyBuildItem.class) would be clarity: if we gave the new build item you mentioned a good name, it would make it clear that the only purpose of the @Produces annotation is to make sure the build step is always executed. E.g. @Produces(AlwaysConsumedBuildItem.class) or @Produces(AlwaysExecutedBuildStep.class).

@mkouba
Copy link
Contributor

mkouba commented Aug 17, 2022

I guess one argument to forbid using @Produces(EmptyBuildItem.class) would be clarity: ...

This is actually a very good argument because EmptyBuildItem does not indicate "when" the build step will be executed. It should be probably a final item that will be consumable after the build step chain completes.

@Sgitario
Copy link
Contributor Author

Thanks for your comments, they all make sense to me.
As I really need this, I'm volunteering in working on this, just let me know if I should proceed and any hints are welcome :)

@yrodiere
Copy link
Member

This is actually a very good argument because EmptyBuildItem does not indicate "when" the build step will be executed. It should be probably a final item that will be consumable after the build step chain completes.

That raises an interesting question: do we really want the build step marked with @Produce(AlwaysConsumedBuildItem.class) to always be executed? E.g. I know we somehow consume a particular build item when generating dev services configuration; surely we don't want to execute the build step then?

Maybe the name should be something like ExtensionCompleteBuildItem, to convey the meaning "the extension can only be considered complete once all those build items have been produced"? Then it would make sense to execute various checks in build steps that produce that build item.


I'm volunteering in working on this, just let me know if I should proceed

Gladly, thanks :)

any hints are welcome :)

You will have to:

  • Add a guard here:
    private void addProduces(final ItemId itemId, final Constraint constraint, final ProduceFlags flags) {
    to throw an exception when attempting to produce EmptyBuildItem (if EmptyBuildItem.class.equals(itemId.getType()) { throw ... })
  • Agree on a name for the new build item with @mkouba (I suggest AlwaysConsumedBuildItem or ExtensionCompleteBuildItem, but I'm fine with anything else, really)
  • Add that new build item somewhere (probably in the same package as EmptyBuildItem?)
  • Change the message in
    + " Use @Produce(EmptyBuildItem.class) if you want to always execute this step.");
    to mention your new build item.
  • Make sure that AlwaysConsumedBuildItem is indeed always consumed. @mkouba might have a hint about that; I believe initializing io.quarkus.builder.BuildChainBuilder#finalIds with the id of AlwaysConsumedBuildItem might do the trick.
  • Add one or more tests to io.quarkus.builder.BasicTests
  • Add a test somewhere in integration-tests/test-extension/extension/deployment/src/test. Maybe add a build step with @Produce(AlwaysConsumedBuildItem) to the test extension, have that build step set some static variable, and check in the test that the variable was set?
  • Update the documentation in docs/src/main/asciidoc/writing-extensions.adoc to mention that new build item, probably in the same section that mentions EmptyBuildItem.

@mkouba
Copy link
Contributor

mkouba commented Aug 17, 2022

Let's also CC others to see if it actually makes sense to create such a build item... @dmlloyd @geoand @gsmet @stuartwdouglas @Sanne @aloubyansky?

As I really need this, I'm volunteering in working on this, just let me know if I should proceed and any hints are welcome :)

@Sgitario You can add a BuildProducer param for an existing build item (e.g. FeatureBuildItem or ServiceStartBuildItem) to your build step as a workaround. Note that you don't need to produce a build item at all - quarkus will merely use this information to build the chain.

@yrodiere
Copy link
Member

yrodiere commented Aug 17, 2022

Let's also CC others to see if it actually makes sense to create such a build item... @dmlloyd @geoand @gsmet @stuartwdouglas @Sanne?

FWIW we already have multiple places where we want to execute a build step but don't produce anything. I used @Produce(EmptyBuildItem.class) because I thought that would do the trick, but we will need something to make those work:

@BuildStep
@Produce(EmptyBuildItem.class)
void checkTransactionsSupport(Capabilities capabilities) {
// JTA is necessary for blocking Hibernate ORM but not necessarily for Hibernate Reactive
if (capabilities.isMissing(Capability.TRANSACTIONS)
&& capabilities.isMissing(Capability.HIBERNATE_REACTIVE)) {
throw new ConfigurationException("The Hibernate ORM extension is only functional in a JTA environment.");
}
}

@BuildStep
@Produce(EmptyBuildItem.class)
void validateJsonNeededForHal(Capabilities capabilities,
ResteasyReactiveResourceMethodEntriesBuildItem resourceMethodEntriesBuildItem) {
boolean isHalSupported = capabilities.isPresent(Capability.HAL);
if (isHalSupported && isHalMediaTypeUsedInAnyResource(resourceMethodEntriesBuildItem.getEntries())) {
if (!capabilities.isPresent(Capability.RESTEASY_REACTIVE_JSON_JSONB) && !capabilities.isPresent(
Capability.RESTEASY_REACTIVE_JSON_JACKSON)) {
throw new IllegalStateException("Cannot generate HAL endpoints without "
+ "either 'quarkus-resteasy-reactive-jsonb' or 'quarkus-resteasy-reactive-jackson'");
}
}
}

@gsmet
Copy link
Member

gsmet commented Aug 17, 2022

I think it does make sense to create such a build item given some people were expecting it to work anyway.

As for the name, I think I would prefer AlwaysConsumedBuildItem.

@Sgitario
Copy link
Contributor Author

Let's also CC others to see if it actually makes sense to create such a build item... @dmlloyd @geoand @gsmet @stuartwdouglas @Sanne?

As I really need this, I'm volunteering in working on this, just let me know if I should proceed and any hints are welcome :)

@Sgitario You can add a BuildProducer param for an existing build item (e.g. FeatureBuildItem or ServiceStartBuildItem) to your build step as a workaround. Note that you don't need to produce a build item at all - quarkus will merely use this information to build the chain.

I could not find any workaround to my issue. Previously, I was producing a FeatureBuildItem item and everything worked fine until I started using the OpenShift Container Image extension that needs a JAR build item. Here is where the build chain got crazy and I'm sure the issue is related to how the Kubernetes/OpenShift extensions are written.

Note I tried to use either FeatureBuildItem or ServiceStartBuildItem. So, my only hope was to somehow run my build step after the build is finished.

@mkouba
Copy link
Contributor

mkouba commented Aug 17, 2022

I could not find any workaround to my issue. Previously, I was producing a FeatureBuildItem item and everything worked fine until I started using the OpenShift Container Image extension that needs a JAR build item. Here is where the build chain got crazy and I'm sure the issue is related to how the Kubernetes/OpenShift extensions are written.

Well, that's something that needs to be investigated IMO...

So, my only hope was to somehow run my build step after the build is finished.

I don't think that it's possible.

@Sgitario
Copy link
Contributor Author

Well, that's something that needs to be investigated IMO...

Indeed. I've reported an issue to refactor this: #27313

@Sanne
Copy link
Member

Sanne commented Aug 17, 2022

I'm skeptical about such a thing to exist as it would easily allow to abuse it to shape extensions in imperative style, throwing much of the potential optimisations out of the window.

IMO it's better to have to think a bit more about what actually wants to achieve so to insert the action in the right place of the dependency chain.

But since it exists and we're relying on it... OK with that I guess, but let's try to phase it out?

  1. Make this work
  2. Find code which is relying on it and re-think it (including the ORM code, later..)
  3. Deprecate it

In particular the cases raised by Yoann are interesting and would certainly need some more work; I believe conceptually they could use a dedicated "capabilities integrity phase", but also perhaps they could be better solved when we restructure the dependency chain among these modules.

@mkouba
Copy link
Contributor

mkouba commented Aug 17, 2022

But since it exists and we're relying on it... OK with that I guess, but let's try to phase it out?

It does not exist yet ;-). Some code relies on it but it simply does not work.

@Sanne
Copy link
Member

Sanne commented Aug 17, 2022

But since it exists and we're relying on it... OK with that I guess, but let's try to phase it out?

It does not exist yet ;-). Some code relies on it but it simply does not work.

Right I understand that - it's the title of this issue. I mean it's "conceptually relying on it" - but broken.

BTW another reason to not have it is that it would be fairly easy to deadlock if this "empty thing" gets widely used and misunderstood.

@Sgitario
Copy link
Contributor Author

In spite of I need this to workaround a blocker issue I have on my side, I agree with @Sanne .
However, I'm confused about what is the plan. Either:
a) Totally remove the EmptyBuildItem and do not replace it as like Sanne stated, it's against the Quarkus build philosophy.
b) Or keep EmptyBuildItem as it is (its use case is to manually use @Consume(MyEmptyBuildItem.class) and @Produce(MyEmptyBuildItem.class)) and add a new AlwaysConsumedBuildItem build item that does what the previous EmptyBuildItem supposed to do.
c) Replace EmptyBuildItem by the new AlwaysConsumedBuildItem

By the way, thanks to @yrodiere for your hints! I already added the AlwaysConsumedBuildItem and it worked for me pretty well :)

@mkouba
Copy link
Contributor

mkouba commented Aug 17, 2022

However, I'm confused about what is the plan.

We don't have a plan yet - that's what this discussion is for :-)

Totally remove the EmptyBuildItem and do not replace it as like Sanne stated, it's against the Quarkus build philosophy.

Definitely not. It was designed for a legal (but different) use case.

@yrodiere
Copy link
Member

IMO it's better to have to think a bit more about what actually wants to achieve so to insert the action in the right place of the dependency chain.

Technically, the few use cases I found are just about validation. We could move them around to a build step within the same extension that we know is always executed, but that might lead to cyclic dependency problems if the validation depends indirectly on the output of that build step.

If we provide a better solution, I doubt we'll be able to avoid abuses. But we could "hide" the fact that the new build item is always consumed, by giving it a name that's in relation with the use case (validate the extension), rather than with the solution (always execute the build step).

So... what about naming the build item ExtensionValidatedBuildItem, and consuming it in core, e.g. in io.quarkus.deployment.steps.MainClassBuildStep#build? That's where we consume FeatureBuildItem, so it makes sense IMO.

Or, if we really don't want that, we could just use @Produce(FeatureBuildItem.class) wherever we need to perform validation in an extension, consider it a hack, and not document it. It feels wrong to me, though.

@aloubyansky
Copy link
Member

In my eyes @Produce(EmptyBuildItem.class) (or similar) for a build step that is only consuming looks more like a hack and isn't very intuitive. If we are recognizing and intending to support in some way a use-case of a consuming only build step how about creating an annotation with a more straightforward name e.g. @ConsumingOnlyBuildStep or @BuildStep(consumingOnly=true)?

@Sgitario
Copy link
Contributor Author

Sgitario commented Aug 17, 2022

IMO it's better to have to think a bit more about what actually wants to achieve so to insert the action in the right place of the dependency chain.

Technically, the few use cases I found are just about validation. We could move them around to a build step within the same extension that we know is always executed, but that might lead to cyclic dependency problems if the validation depends indirectly on the output of that build step.

If we provide a better solution, I doubt we'll be able to avoid abuses. But we could "hide" the fact that the new build item is always consumed, by giving it a name that's in relation with the use case (validate the extension), rather than with the solution (always execute the build step).

So... what about naming the build item ExtensionValidatedBuildItem, and consuming it in core, e.g. in io.quarkus.deployment.steps.MainClassBuildStep#build? That's where we consume FeatureBuildItem, so it makes sense IMO.

Or, if we really don't want that, we could just use @Produce(FeatureBuildItem.class) wherever we need to perform validation in an extension, consider it a hack, and not document it. It feels wrong to me, though.

In my use case, I do want to generate resources after the build has finished. This is the exact sequence:

  1. MainClassBuildStep: Consume FeatureBuildItem and produce ApplicationClassNameBuildItem
  2. JarResultBuildStep: Consume ApplicationClassNameBuildItem and produce JarBuildItem
  3. OpenshiftProcessor: Consume JarBuildItem and produce KubernetesProcessor
  4. KubernetesProcessor: Consume KubernetesProcessor and produce GeneratedKubernetesResourceBuildItem

Now, I need a fourth step that consumes GeneratedKubernetesResourceBuildItem and produces nothing:

  1. MyProcessor: Consume GeneratedKubernetesResourceBuildItem and produce nothing (or workaround it somehow)

Nothing I tried it worked for me: either producing either FeatureBuildItem (I can avoid the cycle detected issue, but still not working); or EmptyBuildItem (what this issue is about); or ArtifactResultBuildItem or ServiceStartBuildItem.

The only thing that made it work was to create an AlwaysConsumedBuildItem and consumes it at the end of the build item.

@Sgitario
Copy link
Contributor Author

IMO it's better to have to think a bit more about what actually wants to achieve so to insert the action in the right place of the dependency chain.

Technically, the few use cases I found are just about validation. We could move them around to a build step within the same extension that we know is always executed, but that might lead to cyclic dependency problems if the validation depends indirectly on the output of that build step.
If we provide a better solution, I doubt we'll be able to avoid abuses. But we could "hide" the fact that the new build item is always consumed, by giving it a name that's in relation with the use case (validate the extension), rather than with the solution (always execute the build step).
So... what about naming the build item ExtensionValidatedBuildItem, and consuming it in core, e.g. in io.quarkus.deployment.steps.MainClassBuildStep#build? That's where we consume FeatureBuildItem, so it makes sense IMO.
Or, if we really don't want that, we could just use @Produce(FeatureBuildItem.class) wherever we need to perform validation in an extension, consider it a hack, and not document it. It feels wrong to me, though.

In my use case, I do want to generate resources after the build has finished. This is the exact sequence:

  1. MainClassBuildStep: Consume FeatureBuildItem and produce ApplicationClassNameBuildItem
  2. JarResultBuildStep: Consume ApplicationClassNameBuildItem and produce JarBuildItem
  3. OpenshiftProcessor: Consume JarBuildItem and produce KubernetesProcessor
  4. KubernetesProcessor: Consume KubernetesProcessor and produce GeneratedKubernetesResourceBuildItem

Now, I need a fourth step that consumes GeneratedKubernetesResourceBuildItem and produces nothing:

  1. MyProcessor: Consume GeneratedKubernetesResourceBuildItem and produce nothing (or workaround it somehow)

Nothing I tried it worked for me: either producing either FeatureBuildItem (I can avoid the cycle detected issue, but still not working); or EmptyBuildItem (what this issue is about); or ArtifactResultBuildItem or ServiceStartBuildItem.

The only thing that made it work was to create an AlwaysConsumedBuildItem and consumes it at the end of the build item.

Update: adding ArtifactResultBuildItem made actually it work for me:

@BuildStep(onlyIf = { HelmEnabled.class, IsNormal.class })
    void generateResources(ApplicationInfoBuildItem app, OutputTargetBuildItem outputTarget,
            DekorateOutputBuildItem dekorateOutput,
            List<GeneratedKubernetesResourceBuildItem> generatedResources,
            // this is added to ensure that the build step will be run
            BuildProducer<ArtifactResultBuildItem> dummy) {

I took this dummy code snippet from

@Sgitario
Copy link
Contributor Author

Sgitario commented Aug 18, 2022

To follow up on the discussion, I think the proper solution would be:

  • Keep the EmptyBuildItem and fix the documentation to avoid confusion
  • Provide a new AlwaysConsumedBuildItem for this purpose and use it where it's now used EmptyBuildItem

I think a build item ExtensionValidatedBuildItem is misleading and more open to hacks as we could use it for more things other than validation.
Also, I disagree with doing things like @ConsumingOnlyBuildStep or @BuildStep(consumingOnly=true) as it breaks the current pattern of simple @BuildStep, @Consume, @Produce annotations.
wdyt?

@aloubyansky
Copy link
Member

You mean naming-wise @Sgitario?

@stuartwdouglas
Copy link
Member

io.quarkus.deployment.pkg.builditem.ArtifactResultBuildItem was actually intended for this situation (even though the javadoc is a bit misleading). It is a multibuilditem to represent multiple outputs from the build, while JarResultBuild item is a specific output.

@Sgitario
Copy link
Contributor Author

io.quarkus.deployment.pkg.builditem.ArtifactResultBuildItem was actually intended for this situation (even though the javadoc is a bit misleading). It is a multibuilditem to represent multiple outputs from the build, while JarResultBuild item is a specific output.

If the ArtifactResultBuildItem was designed for this purpose, then the solution is simply to fix the javadoc comment that says we can use the EmptyBuildItem build item to always run a build step and replace it to use ArtifactResultBuildItem?

@mkouba
Copy link
Contributor

mkouba commented Aug 18, 2022

So... what about naming the build item ExtensionValidatedBuildItem, and consuming it in core, e.g. in io.quarkus.deployment.steps.MainClassBuildStep#build? That's where we consume FeatureBuildItem, so it makes sense IMO.

@yrodiere Like @Sgitario mentioned this would not help in his use case.

...by giving it a name that's in relation with the use case (validate the extension), rather than with the solution (always execute the build step).

What we need is effectively a FinalBuildItem which is not a good name though.

If the ArtifactResultBuildItem was designed for this purpose, then the solution is simply to fix the javadoc comment...

I don't think that ArtifactResultBuildItem is a good name for such a build item. Also it's not actually "empty". Moreover, I don't think that Stuart thought that it was designed to solve your particular problem.

@stuartwdouglas
Copy link
Member

It actually is what it was designed to represent: https://github.com/quarkiverse/quarkus-helm/blob/main/deployment/src/main/java/io/quarkiverse/helm/deployment/HelmProcessor.java#L54

Basically every one of these files would be an additional output from the build, so you would create one ArtifactBuildItem per file.

I know in this particular case it does not matter, but build steps are not really supposed to do things as a 'side effect', they should only produce things the build has asked for, and conceptually in this case the build has asked for all the output artifacts, and these are also output artifacts (just not a runnable jar).

@mkouba
Copy link
Contributor

mkouba commented Aug 19, 2022

It actually is what it was designed to represent: https://github.com/quarkiverse/quarkus-helm/blob/main/deployment/src/main/java/io/quarkiverse/helm/deployment/HelmProcessor.java#L54

Basically every one of these files would be an additional output from the build, so you would create one ArtifactBuildItem per file.

I know in this particular case it does not matter, but build steps are not really supposed to do things as a 'side effect', they should only produce things the build has asked for, and conceptually in this case the build has asked for all the output artifacts, and these are also output artifacts (just not a runnable jar).

I see. If that's the case then let's just improve the javadoc and maybe even mention this build step in the docs?

@yrodiere WRT ExtensionValidatedBuildItem - in ArC we have ValidationErrorBuildItem that can be used to validate the deployment before the CDI container is initialized.

@Sgitario
Copy link
Contributor Author

It actually is what it was designed to represent: https://github.com/quarkiverse/quarkus-helm/blob/main/deployment/src/main/java/io/quarkiverse/helm/deployment/HelmProcessor.java#L54
Basically every one of these files would be an additional output from the build, so you would create one ArtifactBuildItem per file.
I know in this particular case it does not matter, but build steps are not really supposed to do things as a 'side effect', they should only produce things the build has asked for, and conceptually in this case the build has asked for all the output artifacts, and these are also output artifacts (just not a runnable jar).

I see. If that's the case then let's just improve the javadoc and maybe even mention this build step in the docs?

+1

@yrodiere
Copy link
Member

@yrodiere Like @Sgitario mentioned this would not help in his use case.

Sure, but that use case is different from the examples that already exist.

@yrodiere WRT ExtensionValidatedBuildItem - in ArC we have ValidationErrorBuildItem that can be used to validate the deployment before the CDI container is initialized.

Thanks for the hint. If the error is reported at build time, then indeed I should probably use that.

@yrodiere
Copy link
Member

yrodiere commented Aug 22, 2022

I see. If that's the case then let's just improve the javadoc and maybe even mention this build step in the docs?

+1

So to sum up, here's our to-do list:

  • 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

@Sgitario do you still want to give this a try, or should I have a look?

@mkouba
Copy link
Contributor

mkouba commented Aug 22, 2022

Use ValidationErrorBuildItem wherever we currently use @Produce(EmptyBuildItem.class) in Quarkus core

It depends whether you're ok with the fact that it's CDI-specific. FTR it's already described here: https://quarkus.io/guides/cdi-integration#use-case-i-need-to-validate-the-deployment

@yrodiere
Copy link
Member

Use ValidationErrorBuildItem wherever we currently use @Produce(EmptyBuildItem.class) in Quarkus core

It depends whether you're ok with the fact that it's CDI-specific.

I am. AFAIK CDI is always included in every Quarkus application and the only places where we currently use @Produce(EmptyBuildItem.class) are for validation, so... Whether it's considered CDI validation or something else doesn't matter, IMO 🤷

FTR it's already described here: https://quarkus.io/guides/cdi-integration#use-case-i-need-to-validate-the-deployment

Okay, so a simple link to that part of the documentation might cut it. I'll update the to-do list.

@Sgitario
Copy link
Contributor Author

I see. If that's the case then let's just improve the javadoc and maybe even mention this build step in the docs?

+1

So to sum up, here's our to-do list:

  • 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

@Sgitario do you still want to give this a try, or should I have a look?

Sure

@quarkus-bot quarkus-bot bot added this to the 2.13 - main milestone Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants