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

Exclude commons-logging from resteasy-client-microprofile #21148

Merged
merged 1 commit into from
Nov 4, 2021

Conversation

glefloch
Copy link
Member

@glefloch glefloch commented Nov 2, 2021

This makes sure commons-logging does not end up in the compile classpath of a gradle project.

close #20896

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 2, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building d9895cb

Status Name Step Failures Logs Raw logs
JVM Tests - JDK 11 Build Failures Logs Raw logs
✔️ JVM Tests - JDK 17

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: integration-tests/smallrye-opentracing 

📦 integration-tests/smallrye-opentracing

io.quarkus.it.opentracing.OpenTracingTestCase. line 35 - More details - Source on GitHub

org.testcontainers.containers.ContainerLaunchException: Container startup failed
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:336)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:317)

<exclusion>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
</exclusion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should add it to our exclusion list in the build parent?

Also maybe we should add the JBoss Logging Bridge? You should be able to find which one to use in some of the other extensions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I will give it a try

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsmet, commons-logging is already in the exclude list of the build parent (

<exclude>commons-logging:commons-logging</exclude>
) I try to exclude it directly in the bom, this works too but I don't know if we should exclude it there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commons-logging-jboss-logging is already declared in that pom.xml

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but why don't we have an error if it's supposed to be excluded?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aloubyansky, @famod Do you know why commons-logging can be in transitive dependencies while it is banned in the maven-enforcer-plugin ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it catches everything.

There's something weird going on because I don't get any commons-logging when doing mvn dependency:tree for this module and in particular for resteasy-client-microprofile I get:

[INFO] +- org.jboss.resteasy:resteasy-client-microprofile:jar:4.7.0.Final:compile
[INFO] |  \- org.jboss.resteasy:resteasy-client-microprofile-base:jar:4.7.0.Final:compile
[INFO] |     +- org.jboss.resteasy:resteasy-client:jar:4.7.0.Final:compile
[INFO] |     |  +- org.jboss.resteasy:resteasy-client-api:jar:4.7.0.Final:compile
[INFO] |     |  \- commons-io:commons-io:jar:2.11.0:compile
[INFO] |     \- org.jboss.resteasy:resteasy-cdi:jar:4.7.0.Final:compile

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the problem comes from the plug-in but from gradle ignoring it when resolving dependencies. How does maven handles that enforcer rules from dependencies ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enforcer just checks that the dependencies are not around, it doesn't exclude anything. So if we don't have it in the Maven dependencies, it's because, using the Maven resolver, the dependency is not around. What I don't understand is why Gradle sees it in its dependencies. Maybe it still includes the transitive dependencies of the excluded dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s really weird. Some transitive dependencies may bring commons-logging back but I don’t know how

@michalszynkiewicz
Copy link
Member

the error doesn't seem related

@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Nov 3, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 3, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 7f8e25c

Status Name Step Failures Logs Raw logs
Initial JDK 11 Build Build ⚠️ Check → Logs Raw logs

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, well, in any case, it doesn't hurt and we have checked that our enforcer plugin configuration is not missing anything here. I suspect there's something weird in how the dependencies are resolved in Gradle or our Gradle plugin, but we can merge this.

@gsmet gsmet merged commit 1ff6fd4 into quarkusio:main Nov 4, 2021
@quarkus-bot quarkus-bot bot added this to the 2.5 - main milestone Nov 4, 2021
@glefloch glefloch deleted the fix/20896 branch November 4, 2021 20:21
@gsmet gsmet modified the milestones: 2.5.0.CR1, 2.4.2.Final Nov 11, 2021
@vhmolinar
Copy link

@glefloch
As mentioned in the issue #20896 the extension quarkus-keycloak-admin-client brings back the commons-logging dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClassNotFoundException when injecting REST client in native
4 participants