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

DevMojo add only properties from the quarkus namespace #28000

Merged
merged 1 commit into from Sep 19, 2022

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Sep 16, 2022

There is a difference in the recorded properties between the build and dev mode. In dev mode, all properties were being recorded, which cause the issue described in #27884.

@quarkus-bot quarkus-bot bot added area/config area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/maven labels Sep 16, 2022
@aloubyansky
Copy link
Member

@radcortez could you clarify the issue a bit more? I suppose if I pass -Dxyz.version=bla I won't get a warn/error. I am wondering why build system properties would be different.

@radcortez
Copy link
Member Author

When we run in Dev mode, all properties from the Maven model end up in the Build System Config Source. Properties are collected here:

builder.buildSystemProperties((Map) project.getProperties());

And then end up here:

public static Consumer<BuildChainBuilder> loadStepsFrom(ClassLoader classLoader, Properties buildSystemProps,

Because we don't recognize these properties, they end up being recorded in the Runtime Defaults source. This issue was always there, but it was only uncovered with the change we did in #26802. Before this PR, the Runtime Defaults source was ignored entirely in Dev Mode.

In regular JVM mode and build, we never got this issue, because we filter the properties coming from the Maven model, to only have the quarkus namespace, here:

https://github.com/radcortez/quarkus/blob/08241672f6e3bae14dc54e8957ae81a943a7a39f/devtools/maven/src/main/java/io/quarkus/maven/QuarkusBootstrapProvider.java#L161-L165

The idea of this PR was also to add the filtering to Dev Mode, so it behave the same way as if we compiled and run. If you have any other ideas, I'm happy to change the approach :)

@aloubyansky
Copy link
Member

Makes sense, thanks!

@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Sep 19, 2022

I rebased it to get another CI run as we had quite a lot of failures.

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 19, 2022
@radcortez
Copy link
Member Author

Thanks!

@gsmet gsmet merged commit a46b99c into quarkusio:main Sep 19, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Sep 19, 2022
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Sep 19, 2022
@gsmet gsmet modified the milestones: 2.14 - main, 2.12.3.Final Sep 19, 2022
@radcortez radcortez deleted the fix-27884 branch December 16, 2022 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/maven kind/bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

quarkus-config: Maven-Property conflicts with 'prefix' (Regression since Quarkus 2.12)
3 participants