-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HADOOP-19329. Remove usage of sun.misc.Signal #7145
base: trunk
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
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.
Would you like to open a new jira? HADOOP-19318 is resolved as workaround.
@@ -416,6 +416,12 @@ | |||
<artifactId>lz4-java</artifactId> | |||
<scope>provided</scope> | |||
</dependency> | |||
<dependency> | |||
<groupId>com.github.jnr</groupId> |
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.
the license is tri EPL/GPL/LGPL license. Should be fine.
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.
Thanks for your review, and i have added com.github.jnr to GPLv2 module in LICENSE-binary file.
shadedclient is broken. That needs to be fixed. I am getting this error compiling with JDK8: [ERROR] Rule 0: org.apache.maven.enforcer.rules.dependency.DependencyConvergence failed with message: |
OK. My Jira account will be granted permissions soon, and I’ll create a new Jira right after that. |
I have excluded the conflicting dependencies related to the jnr-posix module,could you please help review this change again?Many thanks. |
@slfan1989 Could you please help review this PR? Thank you very much. |
LICENSE-binary
Outdated
@@ -498,6 +498,7 @@ org.slf4j:slf4j-reload4j:1.7.36 | |||
CDDL 1.1 + GPLv2 with classpath exception | |||
----------------------------------------- | |||
|
|||
com.github.jnr:jnr-posix:3.1.19 |
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.
actually it's not CDDL 1.1 + GPLv2 with classpath exception. Eclipse Public License 1/.02.0 is acceptable though. I think let's add a new section for "Eclipse Public License 2.0" under "Eclipse Public License 1.0"
For more details, check out Apache's thirdparty license policy: https://www.apache.org/legal/resolved.html
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.
Done.You are right, I added EPL 2.0 section.
💔 -1 overall
This message was automatically generated. |
Opened a new JIRA HADOOP-19329. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Eclipse Public License 2.0 | ||
-------------------------- | ||
|
||
com.github.jnr:jnr-posix:3.1.19 |
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.
it also pulls transitive jars.
Honestly, it's a bit heavy to pull bunches of jars to just work around the access of sun.misc.Signal
API. Could it be addressed by using reflection?
Description of PR
JIRA: HADOOP-19329. Remove usage of sun.misc.Signal
Fix compilation errors during the upgrade process, because sun.misc is not supported after jdk11.
we replace sun.misc.Signal and sum.misc.SignalHandler using jnr-posix
How was this patch tested?
exist ut test
For code changes:
replace sun.misc.Signal and sum.misc.SignalHandler using jnr.constants.platform.Signal and jnr.posix.SignalHandler in jnr-posix