-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-25473 [create-release] checkcompatibility.py failing with "KeyE… #2862
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +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.
I think excluding the shaded jar is not a good idea. Or maybe I don't understand the intended audience of this jar.
"${project}"/dev-support/checkcompatibility.py --annotation \ | ||
org.apache.yetus.audience.InterfaceAudience.Public \ | ||
-e "original-hbase.*.jar" \ | ||
-e "hbase-shaded-testing-util.*.jar" \ |
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.
excluding this jar is a problem -- the shaded jars are intended explicitly as our public interface to down-streamers, aren't they?
What if we add an exclude of the [lL][iI][cC][eE][nN][sS][eE]
pattern instead? oh, I see the jar
command doesn't support exclude patterns, only an include pattern. And where does this happen, in the perl script?
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.
Include/exclude are in the python script. Does jar files only. The jar files are subsequently passed to the wrapped perl script... which then does unzip... and fails with hbase-shaded-testing-util.
I think it ok excluding this shaded jar because it is made from other jars that are NOT excluded; if an api change violation, it'll be caught there, at the source. Testing the shade would be more comprehensive yes but exclude is fine for now given it allows us getting the RC ball rolling again. The reason-for-exclusion is for fix in HBASE-25486
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.
Include/exclude are in the python script. Does jar files only. The jar files are subsequently passed to the wrapped perl script... which then does unzip... and fails with hbase-shaded-testing-util.
I meant exclude of unpacked archive content, not entire archives. And yes, that's what I feared, that the unpack happens in the perl script, "outside of our control."
I'm thinking the reason to retain the shaded jar is to notice if we break shading entirely. i.e., a change that adds a new jar to the shaded package would show in the report as loads of new classes. Perhaps we have other mechanisms in place already for catching these kinds of errors?
I like HBASE-25486.
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 thinking the reason to retain the shaded jar is to notice if we break shading entirely. i.e., a change that adds a new jar to the shaded package would show in the report as loads of new classes. Perhaps we have other mechanisms in place already for catching these kinds of errors?
This is a valid concern. Lets fix HBASE-25486 and get the shaded jar back in the loop.
…rror: 'binary'" Exclude hbase-shaded-testing-util*.jar from checkcompatibility; this jar can not be unzipped on a case-insensitive filesystem. Added some means of debug into the checkcompatibility to help when cryptic failures.
0b22ae0
to
dd8dc7e
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +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.
Thank you sir, better than you found it!
…rror: 'binary'"
Exclude hbase-shaded-testing-util*.jar from checkcompatibility; this
jar can not be unzipped on a case-insensitive filesystem. Added
some means of debug into the checkcompatibility to help when
cryptic failures.