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

Add check for SPDX license identifier #144

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions kodi_addon_checker/check_addon.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ def start(addon_path, args, all_repo_addons, config=None):

check_files.check_file_permission(addon_report, file_index)

check_files.check_license_identifier(addon_report, addon_path)

check_files.check_license_tag(addon_report, parsed_xml)

check_files.check_for_invalid_xml_files(addon_report, file_index)

check_files.check_for_invalid_json_files(addon_report, file_index)
Expand Down
34 changes: 33 additions & 1 deletion kodi_addon_checker/check_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
See LICENSES/README.md for more information.
"""

import json
import os
import re
import json
import ast
import xml.etree.ElementTree as ET
from pathlib import Path

from . import handle_files
from .common import relative_path
Expand Down Expand Up @@ -146,3 +148,33 @@ def check_file_permission(report: Report, file_index: list):
file = os.path.join(file["path"], file["name"])
if os.path.isfile(file) and os.access(str(file), os.X_OK):
report.add(Record(PROBLEM, f"{relative_path(str(file))} is marked as stand-alone executable"))


def check_license_identifier(report: Report, addon_path: str):
"""Check whether all the files present in the addon
contains the SPDX license header or not
:addon_path: Path of the addon
"""

files = Path(addon_path).glob('**/*.py')

for file in files:
with open(str(file), 'r', encoding="utf-8") as f:
tree = ast.parse(f.read())

docstring = ast.get_docstring(tree)
if docstring and "SPDX-License-Identifier" not in docstring:
report.add(Record(WARNING,
f"SPDX-License-Identifier is missing from addon file {relative_path(str(file))}"))


def check_license_tag(report: Report, parsed_xml):
"""Check license tag in addon xml file
:parsed_xml: parsed tree for xml file
"""

license_text = parsed_xml.find("extension/license").text
spdx_string = "GNU General Public License v2.0 or later"
Copy link
Member

Choose a reason for hiding this comment

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

This forces every add-on to be GPLv2+.
I don't think that was your intention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh Yeah, I think I just did what was given to me as an example. Without keeping in mind that XBMC also have MIT, LPGL, BSD etc.
I'll fix this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Rechi There's a problem with the identification of Licenses, lot's of addon have GNU GENERAL PUBLIC LICENSE Version 3, 29 June 2007 this type of text in their license tag. Now should I follow the identifiers in https://spdx.org/licenses/, doing that will generate an error on the type I mentioned above or should I just check that the license tag is non-empty, but doing this will pass all types of Licenses including any random string in the license tag.

One solution could be that we create a dictionary of all the license that we need and then use it.

Copy link
Member

Choose a reason for hiding this comment

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

As I already mentioned at another PR, first require the licence tag to be listed exactly once and have a non empty string as value.
Also there is no reason to disallow non standard licences.


if spdx_string != license_text:
report.add(Record(WARNING, "License is not of SPDX format"))