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

Jump to bottom

KAFKA-12625: Fix the NOTICE file #10693

Merged

vvcephei merged 1 commit into apache:trunk from jlprat:KAFKA-12625-fix-notice-file

Jul 29, 2021

Contributor

jlprat commented May 14, 2021

Adds new NOTICE-binary file and packages it in the binary release
This follows up the #10474 pull where LICENSE was fix.

Similarly as in that PR, I do not know if this is correct, and I would add the same disclaimer @vvcephei did: "Please make no assumption that I know what I'm doing and let me know if anything seems wrong."

I went through all jar files within the distribution file and copied the content of any existing NOTICE file.

Notes:

For the cases where several dependencies including the same NOTICE, I added it only once.
There are cases where the jar included in Kafka's distribution file doesn't contain any NOTICE file, hence nothing was copied. These are:
- activation-1.1.1.jar
- argparse4j-0.7.0.jar
- jackson-annotations-2.10.5.jar
- jackson-dataformat-csv-2.10.5.jar
- jackson-datatype-jdk8-2.10.5.jar
- jackson-jaxrs-base-2.10.5.jar
- jackson-module-scala_2.13-2.10.5.jar
- jakarta.validation-api-2.0.2.jar
- javassist-3.27.0-GA.jar
- javax.servlet-api-3.1.0.jar
- javax.ws.rs-api-2.1.1.jar
- jaxb-api-2.3.0.jar
- jline-3.12.1.jar
- jopt-simple-5.0.4.jar
- lz4-java-1.7.1.jar
- metrics-core-2.2.0.jar
- netty-buffer-4.1.62.Final.jar
- netty-codec-4.1.62.Final.jar
- netty-common-4.1.62.Final.jar
- netty-handler-4.1.62.Final.jar
- netty-resolver-4.1.62.Final.jar
- netty-transport-4.1.62.Final.jar
- netty-transport-native-epoll-4.1.62.Final.jar
- netty-transport-native-unix-common-4.1.62.Final.jar
- reflections-0.9.12.jar
- rocksdbjni-6.19.3.jar
- scala-collection-compat_2.13-2.3.0.jar
- scala-java8-compat_2.13-0.9.1.jar
- scala-logging_2.13-3.9.2.jar
- slf4j-api-1.7.30.jar
- slf4j-log4j12-1.7.30.jar
- snappy-java-1.1.8.1.jar
- zstd-jni-1.4.9-1.jar
For some of those NOTICE-missing-in-jar dependencies, there were other dependencies from the same project that included a NOTICE file, for example, the jackson ones.
For the Netty project, I manually copied their NOTICE file from github

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)


          KAFKA-12625: Fix the NOTICE file

ca52c38

Adds new NOTICE-binary file and packages it in the binary release

Contributor Author

jlprat commented May 14, 2021

I would highly appreciate any hint on how to proceed for the dependencies that do not include a notice file in their jar files but might include them in their source code repositories.

jlprat reviewed

View changes

LICENSE-binary

  
          @@ -243,7 +243,6 @@ netty-handler-4.1.59.Final
        
            netty-resolver-4.1.59.Final

            netty-transport-4.1.59.Final

            netty-transport-native-epoll-4.1.59.Final

            netty-transport-native-epoll-4.1.59.Final

Contributor Author

jlprat May 14, 2021 •

edited

I removed this line from LICENSE-binary as it was a duplicate one

Contributor Author

jlprat commented May 14, 2021

Failures were https://issues.apache.org/jira/browse/KAFKA-9009 and https://issues.apache.org/jira/browse/KAFKA-12629

Contributor Author

jlprat commented May 21, 2021

@vvcephei As you were the last one touching that area, would you like to review this PR?

Contributor Author

jlprat commented May 31, 2021

Hi @ableegoldman as you reviewed the last change in that area (fix license files) do you think you can review this one? Thanks in advance!

Contributor

ableegoldman commented Jun 1, 2021

Hey @jlprat , can you elaborate on (a) the motivation behind adding this NOTICE-binary file (are we missing licenses completely? or we had them but not in the correct format? etc), (b) why you put this in a new NOTICE-binary file instead of the existing NOTICE file (do we need both? if we do add this NOTICE-binary then should we remove some or all of the NOTICE file contents?), and (c) how the NOTICE-binary file relates to the existing stuff under licenses/

I'll extend the same disclaimer, make no assumption that I know what I'm talking about 🙂

Contributor Author

jlprat commented Jun 1, 2021

Thanks for looking at it @ableegoldman ,
I'll do my best explaining:

a) I added a NOTICE-binary file following the same pattern done for the LICENSE patch. The purpose of the NOTICE-binary is to be included in the distribution file only.
b) The content of our NOTICE is what Kafka declares, that I left untouched. The purpose of the NOTICE-binary is to collect all notices from all our dependencies and paste them there.
c) I'm no expert at all, but I'd say the licenses folder was merely to collect all the different texts of licences so we could reference them in our LICENSE-binary (I think licenses say their text needs to be included)

I looked at https://github.com/apache/hadoop and attempted to use the same approach.

On short, NOTICE contains Kafka's notices while NOTICE-binary contains Kafka's and its dependencies' notices so it can be packaged in our distribution files.

ableegoldman requested review from vvcephei and ijuma

Contributor

ableegoldman commented Jun 7, 2021

Thanks. We just went through a large cleanup of the licensing and notices for the 2.8.0 release, which is how we ended up in the current state of things. It was my understanding that we had reached a place in which all the necessary pieces were being included in the correct place, which is why I was asking what the motivation for making further changes was. I'll let @vvcephei chime in from here

Contributor Author

jlprat commented Jun 8, 2021

Thanks @ableegoldman for your comments.

As far as I can tell after reading https://issues.apache.org/jira/browse/KAFKA-12625, mostly the copyright files were worked on, but NOTICE file was still not done yet. Or at least not completely.
Let's see if @vvcephei can clarify things.

vvcephei approved these changes

View changes

Contributor

vvcephei left a comment

Hey, all, sorry I was so slow in responding.

IIRC, what happened in 2.8 was that we invested a ton of time into fixing the license and copyright notices, and decided that the notice files weren't to the same level of urgency to continue blocking the ongoing releases. To be honest, I meant to follow up on the notice files myself, but lost the thread. Thanks to @jlprat for picking it up!

vvcephei merged commit 52f87e2 into apache:trunk

6 of 10 checks passed

vvcephei pushed a commit that referenced this issue


          KAFKA-12625: Fix the NOTICE file (#10693)

a146cb2

Adds new NOTICE-binary file and packages it in the binary release

Contributor Author

jlprat commented Jul 29, 2021

Thanks @vvcephei for the review!

jlprat deleted the KAFKA-12625-fix-notice-file branch

xdgrulez pushed a commit to xdgrulez/kafka that referenced this issue


          KAFKA-12625: Fix the NOTICE file (apache#10693)

3d79d1a

Adds new NOTICE-binary file and packages it in the binary release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment