Include NOTICE+LICENSE in every jar #10331
Conversation
|
Review note: run |
snazy
force-pushed
the
license-notice-again
branch
from
4063298 to
0fa20eb
Compare
snazy
force-pushed
the
license-notice-again
branch
from
0fa20eb to
25a47d9
Compare
snazy
force-pushed
the
license-notice-again
branch
from
25a47d9 to
e6f397a
Compare
There was a problem hiding this comment.
It's a way better change. For clarity, I would rename the LICENSE/NOTICE specific to docker (and runner fat jar). However:
- in
NOTICE, I would remove the list of license for transitive deps. ThisNOTICEfile is for source distribution, so dependencies don't matter here. Basically, I would remove this part:
This product has transitive dependencies to additional software licensed
under the terms of the following licenses:
* Apache Software License, Version 2.0
* Creative Commons 1.0 Universal
* Eclipse Distribution License, Version 1.0
* Eclipse Public License, Version 1.0
* Eclipse Public License, Version 2.0
See also the LICENSE file.
The Nessie web site provides aggregated license reports for recent Nessie
binary distributions, available at https://projectnessie.org/docs/ and
https://projectnessie.org/downloads/.
This product includes software developed at
The Apache Software Foundation (http://www.apache.org/).
- The
LICENSE-BINARY-DIST/NOTICE-BINARY-DISTis only valid for container images (not sure about the runner fat jar). It's not correct for jar files. I would rename to be explicit likeLICENSE-CONTAINER-DIST. - The distributed nessie jar files should include a clean
META-INF/LICENSE/META-INF/NOTICE. Currently, the nessie jar files don't includeLICENSE/NOTICE.
Adds the contents of `LICENSE` and `NOTICE` to every jar using the following patterns: * `META-INF/licenses/<maven-group-id>/<maven-artifact-id>-<version>/LICENSE` * `META-INF/licenses/<maven-group-id>/<maven-artifact-id>-<version>/NOTICE` Also explicitly clarifies where dependencies only licensed under "GNU General Public License, Version 2 with the GNU Classpath Exception" are used (included in binary executable and containers).
snazy
force-pushed
the
license-notice-again
branch
from
e6f397a to
21b4d21
Compare
| ### Note | ||
|
|
||
| - This release has no code changes. | ||
| - NOTICE and LICENSE files clarifications, included in jars published to Maven Central. |
There was a problem hiding this comment.
nit: included in published jars... technically, they are included inside the jars, inside the docker images too.
There was a problem hiding this comment.
This mention is (only) about /NOTICE + /LICENSE
| This product includes software developed at | ||
| The Apache Software Foundation (http://www.apache.org/). | ||
|
|
||
| --- |
There was a problem hiding this comment.
Use the same separator and general text style as the root NOTICE?
There was a problem hiding this comment.
i'll leave something to follow-up :P
There was a problem hiding this comment.
Nessie distributions contain some or all of the following dependencies (in the unchanged section) -- I believe the language has to be exact. We have to be sure what we include, otherwise how can claim that our license is correct? @jbonofre WDYT?
If we want to use the same text for all binary artifacts (each having a different sub-set of bundled dependendencies) how about this text:
Nessie distributions contain only dependencies licensed under the Apache License.
The following is the list of all redistributed dependencies unified across all published
Nessie artifacts. Each Nessie artifact bundles a sub-set of those dependencies.
WDYT?
There was a problem hiding this comment.
Yes, as showed in my "illustration PR", we should be explicit.
I think it's OK as a quick "fix" but that should be improved for sure.
There was a problem hiding this comment.
We need the list for the automatic license check
There was a problem hiding this comment.
Yes, I'm suggesting clarifying the text as it relates to what is possibly bundled.
| @@ -36,6 +36,7 @@ dependencies { | |||
| compileOnly("org.apache.spark:spark-core_${sparkScala.scalaMajorVersion}") { forSpark(sparkScala.sparkVersion) } | |||
| compileOnly("org.apache.spark:spark-hive_${sparkScala.scalaMajorVersion}") { forSpark(sparkScala.sparkVersion) } | |||
| implementation(nessieClientForIceberg()) | |||
| implementation(nessieProject("nessie-notice")) | |||
There was a problem hiding this comment.
LICENSE and NOTICE are included in the jar under /META-INF/resources/... why not directly under META-INF (e.g. Guava has it there)?
There was a problem hiding this comment.
The "regular" place is META-INF/LICENSE and META-INF/NOTICE (at least it's what some maven plugins like shade plugin are looking for).
There was a problem hiding this comment.
META-INF/LICENSE and META-INF/NOTICE
Pretty bad for uber-jars - everything overwrites everything
There was a problem hiding this comment.
true, but Nessie has a specific binary NOTICE for uber jars, right?
There was a problem hiding this comment.
That's not what LICENSE + NOTICE are for.
There was a problem hiding this comment.
The "binary uber/container/..." distributables get theirs in /META-INF/resources/LICENSE.txt (and NOTICE.txt)
|
Which profile should be used to see |
|
Adds the contents of
LICENSEandNOTICEto every jar using the following patterns:META-INF/licenses/<maven-group-id>/<maven-artifact-id>-<version>/LICENSEMETA-INF/licenses/<maven-group-id>/<maven-artifact-id>-<version>/NOTICEAlso explicitly clarifies where dependencies only licensed under "GNU General Public License, Version 2 with the GNU Classpath Exception" are used (included in binary executable and containers).