Skip to content

Conversation

matthiasblaesing
Copy link
Contributor

With the recent release vote concerns about bundled sourcefiles were raised.
This PR tries to address these concerns. Please see the individual commits for
details.

The primary point is the introduction of a licenseinfo.xml file, which addresses
individual files and marks these with their respective licenses.

This file is then used to:

  • add info the LICENSE file about the sourcefile belonging to the release
  • ensure license is present when build
  • exclude the referenced files from rat check (they would fail it)
  • create a nbbuild/build/rat-licenseinfo.txt with license and comment info for reviewers

Some modules where converted from the classical rat excludes to this approach,
some files were fixed by adding the license header or removing them.

<file>src/org/netbeans/modules/css/resources/icons/safari20-disabled.png</file>
<file>src/org/netbeans/modules/css/resources/icons/safari20.png</file>
<license ref="CDDL-1.0" />
<comment>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this is correct, but someone else will need to comment I guess (e.g. I don't know how where these icons were from in neither repository).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reasoning: the icons are present in the distributed packages from oracle. There are various license statements, but I did not find statements addressing the icons specifically. So I deduce, that they are distributed under the global GPLv2-CDDL license and from that only the CDDL can even be considered for Apache NetBeans.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand what you are saying, but I don't know if that's correct (sorry).

@jlahoda
Copy link
Contributor

jlahoda commented Jan 28, 2018

Thanks for doing this! I didn't went through everything in detail yet, but overall seems good (maybe except the icons, where I don't know if that's correct).

Bertrand has asked if we can have a separate file for rat exclusions, so maybe we should ask about having (some) exclusions per module (which does make sense in a module world). OTOH, we could generate a complete "rat-exclusion.txt" automatically when doing the source build if needed.

@geertjanw
Copy link
Member

@jlahoda
Copy link
Contributor

jlahoda commented Jan 28, 2018

@geertjanw - yes, I know. But this patch is moving some entries from that file into the individual licenseinfo.xml files. To me, that seems fine (and represents the modular structure). But we should check for reviewers think - we could e.g. generate a complete rat-exclusion.txt for the source distributions to make them easier to review (while still keeping licenseinfo.xml per module as needed).

@matthiasblaesing
Copy link
Contributor Author

@jlahoda I see where you are going. The summarized list of the licenseinfo.xml exclusions can be found in: nbbuild/build/rat-licenseinfo.txt. The rat-exclusion.txt is not yet incorporated, but it could also be integrated, turning that file into a "this was excluded from" rat report. Should I pull it in?

I also realized, that the generated report does not cover the correct set of files. While the rat report works through all the files (minus the explicit excludes), the ReportFromLicenseinfo is limited to the currently build cluster. I'll align these.

@jlahoda
Copy link
Contributor

jlahoda commented Jan 28, 2018

@matthiasblaesing I guess I'd start with asking what Bertrand and other mentors think.

…integrate into build process

This commit adds the option to add a licenseinfo.xml file to each
netbeans module. This file holds for a set of source files the
information:

- license reference to one of the "well-known" licenses
  (not all file formats allow for license information)
- addition license information (for example the eclipse distribution
  license requires the copyright to be present and not just the
  license text)
- required addition to notice file
- comment field for explanation why the file is present

The LICENSE file get very big with this change. To make it still readable, the license
texts are not put into the same file, but into the licenses subdirectory.
At this point in time the vast majority of icons+images were donated
by oracle to the ASF. The browser icons in the css.editor and
web.browser.api modules were not part of that donation.

As the original artwork is necessary for the GUI to have the icons be
connected to the corresponding browsers, they were readded from the
netbeans repository. This files are also distributed with the binary
netbeans distribution and so are understood to be covered by the
GPLv2 + CDDL license of netbeans.

Redistribution in apache netbeans would elect the CDDL.
The licensing situation for the RSS rss-0_91.dtd is unclear and the
easiest way is replacing it by using the XHTML 1 DTDs as a replacement.

The structural component of the RSS DTD is not used, just the entity
definitions were relevant. These were copied from HTML 3.2 to the RSS
DTD and so the XHTML 1 DTDs are a logical replacement for an XML parser.

The licensing and attribution requirements of the W3C license need to
be solved separatedly (tracked by NETBEANS-314)
@matthiasblaesing
Copy link
Contributor Author

I pushed an update, that:

  • Changes resulting filename to rat-appendix.txt
  • Integrates rat-exclusions into rat-appendix.txt to have one file to offer reviewers
  • tweaks layout of rat-appendix.txt
  • adjustes license infos
  • squashes update commits into their base commits.

Copy link
Contributor

@jlahoda jlahoda left a comment

Choose a reason for hiding this comment

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

I quickly went through the patch, seems reasonable to me. The biggest thing unclear to me are the browser icons.

<file>release/modules/dict/dictionary_en_GB.description</file>
<file>release/modules/dict/dictionary_en_US.description</file>
<license ref="Apache-2.0" />
<comment type="COMMENT_UNSUPPORTED" />
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, we could add ability to have comments in these files, it probably (/hopefully) wouldn't be a big deal. Not a change for this patch, of course.

<file>src/org/netbeans/modules/css/resources/icons/safari20-disabled.png</file>
<file>src/org/netbeans/modules/css/resources/icons/safari20.png</file>
<license ref="CDDL-1.0" />
<comment>
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand what you are saying, but I don't know if that's correct (sorry).

*/
package org.netbeans.nbbuild.extlibs.licenseinfo;

public enum CommentType {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessarily in this patch, but we may need to add ability to add URLs here, pointing to either ASF, incubator or our FAQs, so that we can add more detailed explanation. Out of the 4 below, the only one that may be understandable without specific knowledge is COMMENT_UNSUPPORTED. (I think I understand what are those used for, but not sure if reviewers will without explanation.)

@matthiasblaesing
Copy link
Contributor Author

@jlahoda thank you for looking through this. I agree, that it can be improved (better explanation texts, links, format the rat-appendix file as html to make it more readable, ...).

I merge this now nonetheless, as I want to see the fallout before putting more work into this

@matthiasblaesing matthiasblaesing merged commit 944f58a into apache:master Jan 30, 2018
@jlahoda
Copy link
Contributor

jlahoda commented Jan 30, 2018

@matthiasblaesing, yes, I think merging this is fine, I just was noting what we could enhance for the future.

@matthiasblaesing matthiasblaesing deleted the file-licenses branch August 28, 2018 18:33
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.

3 participants