RAT-469: Fixes to correctly detect additional valid licenses #413

Claudenw · 2024-12-10T17:29:51Z

A regression was spotted when another valid license was configured ..... but not properly reported.

If you configure GPL to be a valid license the recognition failed to pass.

ottlinger · 2024-12-11T13:16:49Z

Thanks for raising this issue!

Should we replace all comments in the AbstractRatMojo as
"@deprecated Use <config>." seems to be wrong as the configuration options appear directly under configuration, which is the standard element in Maven plugin configuration.

WDYT?

ottlinger · 2024-12-11T15:11:37Z

@Claudenw apart from the fact the IT needs to be adapted once the patch is applied I wonder why GPL is not yet recognized and logged explicitly.

Claudenw · 2024-12-11T15:17:01Z

Gpl is not allowed at apache. So you have to add to the defaults

…

On Wed 11 Dec 2024, 15:12 P. Ottlinger, ***@***.***> wrote: @Claudenw <https://github.com/Claudenw> apart from the fact the IT needs to be adapted once the patch is applied I wonder why GPL is not yet recognized and logged explicitly. — Reply to this email directly, view it on GitHub <#413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASTVHTSDPYQ6L3UFPJJEC32FBI4BAVCNFSM6AAAAABTLVX2FKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZWGI3TCMJQGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

ottlinger · 2024-12-11T15:51:29Z

@Claudenw I added GPL as a valid license - I always thought this could be configured and I do not need to add a new defaults.xml file in my project. I used the proposed changes in https://github.com/ottlinger/mailclena/pull/573/files and applied it to an integration test within RAT.

Claudenw · 2024-12-11T16:30:11Z

I think I mistyped. I said "So you have to add to the defaults" when what I meant was you need to add GPL-1, GPL-2, or GPL-3 as an approved license ID or add GPL as an approved license family. The licenses are defined but they are not listed approved.

Claudenw · 2024-12-11T16:32:12Z

Should we replace all comments in the AbstractRatMojo as
"@deprecated Use ." seems to be wrong as the configuration options appear directly under configuration, which is the standard element in Maven plugin configuration.

I think we need to step through each of them and make a decision. "" is not always wrong but there are better solutions for most entries.

I think this falls under the documentation verification step that we should get to in mid Jan.

…ror' into Changes_for_invalid_license_error

…id_license_error' into Changes_for_invalid_license_error

ottlinger · 2024-12-12T09:06:49Z

If I have a look at the log of the integration test I see:

[WARNING] Duplicate LicenseFamily category: AL    (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: BSD-3 (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: CDDL1 (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: GEN   (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: GPL   (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: MIT   (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: OASIS (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: W3C   (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: W3CD  (action: IGNORE)

could this be the cause for not recognizing GPL but labeling it UNKNOWN?
(apache-rat-plugin/target/invoker/RAT-469/build.log)

…_license_error

Claudenw · 2024-12-12T14:23:02Z

First GPL should be recognized as a license FAMILY it is in the default.xml
GPL1, GPL2, and GPL3 should be recognised as license IDs also in the default.xml

[WARNING] Duplicate LicenseFamily category: W3C   (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: W3CD  (action: IGNORE)

This indicates that at least 2 configurations were read and that the LicenseFamilies were defined in both. I don't see how this happens but the warning is there so that if you accidentally overwrite an existing license you get warned.

ottlinger · 2024-12-12T14:30:30Z

First GPL should be recognized as a license FAMILY it is in the default.xml GPL1, GPL2, and GPL3 should be recognised as license IDs also in the default.xml
[WARNING] Duplicate LicenseFamily category: W3C   (action: IGNORE)
[WARNING] Duplicate LicenseFamily category: W3CD  (action: IGNORE)
This indicates that at least 2 configurations were read and that the LicenseFamilies were defined in both. I don't see how this happens but the warning is there so that if you accidentally overwrite an existing license you get warned.

Just wanted to make sure that the IGNORE does not mean that any such license will be ignored.

ottlinger · 2024-12-12T14:43:37Z

I merged in master and HeaderCheckWorkerTest is broken.

@Claudenw Is it intended that the license family "unknown" is approved by default?

+++ b/apache-rat-core/src/test/java/org/apache/rat/analysis/HeaderCheckWorkerTest.java
@@ -43,8 +43,8 @@ public class HeaderCheckWorkerTest {
         ILicense matcher = new TestingLicense("test", "test");
         HeaderCheckWorker worker = new HeaderCheckWorker(new TestingMatcher(), new StringReader(""), Lists.list(matcher), subject);
         worker.read();
-        assertThat(subject.getMetaData().unapprovedLicenses().count()).isEqualTo(1);
-        assertThat(subject.getMetaData().unapprovedLicenses().collect(Collectors.toList()).get(0).getLicenseFamily()).isEqualTo(ILicenseFamily.UNKNOWN);
+        assertThat(subject.getMetaData().approvedLicenses().count()).isEqualTo(1);
+        assertThat(subject.getMetaData().approvedLicenses().collect(Collectors.toList()).get(0).getLicenseFamily()).isEqualTo(ILicenseFamily.UNKNOWN);
     }

Claudenw · 2024-12-13T11:17:14Z

In this case "IGNORE" is the action taken. It is configurable in software but not config.

…

On Thu 12 Dec 2024, 14:30 P. Ottlinger, ***@***.***> wrote: First GPL should be recognized as a license FAMILY it is in the default.xml GPL1, GPL2, and GPL3 should be recognised as license IDs also in the default.xml [WARNING] Duplicate LicenseFamily category: W3C (action: IGNORE) [WARNING] Duplicate LicenseFamily category: W3CD (action: IGNORE) This indicates that at least 2 configurations were read and that the LicenseFamilies were defined in both. I don't see how this happens but the warning is there so that if you accidentally overwrite an existing license you get warned. Just wanted to make sure that the IGNORE does not mean that any such license will be ignored. — Reply to this email directly, view it on GitHub <#413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASTVHRK6X52X3H7F6TT5YD2FGMZ3AVCNFSM6AAAAABTLVX2FKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZZGEYTKOJQHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Claudenw · 2024-12-13T11:20:19Z

Nope. Unknown lice se can not be approved unless specifically added by the user. I will check on this this weekend

…

On Thu 12 Dec 2024, 14:44 P. Ottlinger, ***@***.***> wrote: I merged in master and HeaderCheckWorkerTest is broken. @Claudenw <https://github.com/Claudenw> Is it intended that the license family "unknown" is approved by default? +++ b/apache-rat-core/src/test/java/org/apache/rat/analysis/HeaderCheckWorkerTest.java @@ -43,8 +43,8 @@ public class HeaderCheckWorkerTest { ILicense matcher = new TestingLicense("test", "test"); HeaderCheckWorker worker = new HeaderCheckWorker(new TestingMatcher(), new StringReader(""), Lists.list(matcher), subject); worker.read(); - assertThat(subject.getMetaData().unapprovedLicenses().count()).isEqualTo(1); - assertThat(subject.getMetaData().unapprovedLicenses().collect(Collectors.toList()).get(0).getLicenseFamily()).isEqualTo(ILicenseFamily.UNKNOWN); + assertThat(subject.getMetaData().approvedLicenses().count()).isEqualTo(1); + assertThat(subject.getMetaData().approvedLicenses().collect(Collectors.toList()).get(0).getLicenseFamily()).isEqualTo(ILicenseFamily.UNKNOWN); } — Reply to this email directly, view it on GitHub <#413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASTVHQVV46VIDZC2AXX3Y32FGOK7AVCNFSM6AAAAABTLVX2FKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZZGE2DSNRRGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

…s need to be put under the existing configuration instead a newly/non-existing config

ottlinger · 2024-12-13T13:26:08Z

Should we replace all comments in the AbstractRatMojo as
"@deprecated Use ." seems to be wrong as the configuration options appear directly under configuration, which is the standard element in Maven plugin configuration.

I think we need to step through each of them and make a decision. "" is not always wrong but there are better solutions for most entries.

I think this falls under the documentation verification step that we should get to in mid Jan.

I changed the default text in ef5c93b

apache-rat-core/src/test/java/org/apache/rat/analysis/PolicyTest.java

apache-rat-core/src/test/java/org/apache/rat/license/LicenseSetFactoryTest.java

apache-rat-core/src/main/java/org/apache/rat/license/LicenseSetFactory.java

ottlinger

Thanks for having another deeper look into the matter and for fixing the tests.

ottlinger · 2025-01-12T21:59:15Z

GPL content is still recognised as UNKNOWN even if configured as an approvedLicense.

Fixes to correctly detect additional valid licenses

a4cc18b

Claudenw requested a review from ottlinger December 10, 2024 17:29

Claudenw added 3 commits December 10, 2024 23:16

reverted ubuntu to 22.04

9735c09

reverted change to ubuntu version

8987032

fixed tools spotbugs issue under java 21

2713a15

ottlinger changed the title ~~Fixes to correctly detect additional valid licenses~~ RAT-469: Fixes to correctly detect additional valid licenses Dec 11, 2024

ottlinger added 3 commits December 11, 2024 14:17

RAT-469: Add Itest which should work after the patch is applied

d8b4ce7

RAT-469: Add changelog entry

0b95cb6

LHF: fix typo

c45e75d

Claudenw and others added 4 commits December 11, 2024 16:35

removed RELEASE_NOTES from tools build

8f0ad6c

Merge remote-tracking branch 'upstream/Changes_for_invalid_license_er…

53772d9

…ror' into Changes_for_invalid_license_error

RAT-469: Ensure inclusion/exclusion rules are logged properly

8645b12

Merge remote-tracking branch 'refs/remotes/upstream/Changes_for_inval…

3e21c56

…id_license_error' into Changes_for_invalid_license_error

Merge remote-tracking branch 'origin/master' into Changes_for_invalid…

63fa15a

…_license_error

RAT-469: Replace misleading default text as new configuration element…

ef5c93b

…s need to be put under the existing configuration instead a newly/non-existing config

Claudenw and others added 2 commits December 14, 2024 13:46

Cleaned up analysis and verified correctness of LicenseSetFactory

a4d720e

RAT-469: Minor fixes

6a5b17c

RAT-469: Remove hungarian notation

b35df25

ottlinger reviewed Dec 15, 2024

View reviewed changes

apache-rat-core/src/test/java/org/apache/rat/analysis/PolicyTest.java Outdated Show resolved Hide resolved

ottlinger reviewed Dec 17, 2024

View reviewed changes

apache-rat-core/src/test/java/org/apache/rat/license/LicenseSetFactoryTest.java Outdated Show resolved Hide resolved

ottlinger and others added 3 commits December 17, 2024 17:01

RAT-469: Minor fixes

83ac502

fixed tests

aee4042

removed dead code

1e4c7b1

Claudenw requested a review from ottlinger January 12, 2025 14:27

ottlinger reviewed Jan 12, 2025

View reviewed changes

apache-rat-core/src/main/java/org/apache/rat/license/LicenseSetFactory.java Outdated Show resolved Hide resolved

ottlinger added 2 commits January 12, 2025 16:48

Minor cleanups

fdac590

Minor cleanups

a886a07

ottlinger approved these changes Jan 12, 2025

View reviewed changes

ottlinger added 4 commits January 12, 2025 17:47

RAT-469: Minor cleanup

16a2319

RAT-469: Build failure on GHA-windows

7cba21a

RAT-469: Fix spotbugs issue/encoding

e7883c0

RAT-469: Fix spotbugs issue/encoding

cf9c2f5

Claudenw merged commit 9ddc32a into apache:master Jan 12, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RAT-469: Fixes to correctly detect additional valid licenses #413

RAT-469: Fixes to correctly detect additional valid licenses #413

Claudenw commented Dec 10, 2024 •

edited by ottlinger

Loading

ottlinger commented Dec 11, 2024 •

edited

Loading

ottlinger commented Dec 11, 2024

Claudenw commented Dec 11, 2024 via email

ottlinger commented Dec 11, 2024 •

edited

Loading

Claudenw commented Dec 11, 2024

Claudenw commented Dec 11, 2024 •

edited

Loading

ottlinger commented Dec 12, 2024

Claudenw commented Dec 12, 2024 •

edited

Loading

ottlinger commented Dec 12, 2024

ottlinger commented Dec 12, 2024

Claudenw commented Dec 13, 2024 via email

Claudenw commented Dec 13, 2024 via email

ottlinger commented Dec 13, 2024

ottlinger left a comment

ottlinger commented Jan 12, 2025

RAT-469: Fixes to correctly detect additional valid licenses #413

RAT-469: Fixes to correctly detect additional valid licenses #413

Conversation

Claudenw commented Dec 10, 2024 • edited by ottlinger Loading

ottlinger commented Dec 11, 2024 • edited Loading

ottlinger commented Dec 11, 2024

Claudenw commented Dec 11, 2024 via email

ottlinger commented Dec 11, 2024 • edited Loading

Claudenw commented Dec 11, 2024

Claudenw commented Dec 11, 2024 • edited Loading

ottlinger commented Dec 12, 2024

Claudenw commented Dec 12, 2024 • edited Loading

ottlinger commented Dec 12, 2024

ottlinger commented Dec 12, 2024

Claudenw commented Dec 13, 2024 via email

Claudenw commented Dec 13, 2024 via email

ottlinger commented Dec 13, 2024

ottlinger left a comment

Choose a reason for hiding this comment

ottlinger commented Jan 12, 2025

Claudenw commented Dec 10, 2024 •

edited by ottlinger

Loading

ottlinger commented Dec 11, 2024 •

edited

Loading

ottlinger commented Dec 11, 2024 •

edited

Loading

Claudenw commented Dec 11, 2024 •

edited

Loading

Claudenw commented Dec 12, 2024 •

edited

Loading