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-16895. [thirdparty] Revisit LICENSEs and NOTICEs #6
Conversation
@aajisaka Could you please review this? |
Source releaseWe don't have to include the dependencies in LICENSE and NOTICE files. Binary releaseLICENSE-binary and NOTICE-binary should contain protobuf, jaegartracing, and all their dependencies (such as gson). In addition, we should include LICENSE-binary and NOTICE-binary files in the jar files. |
Thanks @aajisaka, I will update accordingly. |
818c189
to
0b57a3f
Compare
@aajisaka Updated the PR to include License-binary and NOTICE-binary as LICENCE.txt and NOTICE.txt and all other Non-ALV2 licenses under /META-INF/licences-binary inside shaded jars. Please review. |
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.
+1
Hi @elek, would you check this?
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 very much to create this patch @vinayakumarb.
TLDR; Overall it looks good to me, but I would do a cp NOTICE.txt NOTICE-binary
.
In more details:
I learned all of my knowledge about licensing from the workshops from Justin Mclean. I might be wrong (If I misunderstood something), but here is what is my understanding.
-
source package: we don't package any external code here (ideally Yetus is excluded), so the current LICENSE/NOTICE looks good.
-
binary packages: we don't have the traditional tar binary package, just maven artifacts.
As a main rule, we need separated LICENSE.txt and mention the LICENSE of all the included files
Current approach seems to be fine:
unzip -l hadoop-shaded-jaeger/target/hadoop-shaded-jaeger-1.1.0-SNAPSHOT.jar
unzip -p hadoop-shaded-jaeger/target/hadoop-shaded-jaeger-1.1.0-SNAPSHOT.jar META-INF/LICENSE.txt
unzip -l hadoop-shaded-protobuf_3_7/target/hadoop-shaded-protobuf_3_7-1.1.0-SNAPSHOT.jar
unzip -p hadoop-shaded-protobuf_3_7/target/hadoop-shaded-protobuf_3_7-1.1.0-SNAPSHOT.jar META-INF/LICENSE.txt
They seems to be in sync, I can see only packages which are mentioned in the LICENSE.
binary licenses are also included:
unzip -l hadoop-shaded-protobuf_3_7/target/hadoop-shaded-protobuf_3_7-1.1.0-SNAPSHOT.jar | grep lice
Strictly speaking we have no protobuf in the jaeger jar and no jaeger in the protobuf file but I don't think it's a big problem.
- NOTICE file
We maintain NOTICE file only because it should be done according to the ASF license:
4.d If the Work includes a "NOTICE" text file as part of its distribution, then any Derivative Works that You distribute must include a readable copy of the attribution notices contained within such NOTICE file, excluding those notices that do not pertain to any part of the Derivative Works, in at least one of the following places: within a NOTICE text file distributed as part of the Derivative Works; within the Source form or documentation, if provided along with the Derivative Works; or, within a display generated by the Derivative Works, if and wherever such third-party notices normally appear
- We don't need to do anything with an external NOTICE file if we use something which is not under Apache license. (BSD, MIT licenses don't ask us to do anything. We don't need to add any content).
- If the used library is Apache AND it contains a NOTICE, we should copy the content of the external NOTICE to our NOTICE.
As I see we use multiple Apache dependencies, but I can't see a NOTICE in any of them. Therefore we can keep NOTICE-binary as empty as the NOTICE of the src.
(Strictly speaking there is one other usage of NOTICE. During the donation of the code to Apache, some notices from the sources can be moved to the NOTICE file, but it almost never happens...)
0b57a3f
to
bdbffa5
Compare
Thanks @elek
Hope I have addressed all your concerns. Please review. |
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.
+1 LGTM
Thanks the update @vinayakumarb You were more precious than me.
Will commit this tomorrow morning if nobody else.
bdbffa5
to
a11c32c
Compare
Thank you very much the work @vinayakumarb (And sorry for the occasionally delayed answer...) |
addendum to avoid rat check failure for NOTICE-binary
addendum to avoid rat check failure for NOTICE-binary
No description provided.