Add new flag in License Data Model definition

[akugarg]

Signed-off-by: akugarg akanksha.garg2k@gmail.com

Fixes #2549

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]

@akugarg btw when you link the issue using the keywords it'll be closed. This is a general big-picture issue, so feel free to create smaller issues and link to them.

Also, the commit messages are the same here and they should be more specific and different IMO. Atleast:

• Add is_unknown flag in data model
• Add is_unknown flag to data files of unknown rules

If you could rebase to change the messages that'd be great.

[akugarg]

@AyanSinhaMahapatra I have made the requested changes.

[AyanSinhaMahapatra]

1. Did you do this with the script checking if the license-expression in .yml file has unknown in it?

From discuss@gitter
Akanksha Garg:
are there instances where license can be considered unknown and may not be indicated in file name?

Ayan Sinha Mahapatra:
File name is just a naming convention. The key value pairs in the .yml files is what are used in scancode, and not the filename. And these are structured .yml files so faily easy to read and search in them specificly.

I see two issues:

• I see cases of adding is_unknown flag in false-positive rules (i.e. is_false_positive flag set to true True), which should not be the case.
• There are files which don't have unknown in their names, but have unknown in their license expressions also. These are files with is_license_intro set to True and some other cases.

2. Paste the script here for future references.

[akugarg]

import os, glob

os.chdir("git/scancode-toolkit/src/licensedcode/data/rules")
print("Changed")
to_find="unknown"
fp="is_false_positive: yes"
for file in glob.glob("*.yml"):
f=0
file1 = open(file,"r+")
for line in file1:
if fp in line:
break
else:
if to_find in line:
file1.write("is_unknown: yes")
break

Used the above script for finding the "unknown" tag in license expression and for ignoring the false-positive case.
@AyanSinhaMahapatra Hope this works.
I have not yet added for category "unknown-spdx"

[AyanSinhaMahapatra]

@akugarg btw, in addition to processing the unknown-spdx case, we also need to

1. add the is_unknown flag to the License class also, like in the Rule, and add them to the respective .yml files in the licenses folder. Like here
2. Add a test to the validate_licenses funtion, (which is run when loading the licenses/rules) as a sanity check to make sure every rule with is_unknown set true has unknown in it's license expression. This means adding the test to the Rule.validate() function here: https://github.com/nexB/scancode-toolkit/blob/develop/src/licensedcode/models.py#L866

[akugarg]

For adding to license class I have to add similar tag like ones present here https://github.com/nexB/scancode-toolkit/blob/develop/src/licensedcode/models.py#L102 set to default = false
right?

[AyanSinhaMahapatra]

Yes!

[akugarg]

When I am trying to add flag for license class and trying to run a scan for samples directory it is showing this error..

For this https://github.com/nexB/scancode-toolkit/blob/develop/samples/JGroups/LICENSE

[AyanSinhaMahapatra]

@akugarg you could push your changes for addition of the flag to License Model if you want us to have a look at it (there's no point guessing where the failure is without the code), and always run the specific tests for licensedcode first and fix failures there before actually running a whole scan.

Also, you could also try to fix your tests first for the addition of is_exception to the Rule model only, instead of trying to debug both tasks at once.

[akugarg]

OKay! I will first fix the test cases for flag addition to rule class only.

[akugarg]

@AyanSinhaMahapatra Please have a look at this once!

[AyanSinhaMahapatra]

@akugarg The test expectations need to be changed in many cases like this. Btw, note that you can set regen=True in the tests to regenerate and then check the diff carefully, to see if the changes make sense, it's faster. Be careful to not commit regen as True, and do this one function at a time.

There's also another test to be added, which fetches the corresponding license object for is_unknown rules and checks if the is_unknown falg for the license is set True (and category: Unstated License),

We're nearly there! 👍

[akugarg]

In order means that instead of having the flag added at end in YAML files as done here we should have inserted in between (may be after license expression or some flags) according to as defined in object definition right?
Also can I proceed as follows :
add this flag above other flags instead in definition itself.
then add this flag after license expressions for the required files.

[AyanSinhaMahapatra]

@akugarg I think you should instead use load_rules() and then check for unknown in license expression explicitly, set the is_unknown flag to be True for them, and then use Rule.dump() to overwrite the rules that should have is_unknown as True. This should take care of both the order and missing line returns.

There are similar functions for licenses also.

You should also merge to latest before doing this (delete the commits adding the is_unknown flag to .yml files by rebasing, merge to latest develop, and then add the changes as suggested), some more unknown rules were added.

[pombredanne]

@akugarg this is ready to merge imho but you have one failing test

[akugarg]

@pombredanne I am not able to figure out what's causing this failing check or if it is something related to what I added.

[akugarg]

@AyanSinhaMahapatra
All green! :)

[AyanSinhaMahapatra]

Looking good now.

[pombredanne]

There are a few nits for your consideration, but this is good to go otherwise! Thanks

[pombredanne]

All good!
Merging