Solve license detection bugs #1963

AyanSinhaMahapatra · 2020-03-10T20:46:36Z

Fixes #1907 #1910 #1911 #1912 #1914

Tasks

Reviewed contribution guidelines
PR is descriptively titled 📑 and links the original issue above 🔗
Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
Run tests locally to check for errors.
Commits are in uniquely-named feature branch and has no merge conflicts 📁

AyanSinhaMahapatra · 2020-03-10T21:26:01Z

Also I've reproduced and checked other bugs #1933 #1932 #1931 #1930 #1929 #1928 #1927 #1926 #1925 #1924 #1923 #1922 #1921 #1920 #1919 #1918 #1917 #1915. And all these are already solved by you @pombredanne in the 12-03-license-updates branch.

I've collected all these files to reproduce these bugs, initial scan reproduces them - scan results file. Then all the rules added by @pombredanne are indexed and the scan results are here - scan results files . Comparing you can see that the bugs are solved. Some them were remaining, are solved in this PR - #1908 #1910 #1911 #1912 #1914.
Remaining - #1905
All of these were reported by @armijnhemel . Thanks!

codecov · 2020-03-10T22:28:56Z

Codecov Report

Merging #1963 into 12-03-license-updates will decrease coverage by 0.66%.
The diff coverage is n/a.

@@                    Coverage Diff                    @@
##           12-03-license-updates    #1963      +/-   ##
=========================================================
- Coverage                  79.61%   78.94%   -0.67%     
=========================================================
  Files                        131      131              
  Lines                      17642    16946     -696     
=========================================================
- Hits                       14045    13378     -667     
+ Misses                      3597     3568      -29

Impacted Files	Coverage Δ
src/packagedcode/build.py	`74.44% <0.00%> (-5.56%)`	⬇️
src/packagedcode/rubygems.py	`70.89% <0.00%> (-5.38%)`	⬇️
src/scancode/resource.py	`86.98% <0.00%> (-3.62%)`	⬇️
src/summarycode/plugin_consolidate.py	`94.21% <0.00%> (-2.36%)`	⬇️
src/scancode/cli.py	`76.96% <0.00%> (-2.23%)`	⬇️
src/scancode/api.py	`95.54% <0.00%> (-0.64%)`	⬇️
src/licensedcode/match.py	`77.69% <0.00%> (-0.37%)`	⬇️
src/scancode/outdated.py	`81.25% <0.00%> (+0.19%)`	⬆️

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 2a90d9f...7984a2e. Read the comment docs.

pombredanne

@AyanSinhaMahapatra This is looking great! You really grok license detection now!
See my comments in line.

pombredanne · 2020-03-10T23:22:19Z

src/licensedcode/data/rules/gpl-2.0-plus_with_bison-exception-2.2_3.RULE

+  implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+  See the License for more information.
+
+   Copyright (C) 1984, 1989, 1990, 2000, 2001, 2002, 2003, 2004, 2005, 2006


In most cases , we are trying to avoid keeping copyright statements in license detection rules. So IMHO you could remove these two lines

This works perfectly. Pushing changes with removed lines.

pombredanne · 2020-03-10T23:26:01Z

src/licensedcode/data/rules/lgpl-2.0-plus_with_libtool-exception-2.0_1.RULE

@@ -0,0 +1,20 @@
+GNU Libltdl is free software; you can redistribute it and/or


It would be interesting to also have another rule (and may be have only that rule) WITHOUT the libltdl reference?
For instance rather than start with:
GNU Libltdl is free software; you can redistribute it and/or
use this:
GNU is free software; you can redistribute it and/or

and remove the libtdl word elsewhere?
Because of the way the license detection ignores words it doe not know about (such as libtdl) this will not have any impact and will be detected with the automaton... And it can also detected exactly several other variants.

This works perfectly for this case, i.e. (having only that rule) it would also detect if the text's exactly same apart from "Libltdl". I was wondering if I can keep the original rule with a higher relevance, and the non-liblltdl one with a lower relevance score, so it detects other similar stuff but in case of multiple matches would prioritize other more relevant matches. I'm pushing that.

IMHO I think there's some work to be done in standardizing relevance scores and minimum coverages across all the rules, after performing statistics on false positive/unknown matches, as these are kind of happening regularly in bugs. This would be one of the smaller important tasks of the GSoC project, what do you think?

Is there some doc on how relevance scores are given out to rules? I see the general trend in rule names/texts vs their relevance/relevance scores, and how they affect license scores, but are there any strict guidelines I mean?

liblltdl

IMHO only the one without liblltdl would be enough

IMHO I think there's some work to be done in standardizing relevance scores and minimum coverages across all the rules, after performing statistics on false positive/unknown matches, as these are kind of happening regularly in bugs. This would be one of the smaller important tasks of the GSoC project, what do you think?

excellent idea 👍

Is there some doc on how relevance scores are given out to rules? I see the general trend in rule names/texts vs their relevance/relevance scores, and how they affect license scores, but are there any strict guidelines I mean?

The relevance is computed dynamically based on the size of a rule OR stored and assigned manually as a curated value based on judgment.
See

https://github.com/nexB/scancode-toolkit/blob/d18fd2db7ba2dd893a4c46bdbbc9d7db8fb710c1/src/licensedcode/models.py#L729

https://github.com/nexB/scancode-toolkit/blob/d18fd2db7ba2dd893a4c46bdbbc9d7db8fb710c1/src/licensedcode/models.py#L1089

IMHO only the one without liblltdl would be enough

Done.

AyanSinhaMahapatra · 2020-03-11T17:05:32Z

bison 2.4.3: GPL-3.0-or-later file also recognized as "Free unknown" #1907 Even after having a 100% match coverage with the correct gpl-3.0-plus LICENSE(not rule), matches with another free-unknown Rule. File

    {
      "path": "Issues/1907-bison-2.4.3-getargs.c",
      "licenses": [
        {
          "key": "gpl-3.0-plus",
          "score": 100.0,
          "matched_rule": {
            "identifier": "gpl-3.0-plus.LICENSE",
            "license_expression": "gpl-3.0-plus",
            "licenses": [
              "gpl-3.0-plus"
            ],,
            "matcher": "2-aho",
            "rule_length": 102,
            "matched_length": 102,
            "match_coverage": 100.0,
            "rule_relevance": 100
          },
          "matched_text": "   This program is free software: you can redistribute it and/or modify\n   it under the terms of the GNU General Public License as published by\n   the Free Software Foundation, either version 3 of the License, or\n   (at your option) any later version.\n\n   This program is distributed in the hope that it will be useful,\n   but WITHOUT ANY WARRANTY; without even the implied warranty of\n   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\n   GNU General Public License for more details.\n\n   You should have received a copy of the GNU General Public License\n   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */"
        },
        {
          "key": "free-unknown",
          "score": 100.0,
          "matched_rule": {
            "identifier": "free-unknown_1.RULE",
            "license_expression": "free-unknown",
            "licenses": [
              "free-unknown"
            ],
            "matcher": "2-aho",
            "rule_length": 23,
            "matched_length": 23,
            "match_coverage": 100.0,
            "rule_relevance": 100
          },
          "matched_text": "This is free software; see the source for copying conditions.  There is NO \\\nwarranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. \\"
        }
      ],
      "license_expressions": [
        "gpl-3.0-plus",
        "free-unknown"
      ],

Even if the free-unknown rule's relevance is decreased to 50, nothing changes.

pombredanne · 2020-03-11T17:32:05Z

@AyanSinhaMahapatra in order to have an efficient discussion on each issue it may be simpler to have the comments you pasted above each in their own respective ticket, otherwise this is going to be a tad hairy to track comments and replies.
Some other points:

it would be best to avoid link to Google docs for external files as these may go away: either paste the file in a comment or attach it as a .txt.
when you paste snippets of scans do not truncate them in the middle (e.g. do not remove the line numbers for instance) as it makes things difficult to understand. You can always attach a whole scan as a .txt too.

AyanSinhaMahapatra · 2020-03-11T19:55:18Z

@pombredanne Added commits solving #1908 .

pombredanne

@AyanSinhaMahapatra may be you could find a slightly more descriptive commit message?

Add requested changes is not super useful: think about it this way: 6 months from now, you are looking at this same message. What information does this message tells you? IMHO not much.
Try to find something more explicit... tell a (mini) story or enough for this to be useful now for others and for you and others in the future.

pombredanne · 2020-03-11T21:53:33Z

src/licensedcode/data/rules/lgpl-2.0-plus_with_libtool-exception-2.0_1.yml

@@ -1,7 +1,7 @@
 license_expression: lgpl-2.0-plus WITH libtool-exception-2.0
 is_license_notice: yes
 minimum_coverage: 90
-relevance: 100
+relevance: 90


Why is this not 100% relevant? is there any ambiguity that is a lgpl-2.0-plus WITH libtool-exception-2.0? I do not think so, furthermore, this is a large enough rule, so you could/should eschew storing a relevance there IMHO

I forgot to remove this change, i.e. after doing this. Fixing this and the commit message too. Thanks for pointing out.

pombredanne

Thank you ++
We are almost there!

pombredanne · 2020-03-12T21:56:03Z

src/licensedcode/data/rules/gpl-2.0-plus_with_bison-exception-2.2_3.yml

@@ -0,0 +1,4 @@
+license_expression: gpl-2.0-plus WITH bison-exception-2.2


This should instead:

license_expression: bsd-new AND gpl-2.0-plus WITH bison-exception-2.2 is_license_notice: yes minimum_coverage: 99 referenced_filenames: - Copyright.txt

You may also want to create a second rule with only the BSD parts:

Distributed under the OSI-approved BSD License (the "License"); see accompanying file Copyright.txt for details.

Right! Updating.

So I've created a second rule with the BSD parts beforehand only, in this commit but didn't add bsd-new AND to the license_expression of gpl-2.0-plus WITH bison-exception-2.2, which I've pushed now. Does this work?

pombredanne · 2020-03-12T21:57:59Z

src/licensedcode/data/rules/not-a-license_429.RULE

@@ -0,0 +1 @@
+label	= "GPL2"


Since this is very close to a possible license tag of sorts, could there be a word before or after that could be included to make this more specific?

Yes! Adding a line before.

pombredanne · 2020-03-12T21:58:28Z

src/licensedcode/data/rules/not-a-license_431.RULE

@@ -0,0 +1 @@
+label	= "GPL3"


Same comment as for the label = gpl2 rule

Yes, will add a line before.

pombredanne · 2020-03-12T21:59:00Z

src/licensedcode/data/rules/not-a-license_432.RULE

@@ -0,0 +1 @@
+S5PC100_GPL4(0)


Was this really detected?

No, only GPL1, GPL2 and GPL3 were detected. On second thought this is really unnecessary, removing and renaming these.

pombredanne · 2020-03-12T21:59:12Z

src/licensedcode/data/rules/not-a-license_434.RULE

@@ -0,0 +1 @@
+S5PC100_GPL0(0)


Was this really detected?

No. On second thought this is really unnecessary, removing these.

pombredanne · 2020-03-12T21:59:22Z

src/licensedcode/data/rules/not-a-license_433.RULE

@@ -0,0 +1 @@
+label	= "GPL4"


Was this really detected?

No. On second thought this is really unnecessary, removing these.

pombredanne · 2020-03-12T21:59:33Z

src/licensedcode/data/rules/not-a-license_435.RULE

@@ -0,0 +1 @@
+label	= "GPL0"


Was this really detected?

No. On second thought this is really unnecessary, removing these.

pombredanne

LGTM! Thanks... Almost there: there is still a minor nitpicking/rename needed.

pombredanne · 2020-03-28T15:20:33Z

src/licensedcode/data/rules/gpl-2.0-plus_with_bison-exception-2.2_3.RULE

@@ -0,0 +1,34 @@
+  Distributed under the OSI-approved BSD License (the "License");


As a convention we name the rule after the license expression...
So may be use something like bsd-new_and_gpl-2.0-plus_with_bison-exception_....
(within reason, unless there many many licenses)

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Solves aboutcode-org#1912 Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>

Solves aboutcode-org#1914 aboutcode-org#1911 Add negative and gpl with exceptions rules Add rules and minimum coverage for false positive case. Add new GPL rule Add new bsd license rules Add bzip2 Rule Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>

Add bsd-new to gpl with bison exception yml file. Also add referenced copyright file. Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne · 2020-04-02T21:42:33Z

This looks ready to merge to me. The tests failing are not related to this.
Thanks!

Adds functions that takes input data from Scancode Scan Results, in JSON, and structures them similarly as the ClearlyDefined DataBase Data, so the same fuctions can be used on them. Adds Jupyter Notebook to explain the Fuction Calls, and Data, and JSON files as sample, from the issues in aboutcode-org/scancode-toolkit#1963. Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>

AyanSinhaMahapatra requested a review from pombredanne March 10, 2020 22:46

pombredanne requested changes Mar 10, 2020

View reviewed changes

pombredanne mentioned this pull request Mar 11, 2020

bison 2.4.3: GPL-3.0-or-later file also recognized as "Free unknown" #1907

Closed

AyanSinhaMahapatra requested a review from pombredanne March 11, 2020 19:55

pombredanne requested changes Mar 11, 2020

View reviewed changes

AyanSinhaMahapatra force-pushed the license-updates-bugs branch from 16cc484 to 3cfea5e Compare March 12, 2020 11:53

pombredanne requested changes Mar 12, 2020

View reviewed changes

AyanSinhaMahapatra requested a review from pombredanne March 16, 2020 11:38

pombredanne requested changes Mar 28, 2020

View reviewed changes

AyanSinhaMahapatra force-pushed the license-updates-bugs branch from ea3be0c to 3e09782 Compare March 29, 2020 19:28

pombredanne and others added 5 commits April 2, 2020 15:52

Rename license rules to avoid merge conflicts

abf5f5d

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Add new lgpl rules with libtool exception

47140b3

Solves aboutcode-org#1912 Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>

Modify gpl2 plus with bison exception rule

03ea190

Add bsd-new to gpl with bison exception yml file. Also add referenced copyright file. Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>

Add ignorables to license rule

7984a2e

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

pombredanne force-pushed the license-updates-bugs branch from 3e09782 to 7984a2e Compare April 2, 2020 14:54

pombredanne merged commit a4b133f into aboutcode-org:12-03-license-updates Apr 2, 2020

AyanSinhaMahapatra mentioned this pull request Oct 22, 2020

Extensive Tests for scancode-results-analyzer aboutcode-org/scancode-analyzer#14

Closed

		@@ -0,0 +1,20 @@
		GNU Libltdl is free software; you can redistribute it and/or

		@@ -0,0 +1,4 @@
		license_expression: gpl-2.0-plus WITH bison-exception-2.2

		@@ -0,0 +1,34 @@
		Distributed under the OSI-approved BSD License (the "License");

		@@ -0,0 +1 @@
		S5PC100_GPL4(0)

		@@ -0,0 +1 @@
		S5PC100_GPL0(0)

Solve license detection bugs #1963

Solve license detection bugs #1963

Conversation

AyanSinhaMahapatra commented Mar 10, 2020 • edited Loading

Tasks

AyanSinhaMahapatra commented Mar 10, 2020 • edited Loading

codecov bot commented Mar 10, 2020 • edited Loading

Codecov Report

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AyanSinhaMahapatra commented Mar 11, 2020 • edited Loading

pombredanne commented Mar 11, 2020

AyanSinhaMahapatra commented Mar 11, 2020

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AyanSinhaMahapatra Mar 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AyanSinhaMahapatra Mar 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AyanSinhaMahapatra Mar 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne commented Apr 2, 2020

AyanSinhaMahapatra commented Mar 10, 2020 •

edited

Loading

AyanSinhaMahapatra commented Mar 10, 2020 •

edited

Loading

codecov bot commented Mar 10, 2020 •

edited

Loading

AyanSinhaMahapatra commented Mar 11, 2020 •

edited

Loading

AyanSinhaMahapatra Mar 16, 2020 •

edited

Loading

AyanSinhaMahapatra Mar 16, 2020 •

edited

Loading

AyanSinhaMahapatra Mar 16, 2020 •

edited

Loading