-
Notifications
You must be signed in to change notification settings - Fork 136
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
Fix LICENSE/LICENSE.bin for release #495
base: main
Are you sure you want to change the base?
Conversation
jbonofre
commented
Nov 28, 2024
- Upgrade to DropWizard 4.0.11
- Upgrade to Hadoop 3.4.1
- Fix LICENSE to be included in source distribution
- Add LICENSE.bin targeted for binary distribution (will be added in another commit)
- Exclude GPL dependencies
- Upgrade to Hadoop 3.4.1 - Fix LICENSE to be included in source distribution - Add LICENSE.bin targeted for binary distribution (will be added in another commit) - Exclude GPL dependencies
@@ -41,6 +41,11 @@ dependencies { | |||
exclude("ch.qos.reload4j", "reload4j") | |||
exclude("log4j", "log4j") | |||
exclude("org.apache.zookeeper", "zookeeper") | |||
exclude("org.apache.hadoop.thirdparty", "hadoop-shaded-protobuf_3_25") |
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.
Hadoop's an Apache project, why these excludes?
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 excluded because it's not actually used in Polaris.
11. BECAUSE THE PROGRAM IS LICENSED FREE OF CHARGE, THERE IS NO WARRANTY FOR THE PROGRAM, TO THE EXTENT PERMITTED BY APPLICABLE LAW. EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS" WITHOUT WARRANTY OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE OF THE PROGRAM IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION. | ||
|
||
12. IN NO EVENT UNLESS REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING WILL ANY COPYRIGHT HOLDER, OR ANY OTHER PARTY WHO MAY MODIFY AND/OR REDISTRIBUTE THE PROGRAM AS PERMITTED ABOVE, BE LIABLE TO YOU FOR DAMAGES, INCLUDING ANY GENERAL, SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE USE OR INABILITY TO USE THE PROGRAM (INCLUDING BUT NOT LIMITED |
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.
Isn't this included in the sentence above? gradlew
is AL2 - and gradle-wrapper.properties
is actually "ours".
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.
GH placed the comment wrong - it's for the "Gradle sentence" at the top
@@ -18,9 +18,9 @@ | |||
# | |||
|
|||
[versions] | |||
hadoop = "3.4.0" | |||
hadoop = "3.4.1" |
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.
Can we do the version bumps in a separate PR?
Should actually be done by renovate ;)
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.
Sure, I can do in a separate PR. The reason why I included here it's because the transitive dependencies changed a bit.
@@ -58,7 +58,7 @@ class LicenseFileValidation : DependencyFilter { | |||
override fun filter(data: ProjectData?): ProjectData { | |||
data!! | |||
|
|||
val rootLicenseFile = data.project.rootProject.file("LICENSE").readText() | |||
val rootLicenseFile = data.project.rootProject.file("LICENSE.bin").readText() |
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.
.bin
implies it's a binary file - maybe DEPENDENCY-LICENSES.txt
or just DEPENDENCY-LICENSES
?
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.
That's correct: LICENSE.bin
is targeted to be included in binary distributions (like polaris-service
tar/zip).
No problem to rename as LICENSE-BINARY-DEPENDENCY
for instance.
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.
Then let's rename LICENSE.bin
to DEPENDENCY-LICENSE
, because .bin
usually means "this" is a binary file.
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.
This PR creates a new license file. Do we need two license files? I think the one including the dependencies license is good enough.
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.
Yes we need two because we have a source distribution and a binary distribution.
If we don't distribute the binary distribution then only LICENSE would be enough. But as we have the polaris-service tar, to distribute it, we need the license for binaries included in this distribution.
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.
For binary distribution, do we need to aggregate all third-party licenses into a text file, as the third party jar files have them already?
org.slf4j:jul-to-slf4j | ||
org.slf4j:slf4j-api | ||
|
||
Copyright (c) 2004-2022 QOS.ch Sarl (Switzerland) | ||
All rights reserved. | ||
|
||
Permission is hereby granted, free of charge, to any person obtaining | ||
a copy of this software and associated documentation files (the | ||
"Software"), to deal in the Software without restriction, including | ||
without limitation the rights to use, copy, modify, merge, publish, | ||
distribute, sublicense, and/or sell copies of the Software, and to | ||
permit persons to whom the Software is furnished to do so, subject to | ||
the following conditions: | ||
|
||
The above copyright notice and this permission notice shall be | ||
included in all copies or substantial portions of the Software. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF | ||
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND | ||
NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE | ||
LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION | ||
OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION | ||
WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
|
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.
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.
if we have to include the third party license, can we use the link to save the space like what Iceberg does?
This product includes code from Apache Hive.
* Hive metastore derby schema in hive-schema-3.1.0.derby.sql
Copyright: 2011-2018 The Apache Software Foundation
Home page: https://hive.apache.org/
License: https://www.apache.org/licenses/LICENSE-2.0
https://github.com/apache/iceberg/blob/feed4e2544b5839fbc2fe040965af3906d053302/LICENSE#L273-L273
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.
We ship slf4j in the binary distribution. No need to include the License only copy the notice content (according to alv2 4d section).
Iceberg is different as it's only source distribution (not binary). So it mentions only the source code coming from other projects.
Just to be clear: I'm pretty sure of my changes here (especially for the source distribution). To license and notice of the binary distribution are generated by the gradle plugin.
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.
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.
Iceberg is different as it's only source distribution (not binary). So it mentions only the source code coming from other projects.
Should we do that for the Polaris source distribution? Oh, I guess this statement does the job. We should be fine?
This product bundles various third-party components also under the
Apache Software License 2.0.
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.
LGTM. Thanks @jbonofre for driving this.