-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Drop and ban commons-io dependency from quarkus-core-deployment #30542
Conversation
@@ -30,6 +30,10 @@ | |||
<groupId>io.smallrye</groupId> | |||
<artifactId>jandex</artifactId> | |||
</dependency> | |||
<dependency> | |||
<groupId>commons-io</groupId> | |||
<artifactId>commons-io</artifactId> |
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.
Required in this module since it's using org.apache.commons.io.input.TeeInputStream
in DefaultDockerContainerLauncher.java
Hi @gsmet, this is a follow up of #30108 (comment) to drop commons-io from quarkus-core-deployment: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I will have a look at this soon. |
This comment has been minimized.
This comment has been minimized.
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 comment has been minimized.
This comment has been minimized.
This also update forbiddenapis to 3.4 and substitute calls to org.apache.commons.io.IOUtils#copy(java.io.InputStream,java.io.OutputStream) and org.apache.commons.compress.utils.IOUtils#copy(java.io.InputStream,java.io.OutputStream) using java.io.InputStream#transferTo(java.io.OutputStream) Signed-off-by: Jorge Solórzano <jorsol@gmail.com>
Thanks a lot! |
@defaultMessage Never use Type#toString() as it's almost always the wrong thing to do. Usually org.jboss.jandex.DotName#toString() is what is needed | ||
org.jboss.jandex.Type#toString() | ||
|
||
org.apache.commons.io.** @ Don't use commons-io dependency |
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.
I'm probably missing something, but why aren't we using maven-enforcer's bannedDependencies
?
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.
Indeed. I'm creating a PR.
Remove the dependency
commons-io
fromquarkus-core-deployment
and add a ban in the module.This also update forbiddenapis to 3.4 and substitute calls to
org.apache.commons.io.IOUtils#copy(java.io.InputStream,java.io.OutputStream)
andorg.apache.commons.compress.utils.IOUtils#copy(java.io.InputStream,java.io.OutputStream)
usingjava.io.InputStream#transferTo(java.io.OutputStream)