Introduce quarkus.package.decompiler config #37445

geoand · 2023-12-01T06:48:32Z

Fixes Rename the Vineflower properties ? #37332

geoand · 2023-12-01T07:21:52Z

Seems like anything Gradle related is failing... I have no idea why

core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java

mkouba · 2023-12-01T08:37:48Z

core/deployment/src/main/java/io/quarkus/deployment/pkg/PackageConfig.java

@@ -249,9 +250,18 @@ public static BuiltInType fromString(String value) {

    /**
     * Vineflower Decompiler configuration
+     *
+     * @Deprecated use {@code quarkus.package.decompiler} instead


Maybe add a note after which version it will be removed?

I don't know when it will be removed :)

For sure, but I usually add a hint like "will be removed at some point after Quarkus 3.10" or something like that. To define the "transition period". So that the next one who reads the javadoc can actually remove it. Because otherwise it may stay there forever...

gsmet · 2023-12-01T12:46:18Z

core/deployment/src/main/java/io/quarkus/deployment/pkg/PackageConfig.java

+     * @Deprecated use {@code quarkus.package.decompiler} instead
+     */
+    @ConfigItem
+    @Deprecated(forRemoval = true)


Not entirely sure this will work at the group level. We should make it work if it doesn't I suppose.

You mean in the generated docs or something else?

gsmet · 2023-12-18T14:43:00Z

The Gradle failures look odd. I haven't seen them anywhere else.

geoand · 2023-12-18T14:44:01Z

Yeah, I really don't know what to make of them...

gsmet · 2023-12-18T14:53:51Z

I'll have a look once we have the new report posted.

geoand · 2023-12-18T14:57:06Z

Thanks for the help!

I'll run them locally tomorrow and let you know if I can't figure it out.

gsmet · 2023-12-18T18:12:41Z

java.lang.NoSuchMethodException: java.util.Optional.<init>() at io.quarkus.runtime.configuration.ConfigInstantiator.handleObject(ConfigInstantiator.java:139) at io.quarkus.runtime.configuration.ConfigInstantiator.handleObject(ConfigInstantiator.java:110) ... 103 more Caused by: java.lang.NoSuchMethodException: java.util.Optional.<init>() at io.quarkus.runtime.configuration.ConfigInstantiator.handleObject(ConfigInstantiator.java:106) ... 104 more Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0. You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins. For more on this, please refer to https://docs.gradle.org/8.5/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation. BUILD FAILED in 5s 7 actionable tasks: 5 executed, 2 up-to-date Configuration cache entry discarded with 3 problems.

My guess is that it's related to something that is not properly handled by our config framework. But I don't see anything odd here.

Maybe @radcortez will have some insights.

radcortez · 2023-12-19T10:50:56Z

Probably this is caused by this:

quarkus/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/BaseConfig.java

Line 31 in 44b76fa

ConfigInstantiator.handleObject(packageConfig, config.config());

This ConfigInstantiator is not fully implemented and was added to manually create config objects. Ideally, we should get rid of this, because we can now create such objects (fully implemented) with SmallRye Config.

To move forward, either we implement the missing features in ConfigInstantiator, or we change the config object to only use the supported contracts.

geoand · 2023-12-19T18:10:52Z

I don't understand however what in this PR triggers the problem

dmlloyd · 2023-12-19T19:40:20Z

I'm currently looking at changing this config, possibly to make it independent of packaging altogether, as part of reworking native image. I'm not sure what this would mean for this PR though.

dmlloyd · 2023-12-19T22:04:06Z

core/deployment/src/main/java/io/quarkus/deployment/pkg/PackageConfig.java

    @ConfigGroup
-    public static class VineFlowerConfig {
+    public static class DecompilerConfig {
        /**
         * An advanced option that will decompile generated and transformed bytecode into the 'decompiled' directory.
         * This is only taken into account when fast-jar is used.
         */
-        @ConfigItem(defaultValue = "false")
-        public boolean enabled;
-
-        /**
-         * The version of Vineflower to use
-         */
-        @ConfigItem(defaultValue = "1.9.3")
-        public String version;
+        @ConfigDocDefault("false")
+        public Optional<Boolean> enabled;

        /**
         * The directory into which to save the Vineflower tool if it doesn't exist
         */
-        @ConfigItem(defaultValue = "${user.home}/.quarkus")
-        public String jarDirectory;
+        @ConfigItem
+        @ConfigDocDefault("${user.home}/.quarkus")
+        public Optional<String> jarDirectory;


It seems like this whole configuration should be separate from packaging. For example if we produce multiple artifacts with different packaging configurations, we'd still only produce one set of decompiled sources, right?

Yeah, I'm not against that.

dmlloyd · 2023-12-19T22:05:10Z

core/deployment/src/main/java/io/quarkus/deployment/pkg/PackageConfig.java

-        @ConfigItem(defaultValue = "1.9.3")
-        public String version;
+        @ConfigDocDefault("false")
+        public Optional<Boolean> enabled;


Are these being made into optionals solely to accommodate the fact that the configuration moved? If so, I think we can/should probably brainstorm a better solution, otherwise these kinds of things are going to accumulate and cause problems.

Correct. I'm definitely open to looking for a better solution

A possible solution is to use relocate functions: https://smallrye.io/smallrye-config/3.4.4/extensions/relocate/

radcortez · 2023-12-20T10:57:19Z

I don't understand however what in this PR triggers the problem

I suspect the new Optional config elements fails here:

quarkus/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigInstantiator.java

Lines 105 to 110 in f10a208

    
           if (configItem == null || fieldClass.isAnnotationPresent(ConfigGroup.class)) { 
        
               Constructor<?> constructor = fieldClass.getConstructor(); 
        
               constructor.setAccessible(true); 
        
               Object newInstance = constructor.newInstance(); 
        
               field.set(o, newInstance); 
        
               handleObject(prefix + "." + dashify(field.getName()), newInstance, config, quarkusPropertyNames);

radcortez · 2023-12-20T10:59:52Z

core/deployment/src/main/java/io/quarkus/deployment/pkg/PackageConfig.java

-         */
-        @ConfigItem(defaultValue = "1.9.3")
-        public String version;
+        @ConfigDocDefault("false")


Adding @ConfigItem should fix #37445 (comment)

Thanks for spotting,what a dumb mistake on my part!

Let me try it out and see how it goes

This is meant to be used instead of quarkus.package.vineflower. Furthermore, this removes the configurable version property which was never being used. Closes: quarkusio#37332

github-actions · 2023-12-20T11:47:04Z

🙈 The PR is closed and the preview is expired.

quarkus-bot · 2023-12-20T16:31:01Z

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

dmlloyd · 2023-12-20T16:43:47Z

Well... I guess I'll make a note to address the problems later then. I probably should have "requested changes" so it's my fault I think.

geoand · 2023-12-20T16:51:40Z

I think the proposed changes were pretty much orthogonal to this. Furthermore, the two different configurations are only meant to last a couple versions and then the old will go away

geoand · 2023-12-20T16:52:22Z

Sorry for misunderstanding how severe you thought the impact was :(

dmlloyd · 2023-12-20T16:55:26Z

No worries, it's my fault. I have to get over my aversion to "Request Changes". It just seems so unkind :-)

geoand · 2023-12-20T16:56:27Z

I totally know how you feel, I rarely use it :)

Follows up on quarkusio#37445

geoand mentioned this pull request Dec 1, 2023

Rename the Vineflower properties ? #37332

Closed

quarkus-bot bot added area/core area/documentation area/panache labels Dec 1, 2023

mkouba reviewed Dec 1, 2023

View reviewed changes

core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java Outdated Show resolved Hide resolved

core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java Outdated Show resolved Hide resolved

geoand force-pushed the #37332 branch from c93211a to 9269c84 Compare December 1, 2023 08:30

mkouba reviewed Dec 1, 2023

View reviewed changes

mkouba approved these changes Dec 1, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

gsmet reviewed Dec 1, 2023

View reviewed changes

geoand force-pushed the #37332 branch from 9269c84 to 04f6301 Compare December 5, 2023 08:37

This comment has been minimized.

Sign in to view

geoand force-pushed the #37332 branch from 04f6301 to 91db9e8 Compare December 18, 2023 13:37

gsmet approved these changes Dec 18, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

dmlloyd reviewed Dec 19, 2023

View reviewed changes

radcortez reviewed Dec 20, 2023

View reviewed changes

geoand added 2 commits December 20, 2023 13:11

Introduce quarkus.package.decompiler config

fdec833

This is meant to be used instead of quarkus.package.vineflower. Furthermore, this removes the configurable version property which was never being used. Closes: quarkusio#37332

Use new decompiler option

fccdcb0

geoand force-pushed the #37332 branch from 91db9e8 to fccdcb0 Compare December 20, 2023 11:13

This comment has been minimized.

Sign in to view

geoand merged commit 683b16c into quarkusio:main Dec 20, 2023
54 checks passed

quarkus-bot bot added the kind/enhancement New feature or request label Dec 20, 2023

quarkus-bot bot added this to the 3.7 - main milestone Dec 20, 2023

geoand deleted the #37332 branch December 20, 2023 16:51

dmlloyd added a commit to dmlloyd/quarkus that referenced this pull request Dec 21, 2023

Use relocation for changing config properties

cf62c91

Follows up on quarkusio#37445

dmlloyd added a commit to dmlloyd/quarkus that referenced this pull request Dec 21, 2023

Use relocation for changing config properties

c54b899

Follows up on quarkusio#37445

dmlloyd mentioned this pull request Dec 21, 2023

Use relocation for changing config properties #37898

Closed

dmlloyd added a commit to dmlloyd/quarkus that referenced this pull request Dec 22, 2023

Use relocation for changing config properties

91db25c

Follows up on quarkusio#37445

dmlloyd added a commit to dmlloyd/quarkus that referenced this pull request Jan 4, 2024

Use relocation for changing config properties

637f311

Follows up on quarkusio#37445

dmlloyd added a commit to dmlloyd/quarkus that referenced this pull request Jan 4, 2024

Use relocation for changing config properties

5ec66e5

Follows up on quarkusio#37445

dmlloyd mentioned this pull request Apr 29, 2024

Fix accidental config breakage of quarkus.package.decompiler.* properties #40277

Merged

manofthepeace mentioned this pull request May 30, 2024

Remove deprecated vineflower properties support #40908

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce quarkus.package.decompiler config #37445

Introduce quarkus.package.decompiler config #37445

geoand commented Dec 1, 2023 •

edited

Loading

geoand commented Dec 1, 2023

mkouba Dec 1, 2023

geoand Dec 1, 2023

mkouba Dec 1, 2023

This comment has been minimized.

gsmet Dec 1, 2023

geoand Dec 5, 2023

This comment has been minimized.

gsmet commented Dec 18, 2023

geoand commented Dec 18, 2023

gsmet commented Dec 18, 2023

geoand commented Dec 18, 2023

This comment has been minimized.

gsmet commented Dec 18, 2023 •

edited

Loading

radcortez commented Dec 19, 2023

geoand commented Dec 19, 2023

dmlloyd commented Dec 19, 2023

dmlloyd Dec 19, 2023

geoand Dec 20, 2023

dmlloyd Dec 19, 2023 •

edited

Loading

geoand Dec 20, 2023

radcortez Dec 20, 2023

radcortez commented Dec 20, 2023

radcortez Dec 20, 2023

geoand Dec 20, 2023 •

edited

Loading

github-actions bot commented Dec 20, 2023 •

edited

Loading

This comment has been minimized.

quarkus-bot bot commented Dec 20, 2023

dmlloyd commented Dec 20, 2023

geoand commented Dec 20, 2023

geoand commented Dec 20, 2023

dmlloyd commented Dec 20, 2023

geoand commented Dec 20, 2023

Introduce quarkus.package.decompiler config #37445

Introduce quarkus.package.decompiler config #37445

Conversation

geoand commented Dec 1, 2023 • edited Loading

geoand commented Dec 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

gsmet commented Dec 18, 2023

geoand commented Dec 18, 2023

gsmet commented Dec 18, 2023

geoand commented Dec 18, 2023

This comment has been minimized.

gsmet commented Dec 18, 2023 • edited Loading

radcortez commented Dec 19, 2023

geoand commented Dec 19, 2023

dmlloyd commented Dec 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmlloyd Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radcortez commented Dec 20, 2023

Choose a reason for hiding this comment

geoand Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 20, 2023 • edited Loading

This comment has been minimized.

quarkus-bot bot commented Dec 20, 2023

dmlloyd commented Dec 20, 2023

geoand commented Dec 20, 2023

geoand commented Dec 20, 2023

dmlloyd commented Dec 20, 2023

geoand commented Dec 20, 2023

geoand commented Dec 1, 2023 •

edited

Loading

gsmet commented Dec 18, 2023 •

edited

Loading

dmlloyd Dec 19, 2023 •

edited

Loading

geoand Dec 20, 2023 •

edited

Loading

github-actions bot commented Dec 20, 2023 •

edited

Loading