Allow to filter dependencies in multi-build mode #16600
Conversation
This PR should do the job cc @aloubyansky |
essobedo
force-pushed
the
16599/filter-dependencies
branch
from
520188e to
145c883
Compare
|
This workflow status is outdated as a new workflow run has been triggered. 🚫 This workflow run has been cancelled. Failing Jobs - Building 520188e
|
|
@essobedo is the only reason Maven profiles wouldn't work here because you want to activate both build configs during the same maven build? |
@aloubyansky Exactly, with those changes, we can for example build the application for different databases with the same maven build |
essobedo
force-pushed
the
16599/filter-dependencies
branch
2 times, most recently
from
fb69c21 to
4795a69
Compare
|
This workflow status is outdated as a new workflow run has been triggered. 🚫 This workflow run has been cancelled. Failing Jobs - Building fb69c21
|
|
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 4795a69
Full information is available in the Build summary check run. Test Failures⚙️ JVM Tests - JDK 15 #📦 integration-tests/artemis-core✖ 📦 integration-tests/artemis-jms✖ 📦 integration-tests/bootstrap-config/application✖ 📦 integration-tests/cache✖ 📦 integration-tests/consul-config✖ 📦 integration-tests/container-image/quarkus-standard-way✖ 📦 integration-tests/devmode✖ 📦 integration-tests/elytron-security-jdbc✖ 📦 integration-tests/elytron-security-ldap✖ 📦 integration-tests/google-cloud-functions-http✖ 📦 integration-tests/grpc-health✖ ✖ 📦 integration-tests/grpc-interceptors✖ 📦 integration-tests/grpc-mutual-auth✖ 📦 integration-tests/grpc-plain-text-gzip✖ 📦 integration-tests/grpc-plain-text-mutiny✖ 📦 integration-tests/grpc-proto-v2✖ 📦 integration-tests/grpc-streaming✖ 📦 integration-tests/grpc-tls✖ 📦 integration-tests/injectmock✖ 📦 integration-tests/jackson✖ 📦 integration-tests/jaxp✖ 📦 integration-tests/jgit✖ 📦 integration-tests/jpa-without-entity✖ 📦 integration-tests/jsch✖ 📦 integration-tests/jsonb✖ 📦 integration-tests/kubernetes-client✖ ✖ ✖ ✖ ✖ ✖ ✖ 📦 integration-tests/kubernetes-service-binding-jdbc✖ 📦 integration-tests/logging-gelf✖ 📦 integration-tests/logging-json✖ 📦 integration-tests/logging-min-level-set✖ 📦 integration-tests/logging-min-level-unset✖ 📦 integration-tests/mailer✖ 📦 integration-tests/micrometer-mp-metrics✖ 📦 integration-tests/micrometer-prometheus✖ 📦 integration-tests/mongodb-client✖ ✖ 📦 integration-tests/mongodb-panache-kotlin✖ 📦 integration-tests/mongodb-panache✖ 📦 integration-tests/mongodb-rest-data-panache✖ 📦 integration-tests/narayana-jta✖ 📦 integration-tests/narayana-stm✖ 📦 integration-tests/neo4j✖ 📦 integration-tests/openshift-client✖ 📦 integration-tests/opentelemetry✖ 📦 integration-tests/picocli-native✖ 📦 integration-tests/quartz✖ 📦 integration-tests/qute✖ 📦 integration-tests/reactive-messaging-amqp✖ 📦 integration-tests/reactive-messaging-http✖ 📦 integration-tests/reactive-messaging-kafka✖ 📦 integration-tests/redis-client✖ 📦 integration-tests/rest-client✖ 📦 integration-tests/resteasy-mutiny✖ 📦 integration-tests/resteasy-reactive-kotlin✖ 📦 integration-tests/resteasy-reactive-rest-client-standalone✖ 📦 integration-tests/resteasy-reactive-rest-client✖ 📦 integration-tests/simple with space✖ 📦 integration-tests/smallrye-config✖ ✖ 📦 integration-tests/smallrye-context-propagation✖ ✖ ✖ ✖ 📦 integration-tests/smallrye-graphql✖ 📦 integration-tests/smallrye-metrics✖ 📦 integration-tests/tika✖ 📦 integration-tests/vertx-graphql✖ 📦 integration-tests/virtual-http-resteasy✖ 📦 integration-tests/virtual-http✖ 📦 integration-tests/webjars-locator✖ |
essobedo
force-pushed
the
16599/filter-dependencies
branch
from
4795a69 to
c28228a
Compare
core/deployment/src/main/java/io/quarkus/deployment/pkg/PackageConfig.java
Show resolved
Hide resolved
| * package with unused dependencies. | ||
| */ | ||
| @ConfigItem(defaultValue = "false") | ||
| public boolean filterOptionalDependencies; |
There was a problem hiding this comment.
Can you give a reasonable example of having configured optionalDependencies and wishing filterOptionalDependencies to be false?
There was a problem hiding this comment.
@aloubyansky No there is no such example. But I need a mechanism to enable the feature in order to distinguish, the case where we don't want to include any optional dependencies by not setting optionalDependencies intentionally and the case where optionalDependencies has not been set after migrating an application (what I called "backward compatibility reasons")
There was a problem hiding this comment.
It seems like there is some config redundancy. I.e. if you specify which dependencies you want to keep, it implies the rest of the optional deps you will want to be filtered out.
There was a problem hiding this comment.
I'm not sure, I get what you mean. This is only a flag to enable the feature.
There was a problem hiding this comment.
If you don't use this flag, there are several tests with optional dependencies that fail because it will filter out all the optional dependencies
There was a problem hiding this comment.
That's probably a sign of a bug? Why would all the optional dependencies be filtered out w/o it?
What I'm saying is if you configure optionalDependencies, you list the dependencies you want to keep. Which means the rest of the optional dependencies aren't desired, right?
There was a problem hiding this comment.
That's probably a sign of a bug? Why would all the optional dependencies be filtered out w/o it?
@aloubyansky No it is actually intended, let's say that you want to build your application twice with two different Quarkus profiles foo and bar. The code for bar needs one additional library "ext-bar". So the dependency "ext-bar" will be marked as optional, foo won't define anything for includedOptionalDependencies as it doesn't need any optional dependencies while bar will set ext-bar as includedOptionalDependencies. Do you see what I mean?
What I'm saying is if you configure
optionalDependencies, you list the dependencies you want to keep. Which means the rest of the optional dependencies aren't desired, right?
That's right, it also means that if no optionalDependencies are configured and filterOptionalDependencies has been set to true, then it will not include any optional dependencies.
Here are the use cases:
filterOptionalDependenciesset tofalse(the default value), it will behaves like before in other words, the optional dependencies are not filtered whatever the value ofincludedOptionalDependenciessince the feature is disabled (what I refer as "backward compatibility reasons")filterOptionalDependenciesset totrueandincludedOptionalDependenciesare absent, then all the optional dependencies are filtered out.filterOptionalDependenciesset totrueandincludedOptionalDependenciesare present, then only the optional dependencies inincludedOptionalDependenciesare kept, the rest is filtered out.
Is it clear enough now?
There was a problem hiding this comment.
Yes, thanks. This PR actually enables reasonable use-cases for optional dependencies in Quarkus apps. Otherwise, I am not sure they make sense in Quarkus apps. There is a use-case of re-augmenting mutable-jar packaged apps though. But even then it might benefit from these config options.
core/deployment/src/main/java/io/quarkus/deployment/pkg/PackageConfig.java
Outdated
Show resolved
Hide resolved
core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java
Outdated
Show resolved
Hide resolved
essobedo
force-pushed
the
16599/filter-dependencies
branch
from
c28228a to
8883c8d
Compare
essobedo
force-pushed
the
16599/filter-dependencies
branch
from
8883c8d to
bcc2e07
Compare
|
Thanks @essobedo, great work! Would you mind writing an article about the multi-build config to the Quarkus guides [1] to promote the features you contributed? |
@aloubyansky Yes, of course, with pleasure, but If you don't mind, I would like first to address the issue with the parallel build to propose a complete solution. Is that ok with you or you rather prefer that I write the article first? |
|
Thanks! Whatever you prefer. The profile/parallel build might not be a quick fix. |
|
I know that it's not a quick fix but I have something in mind that I would like to test (this Friday), if it does work then I will work on the article first. |
cc @bcournaud
fixes #16599
Motivation
Thanks to #16053, it is now possible to build several artifacts from one module but it is not yet possible to have a way to filter out some dependencies such that we could end up with artifacts containing useless dependencies. The idea of this feature is to go a little bit further by proposing a way to filter out the dependencies.
Modifications:
optionalDependenciesto the classPackageConfigin order to specify the list of optional dependencies that we would like to include into the final package of the application.filterOptionalDependenciesto the classPackageConfigto enable or not the feature, it is disabled by default for backward compatibility reasons.optionalDependenciesto the classOutputTargetBuildItemcorresponding to the parameteroptionalDependenciesbut after being post processed to make it absent if the feature is disabled, to make it empty if no optional dependencies were specified and to convert the String representation of the artifacts into the correspondingAppArtifactKey's instancesoptionalDependenciesof the classOutputTargetBuildItemto filter out all the dependencies of the application that have been marked as optional for all supported package types.Result
It is now possible to build several artifacts with specific dependencies, this could be used for example to package the same application for different databases.