-
Couldn't load subscription status.
- Fork 3k
Do not copy quarkus configuration to Gradle plugin forced properties
#50750
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
base: main
Are you sure you want to change the base?
Conversation
| * @param appArtifact the application dependency to retrive the quarkus application name and version. | ||
| * @return a filtered view of the configuration only with <code>quarkus.</code> names. | ||
| */ | ||
| protected Map<String, String> buildSystemProperties(ResolvedDependency appArtifact, Map<String, String> quarkusProperties) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was removed because the method was copied in https://github.com/quarkusio/quarkus/pull/48769/files#diff-c7c4670b6f52fa54bf95459cc59308526b7b4331fe8bfb512245ba93413344f5R384, but the original code was not deleted.
Status for workflow
|
| .withForcedProperties(forced) | ||
| .withTaskProperties(properties) | ||
| .withBuildProperties(quarkusBuildProperties.get()) | ||
| .withProjectProperties(quarkusRelevantProjectProperties.get()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure why we have these quarkusRelevantProjectProperties, created from:
Lines 70 to 80 in f06d871
| private Provider<Map<String, String>> getQuarkusRelevantProjectProperties(Project project) { | |
| if (GradleVersion.current().compareTo(GradleVersion.version("8.0")) >= 0) { | |
| // This is more efficient, i.e.: configuration cache is invalidated only when quarkus properties change | |
| return getProviderFactory().gradlePropertiesPrefixedBy("quarkus."); | |
| } else { | |
| return getProviderFactory().provider(() -> project.getProperties().entrySet().stream() | |
| .filter(e -> e.getValue() != null) | |
| .map(e -> Map.entry(e.getKey(), e.getValue().toString())) | |
| .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); | |
| } | |
| } |
And was being passed down as Project Properties, where in every other place we don't do that filter and pass Project Properties directly:
Lines 71 to 86 in 88bfd66
| private BaseConfig buildBaseConfig() { | |
| // Using common code to construct the "base config", which is all the configuration (system properties, | |
| // environment, application.properties/yaml/yml, project properties) that is available in a Gradle task's | |
| // _configuration phase_. | |
| Set<File> resourcesDirs = getSourceSet(project, SourceSet.MAIN_SOURCE_SET_NAME).getResources().getSourceDirectories() | |
| .getFiles(); | |
| EffectiveConfig effectiveConfig = EffectiveConfig.builder() | |
| .withTaskProperties(Collections.emptyMap()) | |
| .withBuildProperties(quarkusBuildProperties.get()) | |
| .withProjectProperties(project.getProperties()) | |
| .withSourceDirectories(resourcesDirs) | |
| .withProfile(quarkusProfile()) | |
| .build(); | |
| return new BaseConfig(effectiveConfig); | |
| } |
Lines 125 to 151 in 88bfd66
| protected EffectiveConfig buildEffectiveConfiguration(ApplicationModel appModel) { | |
| ResolvedDependency appArtifact = appModel.getAppArtifact(); | |
| Map<String, Object> properties = new HashMap<>(); | |
| exportCustomManifestProperties(properties); | |
| Set<File> resourcesDirs = getSourceSet(project, SourceSet.MAIN_SOURCE_SET_NAME).getResources().getSourceDirectories() | |
| .getFiles(); | |
| Map<String, String> defaultProperties = new HashMap<>(); | |
| String userIgnoredEntries = String.join(",", ignoredEntries.get()); | |
| if (!userIgnoredEntries.isEmpty()) { | |
| defaultProperties.put("quarkus.package.jar.user-configured-ignored-entries", userIgnoredEntries); | |
| } | |
| defaultProperties.putIfAbsent("quarkus.application.name", appArtifact.getArtifactId()); | |
| defaultProperties.putIfAbsent("quarkus.application.version", appArtifact.getVersion()); | |
| return EffectiveConfig.builder() | |
| .withPlatformProperties(appModel.getPlatformProperties()) | |
| .withTaskProperties(properties) | |
| .withBuildProperties(quarkusBuildProperties.get()) | |
| .withProjectProperties(project.getProperties()) | |
| .withDefaultProperties(defaultProperties) | |
| .withSourceDirectories(resourcesDirs) | |
| .withProfile(quarkusProfile()) | |
| .build(); | |
| } |
For some reason, we were copying
quarkus.*configuration to the Gradle plugin forced properties Config Source, which is set to600in ordinality, being the highest of all sources.I believe this started with #41337, where I've copied what was already there in f10a208#diff-2c6f0d62b0763af665a1bc242ad078ce25794f20632651181549d838e3452998R113-R118
Now, I don't think this is required, because Project Properties also contains the same configuration, so we shouldn't be missing anything, but this approach causes #46456.
Subsequent code changes probably just followed the same pattern. Also, in #48769 added yet a new way of doing things and a
quarkusRelevantProjectProperties, which filtersquarkus.*properties from Project Properties, causing inconsistencies in the available configuration when used. I've changed that to just pass all Project Properties like we are doing in other places.