Skip to content
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

Update LICENSE and NOTICE files #3722

Merged
merged 2 commits into from
Jan 31, 2019
Merged

Update LICENSE and NOTICE files #3722

merged 2 commits into from
Jan 31, 2019

Conversation

snleee
Copy link
Contributor

@snleee snleee commented Jan 18, 2019

Followed Apache Spark's approach for handling LICENSE, NOTICE.
apache/spark@f825847

Followed Apache Spark's approach for handling LICENSE, NOTICE.
apache/spark@f825847

- Checked licenses for all libraries that we bundle
- Checked licenses for all js/css files that we bundle
- Removed dependencies of category X libraries (#3718)
@snleee
Copy link
Contributor Author

snleee commented Jan 18, 2019

After removing category X dependency and removing "pinot-perf" from distribution package, we are bundling 109 external jar libraries and 9 js/css libraries within the source code. After this pr, anyone who's changing version of any library or adding a new library needs to update NOTICE & LICENSE files.

The extensive list is present at the following spreadsheet https://docs.google.com/spreadsheets/d/1lMEEvrA8SqHe0Yy0yC2Nd-nN5lg5sPaNwihrj_--y8Q/edit?usp=sharing

@snleee
Copy link
Contributor Author

snleee commented Jan 18, 2019

@felixcheung @olamy Can you check this when you find the time?

@codecov-io
Copy link

codecov-io commented Jan 19, 2019

Codecov Report

Merging #3722 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3722      +/-   ##
============================================
+ Coverage     67.18%   67.21%   +0.02%     
  Complexity        4        4              
============================================
  Files          1027     1027              
  Lines         50830    50830              
  Branches       7093     7093              
============================================
+ Hits          34150    34165      +15     
+ Misses        14336    14324      -12     
+ Partials       2344     2341       -3
Impacted Files Coverage Δ Complexity Δ
...a/manager/realtime/RealtimeSegmentDataManager.java 50% <0%> (-25%) 0% <0%> (ø)
...egation/function/customobject/MinMaxRangePair.java 75.86% <0%> (-24.14%) 0% <0%> (ø)
...e/impl/dictionary/LongOnHeapMutableDictionary.java 82.22% <0%> (-13.34%) 0% <0%> (ø)
...ller/validation/OfflineSegmentIntervalChecker.java 63.73% <0%> (-12.09%) 0% <0%> (ø)
...pache/pinot/core/util/SortedRangeIntersection.java 83.82% <0%> (-7.36%) 0% <0%> (ø)
...mpl/dictionary/DoubleOffHeapMutableDictionary.java 70.9% <0%> (-7.28%) 0% <0%> (ø)
...e/operator/dociditerators/SortedDocIdIterator.java 59.45% <0%> (-5.41%) 0% <0%> (ø)
...elix/core/relocation/RealtimeSegmentRelocator.java 71.42% <0%> (-3.07%) 0% <0%> (ø)
...ation/function/MinMaxRangeAggregationFunction.java 73.25% <0%> (-1.17%) 0% <0%> (ø)
...lix/core/realtime/PinotRealtimeSegmentManager.java 73.82% <0%> (-1.05%) 0% <0%> (ø)
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f09db48...b1dd3af. Read the comment docs.

@Jackie-Jiang
Copy link
Contributor

Awesome! You make my OCD so satisfied!

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the binary releases may contain binary code from third parties. This is very much true for Scala and Java, if binary release which includes all transitive binary dependencies

### Apache License 2

The Apache License 2 variants are typically easiest to deal with as they will not require you to modify LICENSE, nor add to license/. It's still good form to list the ALv2 dependencies in LICENSE for completeness, but optional.

some JS files copy in other JS files! Look out for Modernizr.

# One More Thing: JS and CSS in Binary Release

Now that you've got a handle on source licenses, recall that all the JS and CSS source code will *also* be part of the binary release. Copy that info from source to binary license files accordingly.

LICENSE Outdated Show resolved Hide resolved
NOTICE Outdated Show resolved Hide resolved
@kishoreg
Copy link
Member

+100 for doing this.

qq: why do we need to create a license file for each library? for e.g. args4j is distributed under MIT license. https://github.com/kohsuke/args4j. We can just list this library in the NOTICE file under MIT licenses section rt?

@snleee
Copy link
Contributor Author

snleee commented Jan 21, 2019

@kishoreg In my understanding, we have 2 approaches.

  1. copy license in /licenses and point that in LICENSE
  2. copy license directly to LICENSE file
    Which approach do you prefer?

@felixcheung Please correct me if I'm wrong.

@felixcheung
Copy link
Member

felixcheung commented Jan 22, 2019

@s-sahay
Copy link

s-sahay commented Jan 22, 2019

After removing category X dependency and removing "pinot-perf" from distribution package, we are bundling 109 external jar libraries and 9 js/css libraries within the source code. After this pr, anyone who's changing version of any library or adding a new library needs to update NOTICE & LICENSE files.

The extensive list is present at the following spreadsheet https://docs.google.com/spreadsheets/d/1lMEEvrA8SqHe0Yy0yC2Nd-nN5lg5sPaNwihrj_--y8Q/edit?usp=sharing

Great job on getting this done! It would be good to have the google doc linked either on the cwiki or the RTD(may be in the Developer Guide section).

@snleee
Copy link
Contributor Author

snleee commented Jan 22, 2019

My take is if it's MIT license it needs not be included:
http://www.apache.org/dev/licensing-howto.html
http://www.apache.org/dev/licensing-howto.html#permissive-deps
https://www.apache.org/legal/resolved.html#required-third-party-notices

@felixcheung @kishoreg

Bundling a dependency which is issued under one of the following licenses is straightforward, assuming that said license applies uniformly to all files within the dependency:

BSD (without advertising clause)
MIT/X11
In LICENSE, add a pointer to the dependency's license within the distribution and a short note summarizing its licensing:

This product bundles SuperWidget 1.2.3, which is available under a
"3-clause BSD" license.  For details, see deps/superwidget/.

It seems that the pointer is needed for BSD, MIT licenses. The example above has the link pointer to the full license deps/superwiget. Am I reading this incorrectly?

As long as we don't violate the license issue, I can go ahead for removing copied license files from /licenses for BSD, MIT.

@felixcheung
Copy link
Member

(I think we exchange email on this, we might need to put MIT license under LICENSES/)

@snleee
Copy link
Contributor Author

snleee commented Jan 29, 2019

@felixcheung The current pr includes the license with MIT and BSD under /licenses. I also dealt with license vs license-binary. If you don't have any other concerns, would you give me a shipit along with the final check?

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have the build change to include LICENSE-binary in the binary release tarball?

pinot-controller/src/main/resources/*/css/lib/codemirror*
pinot-controller/src/main/resources/*/css/lib/foundation*
pinot-controller/src/main/resources/*/css/lib/normalize.css
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since I'm not too familiar with this - are we saying these js css files are checked into the git repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those files are the part of the git repo (they are used for some UI components. e.g. pinot query console). After checking in this, I will file a separate PR for handling the build to include LICENSE-binary for the binary release.

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx, LGTM except for the release process comment

@snleee snleee merged commit 96d9c22 into master Jan 31, 2019
@snleee snleee deleted the update-license branch January 31, 2019 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants