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

[incubator-kie-issues-1616] notice file for drools is not correct #6153

Merged
merged 7 commits into from
Nov 18, 2024

Conversation

tkobayas
Copy link
Contributor

@tkobayas tkobayas commented Nov 15, 2024

Issue:

@tkobayas
Copy link
Contributor Author

tkobayas commented Nov 15, 2024

@LightGuard , as mentioned in apache/incubator-kie-issues#1616 (comment)

maybe, it would be best to get your mentors and possibly other IPMC members to review the PRs for license changes

Could you involve someone from mentors/IPMC for review?

@LightGuard
Copy link
Contributor

@pjfanning would you mind taking a look at this, please?

@LightGuard
Copy link
Contributor

@davsclaus and @oscerd it would be good to get your review here too.

Copy link
Contributor

@LightGuard LightGuard left a comment

Choose a reason for hiding this comment

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

My understanding from the ASF release policy document, we should be okay including the licenses as is done in this pull request. I think it is more common to see it in one single file, but it is also okay to have it in a different file, with a pointer (which I think we need to add).

When a package bundles code under several licenses, the LICENSE file MUST contain details of all these licenses. For each component which is not Apache licensed, details of the component MUST be appended to the LICENSE file. The component license itself MUST either be appended or else stored elsewhere in the package with a pointer to it from the LICENSE file, e.g. if the license is long.

drools-docs/ui-bundle/ui-bundle.zip : antora ui-bundle resources. For example, js and svg. See LICENSE-BINARY file
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we drop the docs stuff from the releases all together. @porcelli what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 for dropping drools-docs by the release CI, so we can reduce license concerns and avoid future issues.

LICENSE-BINARY Outdated
This section summarizes those components and their licenses. See licenses-binary/
for text of these licenses.

Mozilla Public License 2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we be including the license text here too, or at the very least a link to the license?

Choose a reason for hiding this comment

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

yes - any non Apache license - we need its text including any customisations like copyright statements

Choose a reason for hiding this comment

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

could we please list the files that are affected by these licenses?

I found:
incubator-kie-drools/drools-docs/supplemental-ui/css/search.css
incubator-kie-drools/drools-docs/supplemental-ui/js/search-ui.js

@porcelli
Copy link
Contributor

would love to hear your feedback here @pjfanning

LICENSE Outdated
Mozilla Public License 2.0
-----------

drools-docs/supplemental-ui/css/search.css

Choose a reason for hiding this comment

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

can you include the text of any non Apache Licenses?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pjfanning you mean copy here the whole license text from the file? basically a copy-n-paste from here https://www.mozilla.org/en-US/MPL/2.0/

Copy link
Contributor

Choose a reason for hiding this comment

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

@pjfanning note that part of this PR we have other files like LICENSE-search-ui.txt - so you suggest we remove those files and add everything here?

Choose a reason for hiding this comment

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

if you are not including the text directly here - you will need to provide the location of the file to check.

I'm not sure having separate files will be popular.

In most ASF source releases, we have one LICENSE file and maybe one directory with 3rd party licenses.

KIE source release has many subdirs - with different git repos in each subdir, each with its own license - and now these subdirs can have more subdirs with 3rd party libraries. This level of splitting up of the license data is kind of unprecedented.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pjfanning do you happen to have an example of license that has more than one license? sorry to insist and bother... but I'm trying to be assertive and avoid a RC5 :D

Copy link
Contributor

@porcelli porcelli left a comment

Choose a reason for hiding this comment

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

this looks very good, however I'd love to hear from @pjfanning - as I'm not experienced to properly evaluate

@LightGuard
Copy link
Contributor

https://github.com/apache/tomee/blob/main/LICENSE

Copy link
Contributor

@porcelli porcelli left a comment

Choose a reason for hiding this comment

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

great job @tkobayas, thank you for digging into this!

however based on @pjfanning input, we need make this slightly better

@tkobayas
Copy link
Contributor Author

tkobayas commented Nov 18, 2024

Thank you for reviewing!

Moved LICENSE-*.txt files into LICENSE / LICENSE-BINARY files. Affected file paths are listed. Following https://github.com/apache/tomee/blob/main/LICENSE style.

I hope this addresses the suggested points. Please review, thanks! @pjfanning @porcelli @LightGuard

Comment on lines +638 to +647
------------------------------------------------------------------------------------
for drools-decisiontables/src/main/java/org/drools/decisiontable/parser/csv/CsvLineParser.java

Inner logic adapted from a C++ original that was Copyright (C) 1999
Lucent Technologies Excerpted from 'The Practice of Programming' by Brian
Kernighan and Rob Pike.

Included by permission of the http://tpop.awl.com/ web site, which says:
"You may use this code for any purpose, as long as you leave the
copyright notice and book citation attached."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed one more issue, which was pointed at in https://lists.apache.org/thread/nrq50szwx37doopb23wyq33v0t3bbccg . CsvLineParser.java has this copyright, so removed Apache license header. This statement doesn't seem to meet with known licenses, but I think it fits in the LICENSE file. Let me know if this should be handled in another way.

FYI) http://tpop.awl.com/ no longer exists.

Choose a reason for hiding this comment

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

This could cause some trouble. I found the book online but I couldn't find any reference to this permission bit.
https://kremlin.cc/rob.pdf

We might need to bring this up on the legal discussion JIRA. https://issues.apache.org/jira/projects/LEGAL/issues

In the end of the day, the path of least resistance might be to find another CSV parser. E.g. commons-csv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the suggestion, @pjfanning

I have found this statement in this store site ( https://www.informit.com/store/practice-of-programming-9780201615869 ). Click "Downloads" tab. It contains the statement You may use this code for any purpose, as long as you leave the copyright notice and book citation attached..

Shall I open a JIRA in https://issues.apache.org/jira/projects/LEGAL/issues ?

Choose a reason for hiding this comment

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

Could you raise the JIRA? Finding the statement on that web site is very helpful though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pjfanning @porcelli I filed https://issues.apache.org/jira/browse/LEGAL-689 Feel free to add comments on it.

Copyright Red Hat, Inc. and/or its affiliates.. All Rights Reserved.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pjfanning it's not entirely clear to me, but isn't expected that the CSV parser copyright info moved here? (or it's ok to be on LICENSE file)

Choose a reason for hiding this comment

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

My experience has been bad with the cases like this where the code is not explicitly licensed or explicitly placed in the public domain. If https://www.apache.org/legal/resolved.html or existing LEGAL JIRAs don't cover it, it is best to get it checked by the Legal team.

Copy link
Contributor

Choose a reason for hiding this comment

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

k, but as this would be a big enough change for the immediate 10.0.0 release... I'd assume this wouldn't be a blocker...
In the meantime.. I'm just wondering if the NOTICE file wouldn't be more appropriated to host the copyright? As this seems to be a good example to relocated copyright.

Choose a reason for hiding this comment

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

I think we can open the LEGAL issue but press on. It is probably not a blocker to wait for the agreed answer.
The LICENSE needs to mention this usage so I'm not sure it is that useful to also include the copyright in the NOTICE but others might have another opinion.

@porcelli
Copy link
Contributor

@tkobayas I don't think the -BINARY files are needed in current form. As it only covers the search-ui thing that is used only by the docs module, which we don't distribute in any binary format.

@pjfanning thoughts?

@tkobayas
Copy link
Contributor Author

@porcelli Thanks. Probably I misunderstood the purpose of *-BINARY file. ui-bundle.zip exists in apache-kie-10.0.0-incubating-sources.zip, so I should write its license in LICENSE file rather than LICENSE-BINARY. Okay? @porcelli @pjfanning

@pjfanning
Copy link

@porcelli Thanks. Probably I misunderstood the purpose of *-BINARY file. ui-bundle.zip exists in apache-kie-10.0.0-incubating-sources.zip, so I should write its license in LICENSE file rather than LICENSE-BINARY. Okay? @porcelli @pjfanning

  • the zip is a problem - we shouldn't have binary files in the source release
  • can we unzip the file and check it the underlying files?

@tkobayas
Copy link
Contributor Author

tkobayas commented Nov 18, 2024

can we unzip the file and check it the underlying files?

$ unzip ui-bundle.zip 
Archive:  ui-bundle.zip
   creating: css/
   creating: font/
   creating: helpers/
   creating: img/
   creating: js/
   creating: layouts/
   creating: partials/
  inflating: css/site.css            
  inflating: font/roboto-latin-400.woff  
  inflating: font/roboto-latin-400.woff2  
  inflating: font/roboto-latin-400italic.woff  
  inflating: font/roboto-latin-400italic.woff2  
  inflating: font/roboto-latin-500.woff  
  inflating: font/roboto-latin-500.woff2  
  inflating: font/roboto-latin-500italic.woff  
  inflating: font/roboto-latin-500italic.woff2  
  inflating: font/roboto-mono-latin-400.woff  
  inflating: font/roboto-mono-latin-400.woff2  
  inflating: font/roboto-mono-latin-500.woff  
  inflating: font/roboto-mono-latin-500.woff2  
  inflating: helpers/and.js          
  inflating: helpers/detag.js        
  inflating: helpers/eq.js           
  inflating: helpers/increment.js    
  inflating: helpers/ne.js           
  inflating: helpers/not.js          
  inflating: helpers/or.js           
  inflating: helpers/relativize.js   
  inflating: helpers/year.js         
  inflating: img/back.svg            
  inflating: img/caret.svg           
  inflating: img/chevron.svg         
  inflating: img/home-o.svg          
  inflating: img/home.svg            
  inflating: img/menu.svg            
  inflating: img/octicons-16.svg     
  inflating: js/site.js              
   creating: js/vendor/
  inflating: layouts/404.hbs         
  inflating: layouts/default.hbs     
  inflating: partials/article-404.hbs  
  inflating: partials/article.hbs    
  inflating: partials/body.hbs       
  inflating: partials/breadcrumbs.hbs  
  inflating: partials/footer-content.hbs  
  inflating: partials/footer-scripts.hbs  
  inflating: partials/footer.hbs     
  inflating: partials/head-icons.hbs  
  inflating: partials/head-info.hbs  
  inflating: partials/head-meta.hbs  
  inflating: partials/head-prelude.hbs  
  inflating: partials/head-scripts.hbs  
  inflating: partials/head-styles.hbs  
  inflating: partials/head-title.hbs  
  inflating: partials/head.hbs       
  inflating: partials/header-content.hbs  
  inflating: partials/header-scripts.hbs  
  inflating: partials/header.hbs     
  inflating: partials/main.hbs       
  inflating: partials/nav-explore.hbs  
  inflating: partials/nav-menu.hbs   
  inflating: partials/nav-toggle.hbs  
  inflating: partials/nav-tree.hbs   
  inflating: partials/nav.hbs        
  inflating: partials/page-versions.hbs  
  inflating: partials/pagination.hbs  
  inflating: partials/toc.hbs        
  inflating: partials/toolbar.hbs    
  inflating: js/vendor/highlight.js 

ui-bundle.zip contains javascript, svg, font, css, hbs (html template). I wrote that in DISCLAIMER-BINARY.

the zip is a problem - we shouldn't have binary files in the source release

Is this a blocker to the 10.0.0 release? @pjfanning

If no, we would drop the drools-docs module from the source code distribution for the next release.
If yes, still I think it's better to drop the drools-docs module from the source code distribution for 10.0.x...

@pjfanning
Copy link

can we unzip the file and check it the underlying files?

$ unzip ui-bundle.zip 
Archive:  ui-bundle.zip
   creating: css/
   creating: font/
   creating: helpers/
   creating: img/
   creating: js/
   creating: layouts/
   creating: partials/
  inflating: css/site.css            
  inflating: font/roboto-latin-400.woff  
  inflating: font/roboto-latin-400.woff2  
  inflating: font/roboto-latin-400italic.woff  
  inflating: font/roboto-latin-400italic.woff2  
  inflating: font/roboto-latin-500.woff  
  inflating: font/roboto-latin-500.woff2  
  inflating: font/roboto-latin-500italic.woff  
  inflating: font/roboto-latin-500italic.woff2  
  inflating: font/roboto-mono-latin-400.woff  
  inflating: font/roboto-mono-latin-400.woff2  
  inflating: font/roboto-mono-latin-500.woff  
  inflating: font/roboto-mono-latin-500.woff2  
  inflating: helpers/and.js          
  inflating: helpers/detag.js        
  inflating: helpers/eq.js           
  inflating: helpers/increment.js    
  inflating: helpers/ne.js           
  inflating: helpers/not.js          
  inflating: helpers/or.js           
  inflating: helpers/relativize.js   
  inflating: helpers/year.js         
  inflating: img/back.svg            
  inflating: img/caret.svg           
  inflating: img/chevron.svg         
  inflating: img/home-o.svg          
  inflating: img/home.svg            
  inflating: img/menu.svg            
  inflating: img/octicons-16.svg     
  inflating: js/site.js              
   creating: js/vendor/
  inflating: layouts/404.hbs         
  inflating: layouts/default.hbs     
  inflating: partials/article-404.hbs  
  inflating: partials/article.hbs    
  inflating: partials/body.hbs       
  inflating: partials/breadcrumbs.hbs  
  inflating: partials/footer-content.hbs  
  inflating: partials/footer-scripts.hbs  
  inflating: partials/footer.hbs     
  inflating: partials/head-icons.hbs  
  inflating: partials/head-info.hbs  
  inflating: partials/head-meta.hbs  
  inflating: partials/head-prelude.hbs  
  inflating: partials/head-scripts.hbs  
  inflating: partials/head-styles.hbs  
  inflating: partials/head-title.hbs  
  inflating: partials/head.hbs       
  inflating: partials/header-content.hbs  
  inflating: partials/header-scripts.hbs  
  inflating: partials/header.hbs     
  inflating: partials/main.hbs       
  inflating: partials/nav-explore.hbs  
  inflating: partials/nav-menu.hbs   
  inflating: partials/nav-toggle.hbs  
  inflating: partials/nav-tree.hbs   
  inflating: partials/nav.hbs        
  inflating: partials/page-versions.hbs  
  inflating: partials/pagination.hbs  
  inflating: partials/toc.hbs        
  inflating: partials/toolbar.hbs    
  inflating: js/vendor/highlight.js 

ui-bundle.zip contains javascript, svg, font, css, hbs (html template)

the zip is a problem - we shouldn't have binary files in the source release

Is this a blocker to the 10.0.0 release? @pjfanning

If no, we would drop the drools-docs module from the source code distribution for the next release. If yes, still I think it's better to drop the drools-docs module from the source code distribution for 10.0.x...

Can we exclude it from the sources zip and come back later? It could easily attract a -1 vote by including it. And in longer run, it needs to be removed and the various source files in the archive should be checked in explicitly. If you need to zip thm up when you are building from source, that's ok - that should be easy to add to your build scripting.

@tkobayas
Copy link
Contributor Author

tkobayas commented Nov 18, 2024

Okay, I removed drools-docs related licenses, as we will not have drools-docs module in source distributions. Now it's much cleaner.

For 10.0.x, I'll raise a PR to remove drools-docs -> #6155
For later releases, I will check with CI experts to exclude drools-docs by the release CI job.
Is it fine? @porcelli

@tkobayas
Copy link
Contributor Author

https://issues.apache.org/jira/browse/LEGAL-689

Justin Mclean:
I would just note the file and statement in the LICENSE file

So the current PR seems to be good.

@tkobayas
Copy link
Contributor Author

@pjfanning @LightGuard @porcelli If this PR is in a good shape under the current situation, please approve and merge, thanks!

@porcelli porcelli merged commit fa989e5 into apache:main Nov 18, 2024
8 of 10 checks passed
rgdoliveira pushed a commit to rgdoliveira/drools that referenced this pull request Nov 18, 2024
…ache#6153)

* [incubator-kie-issues-1616] notice file for drools is not correct

* fix .rat-excludes

* consolidate LICENSE files to single LICENSE file

* fixed missing license header

* fix CsvLineParser license

* exclude drools-docs related licenses, because they will not be included in source distoribution
tkobayas added a commit to tkobayas/drools that referenced this pull request Nov 21, 2024
…ache#6153)

* [incubator-kie-issues-1616] notice file for drools is not correct

* fix .rat-excludes

* consolidate LICENSE files to single LICENSE file

* fixed missing license header

* fix CsvLineParser license

* exclude drools-docs related licenses, because they will not be included in source distoribution
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.

5 participants