Skip to content

Fix some additional build reproducibility issues in ArC #53088

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.

Sign up for GitHub

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

Open
gsmet wants to merge 1 commit into
base: main
Choose a base branch
from
+36 −15
Open

Fix some additional build reproducibility issues in ArC #53088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ConfigBuildStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -239,6 +240,7 @@ void validateRuntimeConfigProperty(
recorder.validateConfigProperties(
configProperties.stream()
.filter(ConfigPropertyBuildItem::isRuntimeInit)
.sorted(Comparator.comparing(ConfigPropertyBuildItem::getPropertyName))
Copy link
Contributor

@mkouba mkouba Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsmet Are you sure this helps? Because Collectors.toSet() is an unordered collector...

Copy link
Member

@Ladicek Ladicek Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, this itself is useless AFAICT

Copy link
Member Author

@gsmet gsmet Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it helped, I did numerous runs, but I agree it might have been by luck and I need to make the Set an ordered one.

.map(p -> configPropertyToConfigValidation(p, reflectiveClass))
.collect(toSet()));
}
Expand Down
41 changes: 29 additions & 12 deletions ...sions/arc/deployment/src/main/java/io/quarkus/arc/deployment/SyntheticBeansProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package io.quarkus.arc.deployment;

import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
Expand Down Expand Up @@ -38,14 +39,20 @@ public class SyntheticBeansProcessor {
void initStatic(ArcRecorder recorder, List<SyntheticBeanBuildItem> syntheticBeans,
BeanRegistrationPhaseBuildItem beanRegistration, BuildProducer<BeanConfiguratorBuildItem> configurators) {

Map<String, Function<SyntheticCreationalContext<?>, ?>> creationFunctions = new HashMap<>();
Map<String, Supplier<ActiveResult>> checkActiveSuppliers = new HashMap<>();

TreeMap<String, SyntheticBeanBuildItem> sortedBeans = new TreeMap<>();
Copy link
Contributor

@mkouba mkouba Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I don't understand this change. The ordering follows the ordering of List<SyntheticBeanBuildItem> syntheticBeans and the fact that we first store all beans in a sorted map would not make any difference, or?

Copy link
Member

@Ladicek Ladicek Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member Author

@gsmet gsmet Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The List<SyntheticBeanBuildItem> syntheticBeans is not ordered.

And when calling the recorders in random order, I had sometimes some changes in the ordering.

Copy link
Member Author

@gsmet gsmet Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can probably find the diff back to help showing what I was trying to fix but it will require some archeology :)

Copy link
Contributor

@mkouba mkouba Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The List syntheticBeans is not ordered.

Isn't it? I thought that build steps are guaranteed to be executed in the same order 🤔.

Copy link
Member Author

@gsmet gsmet Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, they are not guaranteed to run in the same order because of parallelism. But that's not the problem here.

What we guarantee is that their result will be contributed in the same order to the list of build steps. Which you would think would be enough in this case, except you can have the following in a build step:

for (element in my hashset/map) {
    create a synthetic bean build item
}

which happens, and then your order is not guaranteed.

I thought it would be better to enforce the order at this level rather than going through all the possible code paths creating synthetic beans and enforcing strong ordering there.

I'm not working on the simple project I was working until now, I'm iterating on all the quickstarts so you have all sorts of cases.

Also, I often have 3-4, sometimes 8 reproducible builds until I have one that is different.

Let me know if it makes more sense :).

Copy link
Contributor

@mkouba mkouba Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which happens, and then your order is not guaranteed.

Ah, I see. Basically, a build step can produce the SyntheticBeanBuildItems in random order.

Copy link
Member Author

@gsmet gsmet Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is what I was trying to solve here:

Screenshot From 2026-03-17 18-17-14 Screenshot From 2026-03-17 18-18-27

Copy link
Member

@Ladicek Ladicek Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at MultiBuildItem javadoc, it says it gets injected in a sorted order if the MultiBuildItem implements Comparable. That sounds like a proper solution, instead of sorting on each use site?

Copy link
Contributor

@mkouba mkouba Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at MultiBuildItem javadoc, it says it gets injected in a sorted order if the MultiBuildItem implements Comparable. That sounds like a proper solution, instead of sorting on each use site?

I have to admit that I completely missed this sentence in the javadoc of MultiBuildItem but it really looks like a proper solution. @gsmet WDYM?

for (SyntheticBeanBuildItem bean : syntheticBeans) {
if (bean.hasRecorderInstance() && bean.isStaticInit()) {
configureSyntheticBean(recorder, creationFunctions, checkActiveSuppliers, beanRegistration, bean);
sortedBeans.put(createName(bean.configurator()), bean);
}
}

Map<String, Function<SyntheticCreationalContext<?>, ?>> creationFunctions = new LinkedHashMap<>();
Map<String, Supplier<ActiveResult>> checkActiveSuppliers = new LinkedHashMap<>();

for (Map.Entry<String, SyntheticBeanBuildItem> entry : sortedBeans.entrySet()) {
configureSyntheticBean(recorder, entry.getKey(), creationFunctions, checkActiveSuppliers, beanRegistration,
entry.getValue());
}
// Init the map of bean instances
recorder.initStaticSupplierBeans(creationFunctions, checkActiveSuppliers);
}
Expand All @@ -56,14 +63,20 @@ void initStatic(ArcRecorder recorder, List<SyntheticBeanBuildItem> syntheticBean
ServiceStartBuildItem initRuntime(ArcRecorder recorder, List<SyntheticBeanBuildItem> syntheticBeans,
BeanRegistrationPhaseBuildItem beanRegistration, BuildProducer<BeanConfiguratorBuildItem> configurators) {

Map<String, Function<SyntheticCreationalContext<?>, ?>> creationFunctions = new HashMap<>();
Map<String, Supplier<ActiveResult>> checkActiveSuppliers = new HashMap<>();

TreeMap<String, SyntheticBeanBuildItem> sortedBeans = new TreeMap<>();
for (SyntheticBeanBuildItem bean : syntheticBeans) {
if (bean.hasRecorderInstance() && !bean.isStaticInit()) {
configureSyntheticBean(recorder, creationFunctions, checkActiveSuppliers, beanRegistration, bean);
sortedBeans.put(createName(bean.configurator()), bean);
}
}

Map<String, Function<SyntheticCreationalContext<?>, ?>> creationFunctions = new LinkedHashMap<>();
Map<String, Supplier<ActiveResult>> checkActiveSuppliers = new LinkedHashMap<>();

for (Map.Entry<String, SyntheticBeanBuildItem> entry : sortedBeans.entrySet()) {
configureSyntheticBean(recorder, entry.getKey(), creationFunctions, checkActiveSuppliers, beanRegistration,
entry.getValue());
}
recorder.initRuntimeSupplierBeans(creationFunctions, checkActiveSuppliers);
return new ServiceStartBuildItem("runtime-bean-init");
}
Expand All @@ -72,18 +85,22 @@ ServiceStartBuildItem initRuntime(ArcRecorder recorder, List<SyntheticBeanBuildI
void initRegular(List<SyntheticBeanBuildItem> syntheticBeans,
BeanRegistrationPhaseBuildItem beanRegistration, BuildProducer<BeanConfiguratorBuildItem> configurators) {

TreeMap<String, SyntheticBeanBuildItem> sortedBeans = new TreeMap<>();
for (SyntheticBeanBuildItem bean : syntheticBeans) {
if (!bean.hasRecorderInstance()) {
configureSyntheticBean(null, null, null, beanRegistration, bean);
sortedBeans.put(createName(bean.configurator()), bean);
}
}

for (Map.Entry<String, SyntheticBeanBuildItem> entry : sortedBeans.entrySet()) {
configureSyntheticBean(null, entry.getKey(), null, null, beanRegistration, entry.getValue());
}
}

private void configureSyntheticBean(ArcRecorder recorder,
private void configureSyntheticBean(ArcRecorder recorder, String name,
Map<String, Function<SyntheticCreationalContext<?>, ?>> creationFunctions,
Map<String, Supplier<ActiveResult>> checkActiveSuppliers, BeanRegistrationPhaseBuildItem beanRegistration,
SyntheticBeanBuildItem bean) {
String name = createName(bean.configurator());
if (bean.configurator().getRuntimeValue() != null) {
creationFunctions.put(name, recorder.createFunction(bean.configurator().getRuntimeValue()));
} else if (bean.configurator().getSupplier() != null) {
Expand Down
3 changes: 2 additions & 1 deletion ...nsions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ValueRegistryProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.arc.deployment;

import java.util.Comparator;
import java.util.HashSet;
import java.util.ServiceLoader;
import java.util.Set;
Expand Down Expand Up @@ -42,7 +43,7 @@ void runtimeInfo(
ValueRegistryRecorder recorder,
BuildProducer<SyntheticBeanBuildItem> syntheticBeans) {

for (Class<?> runtimeInfo : getRuntimeInfoClasses()) {
for (Class<?> runtimeInfo : getRuntimeInfoClasses().stream().sorted(Comparator.comparing(Class::getName)).toList()) {
SyntheticBeanBuildItem.ExtendedBeanConfigurator configurator = SyntheticBeanBuildItem
.configure(runtimeInfo)
.startup()
Expand Down
5 changes: 3 additions & 2 deletions independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1338,8 +1338,9 @@ public BlockCreator checkActiveMethod() {
.append(implClassName)
.append("'\n");
msgBuilder.append("This bean is injected into:");
for (InjectionPointInfo matchingIP : matchingIPs) {
msgBuilder.append("\n\t- ").append(matchingIP.getTargetInfo());
for (String matchingTargetInfo : matchingIPs.stream().map(InjectionPointInfo::getTargetInfo).sorted()
.toList()) {
msgBuilder.append("\n\t- ").append(matchingTargetInfo);
}
}
b1.throw_(InactiveBeanException.class, b1.exprToString(msg));
Expand Down
Loading