Replace ApplicationIndexBuildItem with CombinedIndexBuildItem where it makes sense

[aloubyansky]

.. and search for resources in all the application archives instead of only in the root one

I am not sure whether it all makes sense. I'm doing some refactoring and these changes did help.

[gsmet]

Let’s not rush this thing in. We need to be very careful about it as we did the opposite change in various places a few months ago.

[aloubyansky]

I'd like to clarify though whether it makes sense or not pretty much asap.

[gsmet]

@mkouba any chance you could have a look at that one? IIRC, one issue is that CombinedIndexBuildItem will not contain the generated classes or the result of the transformations so we need to be sure it's safe to use it.

[mkouba]

@gsmet ApplicationIndexBuildItem does not contain the generated classes and the result of the transformations either. I can take a look. But in general, it always depends on whether an extension needs to include the classes from other modules and third-party libs.

[mkouba]

@geoand do you remember the reason why class-based @ConfigProperties requied the ApplicationIndexBuildItem? I think it's safe to use the CombinedIndexBuildItem here...

[geoand]

Yeah, it's probably just an oversight on my part

[aloubyansky]

But in general, it always depends on whether an extension needs to include the classes from other modules and third-party libs.

Right, that's what this change is about.

[mkouba]

@FroMage modelClasses are built from the CombinedIndexBuildItem so it should be safe to use it here as well, or?

[FroMage]

That would probably solve the bug where people access entities outside the root application, but might be costly since we still don't have any way to index users of specific classes.

[aloubyansky]

@mkouba how about this?

+++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java
@@ -56,6 +56,7 @@ import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem;
 import io.quarkus.deployment.builditem.ApplicationClassPredicateBuildItem;
 import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem;
 import io.quarkus.deployment.builditem.CapabilityBuildItem;
+import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
 import io.quarkus.deployment.builditem.ExecutorBuildItem;
 import io.quarkus.deployment.builditem.FeatureBuildItem;
 import io.quarkus.deployment.builditem.GeneratedClassBuildItem;
@@ -78,7 +79,7 @@ import io.quarkus.runner.bootstrap.BootstrapDebug;
  * <li>{@link ValidationPhaseBuildItem}</li>
  * </ol>
  * These build items are especially useful if an extension needs to produce other build items within the given phase.
- * 
+ *
  * @see BeanProcessor
  */
 public class ArcProcessor {
@@ -115,7 +116,8 @@ public class ArcProcessor {
             Optional<TestClassPredicateBuildItem> testClassPredicate,
             Capabilities capabilities,
             BuildProducer<FeatureBuildItem> feature,
-            CustomScopeAnnotationsBuildItem scopes) {
+            CustomScopeAnnotationsBuildItem scopes,
+            CombinedIndexBuildItem combinedIndex) {
 
         if (!arcConfig.isRemoveUnusedBeansFieldValid()) {
             throw new IllegalArgumentException("Invalid configuration value set for 'quarkus.arc.remove-unused-beans'." +
@@ -128,7 +130,7 @@ public class ArcProcessor {
         Set<DotName> generatedClassNames = beanArchiveIndex.getGeneratedClassNames();
         IndexView index = beanArchiveIndex.getIndex();
         BeanProcessor.Builder builder = BeanProcessor.builder();
-        IndexView applicationClassesIndex = applicationArchivesBuildItem.getRootArchive().getIndex();
+        IndexView applicationClassesIndex = combinedIndex.getIndex();

This allows to pickup beans from all the application archives instead of just from the root one.

[mkouba]

This allows to pickup beans from all the application archives instead of just from the root one.

@aloubyansky ArC is using a special index build in the BeanArchiveProcessor and it does pick up classes from all app archives. CombinedIndexBuildItem is not the way to go.

[aloubyansky]

Ok, the change I pasted does fix the issue though. So it's just not a proper index then.

[aloubyansky]

This did work

-        IndexView applicationClassesIndex = applicationArchivesBuildItem.getRootArchive().getIndex();
+        IndexView applicationClassesIndex = beanArchiveIndex.getIndex();

Thanks.

[mkouba]

@aloubyansky Hm, I don't think we should use the BeanArchiveIndex for this predicate. It should only return true for application classes, i.e. classes from the root and possibly other modules. Actually, it's a bit more complicated, because it includes ApplicationClassPredicateBuildItem and others. What's the issue anyway?

On the other hand, it may not be needed after the class loading changes... @stuartwdouglas Any idea?

[aloubyansky]

The issue I'm fixing is a use-case of io.quarkus.arc.test.remove.RemoveFwkBeansTest but when UnusedBean is not in the root app archive. Would that be a completely test?

[aloubyansky]

Looks like it broke the TS. I'll keep looking into it.

[aloubyansky]

I rolled that one back.

[gsmet]

Got approval from various people on Zulip.

Let's merge it early.