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

Native apps throw a warning now that quarkus.uuid is lazily produced #45495

Closed
zakkak opened this issue Jan 10, 2025 · 16 comments · Fixed by #45563
Closed

Native apps throw a warning now that quarkus.uuid is lazily produced #45495

zakkak opened this issue Jan 10, 2025 · 16 comments · Fixed by #45563
Assignees
Labels
area/config kind/bug Something isn't working
Milestone

Comments

@zakkak
Copy link
Contributor

zakkak commented Jan 10, 2025

Describe the bug

Since #45434 native executables throw the following warning:

2025-01-10 11:23:51,956 WARN  [io.qua.config] (main) Unrecognized configuration key "quarkus.uuid" was provided; it will be ignored; verify that the dependency extension for this configuration is set or that you did not make a typo

Expected behavior

Native apps should not throw any warnings like they do not in JVM-mode.

Actual behavior

Native executables throw the following warning:

2025-01-10 11:23:51,956 WARN  [io.qua.config] (main) Unrecognized configuration key "quarkus.uuid" was provided; it will be ignored; verify that the dependency extension for this configuration is set or that you did not make a typo

How to Reproduce?

./mvnw -Dnative -pl integration-tests/picocli-native -Dnative.surefire.skip -Dformat.skip -Dno-descriptor-tests clean package
./integration-tests/picocli-native/target/quarkus-integration-test-picocli-native-999-SNAPSHOT-runner quarkus

Output of uname -a or ver

No response

Output of java -version

No response

Mandrel or 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

@zakkak zakkak added area/native-image kind/bug Something isn't working labels Jan 10, 2025
Copy link

quarkus-bot bot commented Jan 10, 2025

/cc @Karm (native-image), @galderz (native-image)

@zakkak
Copy link
Contributor Author

zakkak commented Jan 10, 2025

@geoand while at it you might want to apply the following patch as well:

diff --git a/core/runtime/src/main/java/io/quarkus/runtime/configuration/RuntimeConfigBuilder.java b/core/runtime/src/main/java/io/quarkus/runtime/configuration/RuntimeConfigBuilder.java
index 5c425918ada..ae3fb083cdc 100644
--- a/core/runtime/src/main/java/io/quarkus/runtime/configuration/RuntimeConfigBuilder.java
+++ b/core/runtime/src/main/java/io/quarkus/runtime/configuration/RuntimeConfigBuilder.java
@@ -29,16 +29,14 @@ public int priority() {
 
     private static class UuiConfigSource implements ConfigSource {
 
-        private static final String QUARKUS_UUID = "quarkus.uuid";
-
         @Override
         public Set<String> getPropertyNames() {
-            return Set.of(QUARKUS_UUID);
+            return Set.of(ConfigUtils.UUID_KEY);
         }
 
         @Override
         public String getValue(String propertyName) {
-            if (propertyName.equals(QUARKUS_UUID)) {
+            if (propertyName.equals(ConfigUtils.UUID_KEY)) {
                 return Holder.UUID_VALUE;
             }
             return null;

@geoand
Copy link
Contributor

geoand commented Jan 10, 2025

@radcortez what do we need to generate to remove the warning?
Should I bring back

suppressNonRuntimeConfigChanged.produce(new SuppressNonRuntimeConfigChangedWarningBuildItem("quarkus.uuid"));

?

@zakkak zakkak changed the title Native apps through a warning now that quarkus.uuid is lazily produced Native apps throw a warning now that quarkus.uuid is lazily produced Jan 10, 2025
@radcortez
Copy link
Member

Hum... actually, I don't think we have a build step to ignore the warning. That one is to ignore when something changes from build time to runtime (recorded), like the profile.

The only quick option that I can see is to hardcode it here:

public static void unknownProperties(Set<String> properties) {
if (properties.isEmpty()) {
return;
}
SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
Set<String> usedProperties = new HashSet<>();
StringBuilder tmp = null;
for (String property : config.getPropertyNames()) {
if (properties.contains(property)) {
continue;
}
if (tmp == null) {
tmp = new StringBuilder(property.length());
} else {
tmp.setLength(0);
}
String usedProperty = StringUtil.replaceNonAlphanumericByUnderscores(property, tmp);
if (properties.contains(usedProperty)) {
continue;
}
usedProperties.add(usedProperty);
}
for (String property : properties) {
// Indexed properties not supported by @ConfigRoot, but they can show up due to the YAML source. Just ignore them.
if (property.indexOf('[') != -1 && property.indexOf(']') != -1) {
continue;
}
boolean found = false;
if (!usedProperties.isEmpty()) {
if (tmp == null) {
tmp = new StringBuilder(property.length());
} else {
tmp.setLength(0);
}
String propertyWithUnderscores = StringUtil.replaceNonAlphanumericByUnderscores(property, tmp);
for (String usedProperty : usedProperties) {
if (usedProperty.equalsIgnoreCase(propertyWithUnderscores)) {
found = true;
break;
}
}
}
if (!found) {
ConfigValue configValue = config.getConfigValue(property);
if (property.equals(configValue.getName())) {
unknown(property);
}
}
}
}

@gsmet
Copy link
Member

gsmet commented Jan 10, 2025

What exactly sets the quarkus.uuid value? Because somehow something is complaining that it's being set and not part of the mapping?

@geoand
Copy link
Contributor

geoand commented Jan 10, 2025

Right, I didn't expect to have this warning

@radcortez
Copy link
Member

It was removed from the mapping https://github.com/quarkusio/quarkus/pull/45434/files#r1906838929 :)

Actually, I think we can get away with it by removing it from the list of property names:
https://github.com/geoand/quarkus/blob/8a1ab70291b67ce2134299d28cf2227d9910e219/core/runtime/src/main/java/io/quarkus/runtime/configuration/RuntimeConfigBuilder.java#L36

The list of unknowns is generated by matching the list of all properties to the ones declared in either mappings or roots.

It could still happen if someone overrides the value.

@radcortez
Copy link
Member

Right, I didn't expect to have this warning

It is expected, because we validate that all provided configuration in the quarkus namespace requires a mapping or root declaration.

@geoand
Copy link
Contributor

geoand commented Jan 10, 2025

So @radcortez I'll leave this to you :)

@radcortez
Copy link
Member

Sure.

@geoand
Copy link
Contributor

geoand commented Jan 10, 2025

🙏🏽

@gsmet
Copy link
Member

gsmet commented Jan 10, 2025

@radcortez but what in native image is specific that we only get the problem there? Or do we also get the problem in JVM mode?

@zakkak
Copy link
Contributor Author

zakkak commented Jan 10, 2025

@radcortez but what in native image is specific that we only get the problem there? Or do we also get the problem in JVM mode?

public static void unknownRunTime(String name) {
if (ImageMode.current() == ImageMode.NATIVE_RUN) {
// only warn at run time for native images, otherwise the user will get warned twice for every property
unknown(name);
}
}

public static void unknownPropertiesRuntime(Set<String> properties) {
if (ImageMode.current() == ImageMode.NATIVE_RUN) {
unknownProperties(properties);
}
}

The warning is not thrown at runtime for JVM-mode (didn't check for warnings at compile time).

@radcortez
Copy link
Member

@radcortez but what in native image is specific that we only get the problem there? Or do we also get the problem in JVM mode?

Unsure, I'll have to check.

@gsmet
Copy link
Member

gsmet commented Jan 13, 2025

@radcortez we need this solved very soon or we should revert the change. We can't ship with the warning. We can live with it for the CR1 but we need a fix (or a revert) for the Final so before Tuesday 21st evening.

@radcortez
Copy link
Member

Looking into it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants