From a3cac85b8c127e8178f3e6f221037ad5d11a2a5a Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Tue, 2 Apr 2024 10:56:44 +0200 Subject: [PATCH 01/21] changed each test to also check for warnings and specified which warning must be displayed Signed-off-by: Anton Utz --- test/systemtest/test_separate_pkgs.py | 185 ++++++++++++++++++-------- 1 file changed, 128 insertions(+), 57 deletions(-) diff --git a/test/systemtest/test_separate_pkgs.py b/test/systemtest/test_separate_pkgs.py index 94382ee..0fa3b86 100644 --- a/test/systemtest/test_separate_pkgs.py +++ b/test/systemtest/test_separate_pkgs.py @@ -20,8 +20,13 @@ import subprocess import unittest from test.systemtest._test_helpers import make_repo, remove_repo +from typing import Optional -from ros_license_toolkit.main import main +from ros_license_toolkit.checks import Status + +SUCCESS = Status.SUCCESS +WARNING = Status.WARNING +FAILURE = Status.FAILURE class TestPkgs(unittest.TestCase): @@ -54,16 +59,19 @@ def test_pkg_both_tags_not_spdx(self): """ Test on a package that has two different Licenses that both have a not SPDX conform Tag. License files and source files are SPDX conform""" - self.assertEqual(os.EX_OK, main([ - "test/_test_data/test_pkg_both_tags_not_spdx" - ])) + process, stdout = open_subprocess("test_pkg_both_tags_not_spdx") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status( + stdout, SUCCESS, WARNING, WARNING, WARNING, WARNING)) def test_pkg_both_tags_not_spdx_one_file_own(self): """Test on a package that has two licenses. One is self-defined, other one with not SPDX tag but therefore code and license file in SPDX""" - self.assertEqual(os.EX_DATAERR, main([ - "test/_test_data/test_pkg_both_tags_not_spdx_one_file_own" - ])) + process, stdout = open_subprocess( + "test_pkg_both_tags_not_spdx_one_file_own") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, SUCCESS, WARNING, FAILURE, WARNING, WARNING)) def test_pkg_code_has_no_license(self): """Test on a package that has a correct package.xml with a license @@ -76,78 +84,96 @@ def test_pkg_code_has_no_license(self): def test_pkg_has_code_disjoint(self): """Test on a package with two disjoint sets of source files under a license different from the package main license.""" - self.assertEqual(os.EX_OK, main( - ["test/_test_data/test_pkg_has_code_disjoint"])) + process, stdout = open_subprocess("test_pkg_has_code_disjoint") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) def test_pkg_has_code_of_different_license(self): """Test on a package with source files under a license different from the package main license (here LGPL). It should fail, because the additional license is not declared in the package.xml.""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/test_pkg_has_code_of_different_license"])) + process, stdout = open_subprocess( + "test_pkg_has_code_of_different_license") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status(stdout, exp_lic_in_code=FAILURE)) def test_pkg_has_code_of_different_license_and_tag(self): """Test on a package with source files under a license different from the package main license, but the additional license is declared in the package.xml.""" - self.assertEqual(os.EX_OK, main( - ["test/_test_data/" - "test_pkg_has_code_of_different_license_and_tag"])) + process, stdout = open_subprocess( + "test_pkg_has_code_of_different_license_and_tag") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) def test_pkg_has_code_of_different_license_and_wrong_tag(self): """Test on a package with source files under a license different from the package main license, but the additional license is declared in the package.xml, but with the wrong name.""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/" - "test_pkg_has_code_of_different_license_and_wrong_tag"])) + process, stdout = open_subprocess( + "test_pkg_has_code_of_different_license_and_wrong_tag") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, exp_lic_text_exits=FAILURE, exp_lic_in_code=FAILURE)) def test_pkg_ignore_readme_contents(self): """Test on a package with readme files. READMEs mention licenses that are not in package and shall therefore be ignored.""" - test_result = main(["test/_test_data/test_pkg_ignore_readme_contents"]) - self.assertEqual(os.EX_OK, test_result) + process, stdout = open_subprocess("test_pkg_ignore_readme_contents") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) def test_pkg_name_not_in_spdx(self): """Test on a package that has valid License file with BSD-3-Clause but its license tag BSD is not in SPDX format""" process, stdout = open_subprocess("test_pkg_name_not_in_spdx") self.assertEqual(os.EX_OK, process.returncode) - self.assertIn(b"WARNING", stdout) + self.assertTrue(check_output_status( + stdout, SUCCESS, WARNING, WARNING, SUCCESS, WARNING)) def test_pkg_no_file_attribute(self): """Test on a package with License file that is not referenced in package.xml""" - self.assertEqual(os.EX_OK, main( - ["test/_test_data/test_pkg_no_file_attribute"])) + process, stdout = open_subprocess("test_pkg_no_file_attribute") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) def test_pkg_no_license(self): """Test on a package with no license declared in the package.xml.""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/test_pkg_no_license"])) + process, stdout = open_subprocess("test_pkg_no_license") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, FAILURE, SUCCESS, FAILURE, FAILURE, SUCCESS)) def test_pkg_no_license_file(self): """Test on a package with no license text file.""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/test_pkg_no_license_file"])) + process, stdout = open_subprocess("test_pkg_no_license_file") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, exp_lic_text_exits=FAILURE)) def test_pkg_one_correct_one_license_file_missing(self): """Test on a package that has one correct license with file and code, but also one not known license tag without file""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/test_pkg_one_correct_one_license_file_missing"])) + process, stdout = open_subprocess( + "test_pkg_one_correct_one_license_file_missing") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, SUCCESS, WARNING, FAILURE, FAILURE, SUCCESS)) def test_pkg_spdx_name(self): """Test on a package with a license declared in the package.xml with the SPDX name.""" - self.assertEqual(os.EX_OK, main( - ["test/_test_data/test_pkg_spdx_name"])) + process, stdout = open_subprocess("test_pkg_spdx_name") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) def test_pkg_spdx_tag(self): """Test on a package with a license declared in the package.xml with the SPDX tag.""" - self.assertEqual(os.EX_OK, main( - ["test/_test_data/test_pkg_spdx_tag"])) + process, stdout = open_subprocess("test_pkg_spdx_tag") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) def test_pkg_too_many_license_files(self): """"Test on a package with multiple License files that are not @@ -157,64 +183,69 @@ def test_pkg_too_many_license_files(self): self.assertIn(b"bsd.LICENSE", stdout) self.assertIn(b"apl.LICENSE", stdout) self.assertNotIn(b"../../../LICENSE", stdout) + self.assertTrue(check_output_status( + stdout, exp_lic_files_referenced=FAILURE)) def test_pkg_tag_not_spdx(self): """Test on a package that has one linked declaration, one code file but not in SPDX tag. Tag must be gotten from declaration.""" process, stdout = open_subprocess("test_pkg_tag_not_spdx") self.assertEqual(os.EX_OK, process.returncode) - self.assertIn(b"WARNING", stdout) - self.assertIn(b"'code_with_afl.py' is of AFL-2.0 but its Tag is AFL.", - stdout) + self.assertTrue(check_output_status( + stdout, SUCCESS, WARNING, WARNING, WARNING, WARNING)) def test_pkg_unknown_license(self): """Test on a package with an unknown license declared in the package.xml.""" - # using subprocess.Popen instead of main() to capture stdout - with subprocess.Popen( - ["ros_license_toolkit", - "test/_test_data/test_pkg_unknown_license"], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE - ) as process: - stdout, _ = process.communicate() - self.assertNotEqual(os.EX_OK, process.returncode) - self.assertIn(b'not in SPDX list of licenses', stdout) - self.assertIn(b'my own fancy license 1.0', stdout) + process, stdout = open_subprocess("test_pkg_unknown_license") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, exp_lic_tag_spdx=WARNING, exp_lic_text_exits=FAILURE)) def test_pkg_unknown_license_missing_file(self): """Test on a package that has an unknown license without a license file""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/test_pkg_unknown_license_missing_file"])) + process, stdout = open_subprocess( + "test_pkg_unknown_license_missing_file") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, exp_lic_tag_spdx=WARNING, exp_lic_text_exits=FAILURE)) def test_pkg_with_license_and_file(self): """Test on a package with a license declared in the package.xml and a matching license text file.""" - self.assertEqual(os.EX_OK, main( - ["test/_test_data/test_pkg_with_license_and_file"])) + process, stdout = open_subprocess("test_pkg_with_license_and_file") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) def test_pkg_with_multiple_licenses_no_source_files_tag(self): """Test on a package with multiple licenses declared in the package.xml, none of which have source file tags.""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/" - "test_pkg_with_multiple_licenses_no_source_files_tag"])) + process, stdout = open_subprocess( + "test_pkg_with_multiple_licenses_no_source_files_tag") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, exp_lic_tag_exists=FAILURE)) def test_pkg_with_multiple_licenses_one_referenced_incorrect(self): """Test on a package with multiple licenses declared in the - package.xml. First has tag not in SPDX list with correct source file, - second is in SPDX.""" + package.xml. First has tag not in SPDX list with correct + source file, second is in SPDX.""" process, stdout = open_subprocess( "test_pkg_with_multiple_licenses_one_referenced_incorrect") self.assertEqual(os.EX_OK, process.returncode) self.assertIn(b"WARNING Licenses ['BSD'] are not in SPDX list", stdout) + self.assertTrue(check_output_status( + stdout, SUCCESS, WARNING, WARNING, WARNING, WARNING)) def test_pkg_wrong_license_file(self): """Test on a package with a license text file that does not match the license declared in the package.xml, both tag and file in spdx""" - self.assertEqual(os.EX_DATAERR, main( - ["test/_test_data/test_pkg_wrong_license_file"])) + process, stdout = open_subprocess("test_pkg_wrong_license_file") + self.assertEqual(os.EX_DATAERR, process.returncode) + self.assertTrue(check_output_status( + stdout, exp_lic_text_exits=FAILURE, + exp_lic_files_referenced=WARNING)) def open_subprocess(test_data_name: str): @@ -229,5 +260,45 @@ def open_subprocess(test_data_name: str): return process, stdout +def check_output_status(output: str, + exp_lic_tag_exists: Status = Status.SUCCESS, + exp_lic_tag_spdx: Status = Status.SUCCESS, + exp_lic_text_exits: Status = Status.SUCCESS, + exp_lic_in_code: Status = Status.SUCCESS, + exp_lic_files_referenced: Status = Status.SUCCESS + ) -> bool: + """Check output of each check for expected status.""" + # pylint: disable=too-many-arguments + + real_lic_tag_exists = get_test_result(output, "LicenseTagExistsCheck") + real_lic_tag_spdx = get_test_result(output, "LicenseTagIsInSpdxListCheck") + real_lic_text_exits = get_test_result(output, "LicenseTextExistsCheck") + real_lic_in_code = get_test_result(output, "LicensesInCodeCheck") + real_lic_files_referenced = get_test_result( + output, "LicenseFilesReferencedCheck") + + return exp_lic_tag_exists == real_lic_tag_exists \ + and exp_lic_tag_spdx == real_lic_tag_spdx \ + and exp_lic_text_exits == real_lic_text_exits \ + and exp_lic_in_code == real_lic_in_code \ + and exp_lic_files_referenced == real_lic_files_referenced + + +def get_test_result(output: str, test_name: str) -> Optional[Status]: + """Get single test result for specific test.""" + lines = output.splitlines() + for i, line in enumerate(lines): + if test_name in str(line): + if i + 1 < len(lines): + result_line = str(lines[i + 1]) + if "FAILURE" in result_line: + return FAILURE + if "WARNING" in result_line: + return WARNING + if "SUCCESS" in result_line: + return SUCCESS + return None + + if __name__ == '__main__': unittest.main() From 183d2b7413bffc3e652c098cdc50a5d7b74a9b22 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 8 Apr 2024 11:48:02 +0200 Subject: [PATCH 02/21] implemented change requests Signed-off-by: Anton Utz --- test/systemtest/test_separate_pkgs.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/systemtest/test_separate_pkgs.py b/test/systemtest/test_separate_pkgs.py index 0fa3b86..da061cb 100644 --- a/test/systemtest/test_separate_pkgs.py +++ b/test/systemtest/test_separate_pkgs.py @@ -267,7 +267,9 @@ def check_output_status(output: str, exp_lic_in_code: Status = Status.SUCCESS, exp_lic_files_referenced: Status = Status.SUCCESS ) -> bool: - """Check output of each check for expected status.""" + """Check output of each check for expected status. + each argument except for output tells the expected status of a + certain check. The default is always SUCCESS.""" # pylint: disable=too-many-arguments real_lic_tag_exists = get_test_result(output, "LicenseTagExistsCheck") @@ -285,12 +287,15 @@ def check_output_status(output: str, def get_test_result(output: str, test_name: str) -> Optional[Status]: - """Get single test result for specific test.""" + """Get result status for specific test. + Return None if no status could be found""" lines = output.splitlines() for i, line in enumerate(lines): - if test_name in str(line): - if i + 1 < len(lines): - result_line = str(lines[i + 1]) + if test_name not in str(line): + continue + if i + 1 > len(lines): + continue + result_line = str(lines[i + 1]) if "FAILURE" in result_line: return FAILURE if "WARNING" in result_line: From 55e2230cfc016e6348289d22fe837ce037f678fb Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 8 Apr 2024 11:54:59 +0200 Subject: [PATCH 03/21] linter issues Signed-off-by: Anton Utz --- test/systemtest/test_separate_pkgs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/systemtest/test_separate_pkgs.py b/test/systemtest/test_separate_pkgs.py index 394db05..6afb94f 100644 --- a/test/systemtest/test_separate_pkgs.py +++ b/test/systemtest/test_separate_pkgs.py @@ -261,7 +261,7 @@ def check_output_status(output: str, exp_lic_files_referenced: Status = Status.SUCCESS ) -> bool: """Check output of each check for expected status. - each argument except for output tells the expected status of a + each argument except for output tells the expected status of a certain check. The default is always SUCCESS.""" # pylint: disable=too-many-arguments From d21b4deff15ca371c529ed535e1d29dbe691316f Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 11 Nov 2024 12:36:42 +0100 Subject: [PATCH 04/21] added location of check Signed-off-by: Anton Utz --- src/ros_license_toolkit/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index 0206739..b0b2c69 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -145,6 +145,7 @@ def process_one_pkg(rll_print, package): rll_print( f'git hash of ({package.repo.get_path()}):' f' {package.repo.get_hash()}') + # check for scheme here, then insert scheme version in constructor checks_to_perform = [ LicenseTagExistsCheck(), LicenseTagIsInSpdxListCheck(), From 6ca9be727e24008976f7d90dc8d1dcb6297e4ca2 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 11 Nov 2024 13:53:28 +0100 Subject: [PATCH 05/21] added check class Signed-off-by: Anton Utz --- src/ros_license_toolkit/checks.py | 8 ++++++++ src/ros_license_toolkit/main.py | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/ros_license_toolkit/checks.py b/src/ros_license_toolkit/checks.py index 89698ea..22e9f21 100644 --- a/src/ros_license_toolkit/checks.py +++ b/src/ros_license_toolkit/checks.py @@ -112,6 +112,14 @@ def _check(self, package: Package): raise NotImplementedError("Overwrite this") +class SchemaCheck(Check): + """This checks the xml scheme and returns the version number.""" + + def check(self, package): + + return 3 + + class LicenseTagExistsCheck(Check): """This ensures that a tag defining the license exists.""" diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index b0b2c69..a3ef08f 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -24,7 +24,8 @@ import timeit from typing import Optional, Sequence -from ros_license_toolkit.checks import (LicenseFilesReferencedCheck, +from ros_license_toolkit.checks import (SchemaCheck, + LicenseFilesReferencedCheck, LicensesInCodeCheck, LicenseTagExistsCheck, LicenseTagIsInSpdxListCheck, @@ -146,6 +147,8 @@ def process_one_pkg(rll_print, package): f'git hash of ({package.repo.get_path()}):' f' {package.repo.get_hash()}') # check for scheme here, then insert scheme version in constructor + schema_check = SchemaCheck() + scheme_version = schema_check.check(package) checks_to_perform = [ LicenseTagExistsCheck(), LicenseTagIsInSpdxListCheck(), From 2da0559eaca4c0e5ad90a9ec40ff5574c563f429 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Fri, 15 Nov 2024 10:36:11 +0100 Subject: [PATCH 06/21] added test for xml conformity Signed-off-by: Anton Utz --- .../test_pkg_scheme3_conform/LICENSE | 46 +++++++++++++++++++ .../test_pkg_scheme3_conform/package.xml | 10 ++++ test/systemtest/test_all_packages.py | 1 + test/systemtest/test_separate_pkgs.py | 7 +++ 4 files changed, 64 insertions(+) create mode 100644 test/_test_data/test_pkg_scheme3_conform/LICENSE create mode 100644 test/_test_data/test_pkg_scheme3_conform/package.xml diff --git a/test/_test_data/test_pkg_scheme3_conform/LICENSE b/test/_test_data/test_pkg_scheme3_conform/LICENSE new file mode 100644 index 0000000..008ba30 --- /dev/null +++ b/test/_test_data/test_pkg_scheme3_conform/LICENSE @@ -0,0 +1,46 @@ +The Academic Free License + v. 2.0 + +This Academic Free License (the "License") applies to any original work of authorship (the "Original Work") whose owner (the "Licensor") has placed the following notice immediately following the copyright notice for the Original Work: +Licensed under the Academic Free License version 2.0 + +1) Grant of Copyright License. Licensor hereby grants You a world-wide, royalty-free, non-exclusive, perpetual, sublicenseable license to do the following: +a) to reproduce the Original Work in copies; + +b) to prepare derivative works ("Derivative Works") based upon the Original Work; + +c) to distribute copies of the Original Work and Derivative Works to the public; + +d) to perform the Original Work publicly; and + +e) to display the Original Work publicly. + +2) Grant of Patent License. Licensor hereby grants You a world-wide, royalty-free, non-exclusive, perpetual, sublicenseable license, under patent claims owned or controlled by the Licensor that are embodied in the Original Work as furnished by the Licensor, to make, use, sell and offer for sale the Original Work and Derivative Works. + +3) Grant of Source Code License. The term "Source Code" means the preferred form of the Original Work for making modifications to it and all available documentation describing how to modify the Original Work. Licensor hereby agrees to provide a machine-readable copy of the Source Code of the Original Work along with each copy of the Original Work that Licensor distributes. Licensor reserves the right to satisfy this obligation by placing a machine-readable copy of the Source Code in an information repository reasonably calculated to permit inexpensive and convenient access by You for as long as Licensor continues to distribute the Original Work, and by publishing the address of that information repository in a notice immediately following the copyright notice that applies to the Original Work. + +4) Exclusions From License Grant. Neither the names of Licensor, nor the names of any contributors to the Original Work, nor any of their trademarks or service marks, may be used to endorse or promote products derived from this Original Work without express prior written permission of the Licensor. Nothing in this License shall be deemed to grant any rights to trademarks, copyrights, patents, trade secrets or any other intellectual property of Licensor except as expressly stated herein. No patent license is granted to make, use, sell or offer to sell embodiments of any patent claims other than the licensed claims defined in Section 2. No right is granted to the trademarks of Licensor even if such marks are included in the Original Work. Nothing in this License shall be interpreted to prohibit Licensor from licensing under different terms from this License any Original Work that Licensor otherwise would have a right to license. + +5) This section intentionally omitted. + +6) Attribution Rights. You must retain, in the Source Code of any Derivative Works that You create, all copyright, patent or trademark notices from the Source Code of the Original Work, as well as any notices of licensing and any descriptive text identified therein as an "Attribution Notice." You must cause the Source Code for any Derivative Works that You create to carry a prominent Attribution Notice reasonably calculated to inform recipients that You have modified the Original Work. + +7) Warranty of Provenance and Disclaimer of Warranty. Licensor warrants that the copyright in and to the Original Work and the patent rights granted herein by Licensor are owned by the Licensor or are sublicensed to You under the terms of this License with the permission of the contributor(s) of those copyrights and patent rights. Except as expressly stated in the immediately proceeding sentence, the Original Work is provided under this License on an "AS IS" BASIS and WITHOUT WARRANTY, either express or implied, including, without limitation, the warranties of NON-INFRINGEMENT, MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. THE ENTIRE RISK AS TO THE QUALITY OF THE ORIGINAL WORK IS WITH YOU. This DISCLAIMER OF WARRANTY constitutes an essential part of this License. No license to Original Work is granted hereunder except under this disclaimer. + +8) Limitation of Liability. Under no circumstances and under no legal theory, whether in tort (including negligence), contract, or otherwise, shall the Licensor be liable to any person for any direct, indirect, special, incidental, or consequential damages of any character arising as a result of this License or the use of the Original Work including, without limitation, damages for loss of goodwill, work stoppage, computer failure or malfunction, or any and all other commercial damages or losses. This limitation of liability shall not apply to liability for death or personal injury resulting from Licensor's negligence to the extent applicable law prohibits such limitation. Some jurisdictions do not allow the exclusion or limitation of incidental or consequential damages, so this exclusion and limitation may not apply to You. + +9) Acceptance and Termination. If You distribute copies of the Original Work or a Derivative Work, You must make a reasonable effort under the circumstances to obtain the express assent of recipients to the terms of this License. Nothing else but this License (or another written agreement between Licensor and You) grants You permission to create Derivative Works based upon the Original Work or to exercise any of the rights granted in Section 1 herein, and any attempt to do so except under the terms of this License (or another written agreement between Licensor and You) is expressly prohibited by U.S. copyright law, the equivalent laws of other countries, and by international treaty. Therefore, by exercising any of the rights granted to You in Section 1 herein, You indicate Your acceptance of this License and all of its terms and conditions. + +10) Termination for Patent Action. This License shall terminate automatically and You may no longer exercise any of the rights granted to You by this License as of the date You commence an action, including a cross-claim or counterclaim, for patent infringement (i) against Licensor with respect to a patent applicable to software or (ii) against any entity with respect to a patent applicable to the Original Work (but excluding combinations of the Original Work with other software or hardware). + +11) Jurisdiction, Venue and Governing Law. Any action or suit relating to this License may be brought only in the courts of a jurisdiction wherein the Licensor resides or in which Licensor conducts its primary business, and under the laws of that jurisdiction excluding its conflict-of-law provisions. The application of the United Nations Convention on Contracts for the International Sale of Goods is expressly excluded. Any use of the Original Work outside the scope of this License or after its termination shall be subject to the requirements and penalties of the U.S. Copyright Act, 17 U.S.C. � 101 et seq., the equivalent laws of other countries, and international treaty. This section shall survive the termination of this License. + +12) Attorneys Fees. In any action to enforce the terms of this License or seeking damages relating thereto, the prevailing party shall be entitled to recover its costs and expenses, including, without limitation, reasonable attorneys' fees and costs incurred in connection with such action, including any appeal of such action. This section shall survive the termination of this License. + +13) Miscellaneous. This License represents the complete agreement concerning the subject matter hereof. If any provision of this License is held to be unenforceable, such provision shall be reformed only to the extent necessary to make it enforceable. + +14) Definition of "You" in This License. "You" throughout this License, whether in upper or lower case, means an individual or a legal entity exercising rights under, and complying with all of the terms of, this License. For legal entities, "You" includes any entity that controls, is controlled by, or is under common control with you. For purposes of this definition, "control" means (i) the power, direct or indirect, to cause the direction or management of such entity, whether by contract or otherwise, or (ii) ownership of fifty percent (50%) or more of the outstanding shares, or (iii) beneficial ownership of such entity. + +15) Right to Use. You may use the Original Work in all ways not otherwise restricted or conditioned by this License or by law, and Licensor promises not to interfere with or be responsible for such uses by You. + +This license is Copyright (C) 2003 Lawrence E. Rosen. All rights reserved. Permission is hereby granted to copy and distribute this license without modification. This license may not be modified without the express written permission of its copyright owner. \ No newline at end of file diff --git a/test/_test_data/test_pkg_scheme3_conform/package.xml b/test/_test_data/test_pkg_scheme3_conform/package.xml new file mode 100644 index 0000000..86f0f3d --- /dev/null +++ b/test/_test_data/test_pkg_scheme3_conform/package.xml @@ -0,0 +1,10 @@ + + + + test_pkg_scheme3_conform + 1.2.3 + + + AFL-2.0 + + diff --git a/test/systemtest/test_all_packages.py b/test/systemtest/test_all_packages.py index 6908b5a..97824e4 100644 --- a/test/systemtest/test_all_packages.py +++ b/test/systemtest/test_all_packages.py @@ -56,6 +56,7 @@ def test_all(self): self.assertIn(b"test_pkg_no_license", stdout) self.assertIn(b"test_pkg_no_license_file", stdout) self.assertIn(b"test_pkg_one_correct_one_license_file_missing", stdout) + self.assertIn(b"test_pkg_scheme3_conform", stdout) self.assertIn(b"test_pkg_spdx_tag", stdout) self.assertIn(b"test_pkg_too_many_license_files", stdout) self.assertIn(b"test_pkg_tag_not_spdx", stdout) diff --git a/test/systemtest/test_separate_pkgs.py b/test/systemtest/test_separate_pkgs.py index 6afb94f..7d8fa9f 100644 --- a/test/systemtest/test_separate_pkgs.py +++ b/test/systemtest/test_separate_pkgs.py @@ -161,6 +161,13 @@ def test_pkg_one_correct_one_license_file_missing(self): self.assertTrue(check_output_status( stdout, SUCCESS, WARNING, FAILURE, FAILURE, SUCCESS)) + def test_pkg_scheme3_conform(self): + """Test on a package that has all attributes for being conform to + the official scheme v3""" + process, stdout = open_subprocess("test_pkg_scheme3_conform") + self.assertEqual(os.EX_OK, process.returncode) + self.assertTrue(check_output_status(stdout)) + def test_pkg_spdx_tag(self): """Test on a package with a license declared in the package.xml with the SPDX tag.""" From 01d8ef72efb34b936146c19707e994abfcaf16c5 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Fri, 15 Nov 2024 10:37:32 +0100 Subject: [PATCH 07/21] added check for validating xml with scheme, output not yet processed though Signed-off-by: Anton Utz --- schemas/package_common.xsd | 89 +++++++++++++++++++++++ schemas/package_format1.xsd | 69 ++++++++++++++++++ schemas/package_format2.xsd | 73 +++++++++++++++++++ schemas/package_format3.xsd | 113 ++++++++++++++++++++++++++++++ src/ros_license_toolkit/checks.py | 11 ++- 5 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 schemas/package_common.xsd create mode 100644 schemas/package_format1.xsd create mode 100644 schemas/package_format2.xsd create mode 100644 schemas/package_format3.xsd diff --git a/schemas/package_common.xsd b/schemas/package_common.xsd new file mode 100644 index 0000000..afda493 --- /dev/null +++ b/schemas/package_common.xsd @@ -0,0 +1,89 @@ + + + + + + The version number must have the form "X.Y.Z" where X, Y, and Z + are non-negative integers, and must not contain leading zeroes. + + + + + + + + + + + The description allows any content but should be limited to XHTML. + + + + + + + + + + + A valid email address must follow several complex rules + (see https://en.wikipedia.org/wiki/Email_address). + For ROS packages only a few are enforced, and not the full character set + is supported. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + The version limit must have the form "X.Y.Z", "X.Y", or "X". + See documentation for VersionType for further details. + + + + + + + + + + + + + diff --git a/schemas/package_format1.xsd b/schemas/package_format1.xsd new file mode 100644 index 0000000..b07616a --- /dev/null +++ b/schemas/package_format1.xsd @@ -0,0 +1,69 @@ + + + + + + + + + + + + + + + + + + + + + + + + + Specified in REP 127 (see http://www.ros.org/reps/rep-0127.html). + + + + + + + + The package name should be unique within the ROS community. + It may differ from the folder name into which it is checked out, + but that is not recommended. + It must start with a lower-case letter and consist of only + lower-case letters, numbers and underscores. + It must not have two consecutive underscores. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/schemas/package_format2.xsd b/schemas/package_format2.xsd new file mode 100644 index 0000000..73cd9c1 --- /dev/null +++ b/schemas/package_format2.xsd @@ -0,0 +1,73 @@ + + + + + + + + + + + + + + + + + + + + + + + + + Specified in REP 140 (see http://www.ros.org/reps/rep-0140.html). + + + + + + + + The package name should be unique within the ROS community. + It may differ from the folder name into which it is checked out, + but that is not recommended. + It must start with a lower-case letter and consist of only + lower-case letters, numbers and underscores. + It must not have two consecutive underscores. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/schemas/package_format3.xsd b/schemas/package_format3.xsd new file mode 100644 index 0000000..c09cd8e --- /dev/null +++ b/schemas/package_format3.xsd @@ -0,0 +1,113 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Specified in REP 149 (see http://www.ros.org/reps/rep-0149.html). + + + + + + + + The package name should be unique within the ROS community. + It may differ from the folder name into which it is checked out, + but that is not recommended. + It must start with a lower-case letter and consist of only + lower-case letters, numbers and underscores. + It must not have two consecutive underscores. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/ros_license_toolkit/checks.py b/src/ros_license_toolkit/checks.py index 22e9f21..310cf3c 100644 --- a/src/ros_license_toolkit/checks.py +++ b/src/ros_license_toolkit/checks.py @@ -20,6 +20,7 @@ from enum import IntEnum from pprint import pformat from typing import Any, Dict, List, Optional +from lxml import etree from ros_license_toolkit.common import get_spdx_license_name from ros_license_toolkit.license_tag import (LicenseTag, @@ -116,9 +117,17 @@ class SchemaCheck(Check): """This checks the xml scheme and returns the version number.""" def check(self, package): - + valid = self.validate(package.abspath + "/package.xml", "") return 3 + def validate(self, xml_cont: str, xsd_cont: str) -> bool: + xml_schema_doc = etree.parse('./schemas/package_format3.xsd') + xmlschema = etree.XMLSchema(xml_schema_doc) + + xml_doc = etree.parse(xml_cont) + result = xmlschema.validate(xml_doc) + return result + class LicenseTagExistsCheck(Check): """This ensures that a tag defining the license exists.""" From 7bf022e1474a472bdacb228e63afbaf6d833015e Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 18 Nov 2024 12:35:13 +0100 Subject: [PATCH 08/21] created whole check out of validation function. introduced success and failure messages Signed-off-by: Anton Utz --- src/ros_license_toolkit/checks.py | 69 +++++++++++++++++++++++++------ src/ros_license_toolkit/main.py | 10 +++-- 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/src/ros_license_toolkit/checks.py b/src/ros_license_toolkit/checks.py index 310cf3c..0a0864c 100644 --- a/src/ros_license_toolkit/checks.py +++ b/src/ros_license_toolkit/checks.py @@ -19,7 +19,7 @@ import os from enum import IntEnum from pprint import pformat -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from lxml import etree from ros_license_toolkit.common import get_spdx_license_name @@ -115,18 +115,61 @@ def _check(self, package: Package): class SchemaCheck(Check): """This checks the xml scheme and returns the version number.""" - - def check(self, package): - valid = self.validate(package.abspath + "/package.xml", "") - return 3 - - def validate(self, xml_cont: str, xsd_cont: str) -> bool: - xml_schema_doc = etree.parse('./schemas/package_format3.xsd') - xmlschema = etree.XMLSchema(xml_schema_doc) - - xml_doc = etree.parse(xml_cont) - result = xmlschema.validate(xml_doc) - return result + def __init__(self): + super().__init__() + xml_schema_3_parsed = etree.parse('./schemas/package_format3.xsd') + self.xml_schema_3 = etree.XMLSchema(xml_schema_3_parsed) + xml_schema_2_parsed = etree.parse('./schemas/package_format3.xsd') + self.xml_schema_2 = etree.XMLSchema(xml_schema_2_parsed) + xml_schema_1_parsed = etree.parse('./schemas/package_format1.xsd') + self.xml_schema_1 = etree.XMLSchema(xml_schema_1_parsed) + + def _check(self, package): + version, status = self.validate(package.abspath + "/package.xml") + if status: + self._success(f"Detected package.xml version {version}, " + "validation of scheme successful.") + else: + if version == -1: + self._failed("package.xml does not contain correct package " + "format number. Please use a real version. " + "(e.g. )") + elif version == 1: + self._failed( + "package.xml contains errors: " + f"{self.xml_schema_1.error_log.last_error.message}") + elif version == 2: + self._failed( + "package.xml contains errors: " + f"{self.xml_schema_2.error_log.last_error.message}") + elif version == 3: + self._failed( + "package.xml contains errors: " + f"{self.xml_schema_3.error_log.last_error.message}") + + def validate(self, pck_xml_path: str) -> Tuple[int, bool]: + """This is validating package.xml file from given path. + Automatically detects version number and validates + it with corresponding scheme, e.g. format 3. + If everything is correct, returns format number, else -1.""" + xml_doc_parsed = etree.parse(pck_xml_path) + for element in xml_doc_parsed.getiterator(): + if element.tag == 'package' and 'format' in element.attrib: + version = element.attrib['format'] + status_check = False + if version == '3': + version = 3 + status_check = self.xml_schema_3.validate(xml_doc_parsed) + elif version == '2': + version = 2 + status_check = self.xml_schema_2.validate(xml_doc_parsed) + elif version == '1': + version = 1 + status_check = self.xml_schema_1.validate(xml_doc_parsed) + else: + version = -1 + return (version, status_check) + return (-1, False) class LicenseTagExistsCheck(Check): diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index a3ef08f..5a31b95 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -24,12 +24,12 @@ import timeit from typing import Optional, Sequence -from ros_license_toolkit.checks import (SchemaCheck, - LicenseFilesReferencedCheck, +from ros_license_toolkit.checks import (LicenseFilesReferencedCheck, LicensesInCodeCheck, LicenseTagExistsCheck, LicenseTagIsInSpdxListCheck, - LicenseTextExistsCheck, Status) + LicenseTextExistsCheck, SchemaCheck, + Status) from ros_license_toolkit.package import get_packages_in_path from ros_license_toolkit.ui_elements import (FAILURE_STR, SUCCESS_STR, WARNING_STR, Verbosity, major_sep, @@ -148,7 +148,9 @@ def process_one_pkg(rll_print, package): f' {package.repo.get_hash()}') # check for scheme here, then insert scheme version in constructor schema_check = SchemaCheck() - scheme_version = schema_check.check(package) + schema_check.check(package) + rll_print(schema_check) + rll_print(schema_check.verbose(), Verbosity.VERBOSE) checks_to_perform = [ LicenseTagExistsCheck(), LicenseTagIsInSpdxListCheck(), From 03279cba59cac3e4d4d6a2f23c4a096e1d335cfb Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Thu, 21 Nov 2024 10:38:43 +0100 Subject: [PATCH 09/21] corrected double code Signed-off-by: Anton Utz --- src/ros_license_toolkit/checks.py | 302 +----------------------------- src/ros_license_toolkit/main.py | 17 +- 2 files changed, 15 insertions(+), 304 deletions(-) diff --git a/src/ros_license_toolkit/checks.py b/src/ros_license_toolkit/checks.py index 134337a..2d52012 100644 --- a/src/ros_license_toolkit/checks.py +++ b/src/ros_license_toolkit/checks.py @@ -16,9 +16,10 @@ """This module contains the checks for the linter.""" + from enum import IntEnum -from pprint import pformat -from typing import Any, Dict, List, Optional, Tuple +from typing import Tuple + from lxml import etree from ros_license_toolkit.package import Package, PackageException @@ -148,7 +149,7 @@ def validate(self, pck_xml_path: str) -> Tuple[int, bool]: Automatically detects version number and validates it with corresponding scheme, e.g. format 3. If everything is correct, returns format number, else -1.""" - xml_doc_parsed = etree.parse(pck_xml_path) + xml_doc_parsed: etree._ElementTree = etree.parse(pck_xml_path) for element in xml_doc_parsed.getiterator(): if element.tag == 'package' and 'format' in element.attrib: version = element.attrib['format'] @@ -166,298 +167,3 @@ def validate(self, pck_xml_path: str) -> Tuple[int, bool]: version = -1 return (version, status_check) return (-1, False) - - -class LicenseTagExistsCheck(Check): - """This ensures that a tag defining the license exists.""" - - def _check(self, package: Package): - if len(package.license_tags) == 0: - self._failed("No license tag defined.") - self.verbose_output = red(str(package.package_xml)) - else: - self._success( - f"Found licenses {list(map(str, package.license_tags))}") - - -class LicenseTagIsInSpdxListCheck(Check): - """This ensures that the license tag is in the SPDX list of licenses.""" - - def _check(self, package: Package): - licenses_not_in_spdx_list = [] - for license_tag in package.license_tags.keys(): - if not is_license_name_in_spdx_list( - license_tag): - licenses_not_in_spdx_list.append(license_tag) - if len(licenses_not_in_spdx_list) > 0: - self._warning( - f"Licenses {licenses_not_in_spdx_list} are " - "not in SPDX list of licenses. " - "Make sure to exactly match one of https://spdx.org/licenses/." - ) - else: - self._success("All license tags are in SPDX list of licenses.") - - -class LicenseTextExistsCheck(Check): - """This ensures that the license text file referenced by the tag exists.""" - - def __init__(self: 'LicenseTextExistsCheck'): - Check.__init__(self) - self.license_tags_without_license_text: Dict[LicenseTag, str] = {} - self.missing_license_texts_status: Dict[LicenseTag, Status] = {} - self.files_with_wrong_tags: Dict[LicenseTag, Dict[str, str]] = {} - self.found_license_texts: Dict[str, Any] = {} - - def _check(self, package: Package): - if len(package.license_tags) == 0: - self._failed("No license tag defined.") - return - - self._check_licenses(package) - self._evaluate_results() - - def _check_licenses(self, package: Package) -> None: - '''checks each license tag for the corresponding license text. Also - detects inofficial licenses when tag is not in the SPDX license list''' - self.found_license_texts = package.found_license_texts - for license_tag in package.license_tags.values(): - if not license_tag.has_license_text_file(): - self.license_tags_without_license_text[ - license_tag] = "No license text file defined." - self.missing_license_texts_status[license_tag] = Status.FAILURE - continue - license_text_file = license_tag.get_license_text_file() - if not os.path.exists( - os.path.join(package.abspath, license_text_file)): - self.license_tags_without_license_text[license_tag] =\ - f"License text file '{license_text_file}' does not exist." - self.missing_license_texts_status[license_tag] = Status.FAILURE - continue - if license_text_file not in self.found_license_texts: - self.license_tags_without_license_text[license_tag] =\ - f"License text file '{license_text_file}' not included" +\ - " in scan results." - self.missing_license_texts_status[license_tag] = Status.FAILURE - continue - if not get_spdx_license_name( - self.found_license_texts[license_text_file]): - self.license_tags_without_license_text[license_tag] =\ - f"License text file '{license_text_file}' is not " +\ - "recognized as license text." - self.missing_license_texts_status[license_tag] = Status.FAILURE - continue - actual_license: Optional[str] = get_spdx_license_name( - self.found_license_texts[license_text_file]) - if actual_license is None: - self.license_tags_without_license_text[ - license_tag - ] = f"License text file '{license_text_file}'" +\ - " is not recognized as license text." - self.missing_license_texts_status[license_tag] = Status.FAILURE - continue - if actual_license != license_tag.get_license_id(): - self.license_tags_without_license_text[license_tag] =\ - f"License text file '{license_text_file}' is " +\ - f"of license {actual_license} but tag is " +\ - f"{license_tag.get_license_id()}." - # If Tag and File both are in SPDX but don't match -> Error - if is_license_name_in_spdx_list(license_tag.get_license_id()): - self.missing_license_texts_status[license_tag] =\ - Status.FAILURE - else: - self.missing_license_texts_status[license_tag] =\ - Status.WARNING - self.files_with_wrong_tags[license_tag] = \ - {'actual_license': actual_license, - 'license_tag': license_tag.get_license_id()} - continue - - def _evaluate_results(self): - if len(self.license_tags_without_license_text) > 0: - if max(self.missing_license_texts_status.values()) \ - == Status.WARNING: - self._warning( - "Since they are not in the SPDX list, " - "we can not check if these tags have the correct " - "license text:\n" + "\n".join( - [f" '{x[0]}': {x[1]}" for x in - self.license_tags_without_license_text.items()])) - else: - self._failed( - "The following license tags do not " - "have a valid license text " - "file:\n" + "\n".join( - [f" '{x[0]}': {x[1]}" for x in - self.license_tags_without_license_text.items()])) - self.verbose_output = red( - "\n".join([f" '{x[0]}': {x[1]}" for x in - self.found_license_texts.items()])) - else: - self._success("All license tags have a valid license text file.") - - -class LicensesInCodeCheck(Check): - """Check if all found licenses have a declaration in the package.xml.""" - - def __init__(self: 'LicensesInCodeCheck'): - Check.__init__(self) - self.declared_licenses: Dict[str, LicenseTag] = {} - self.files_with_uncovered_licenses: Dict[str, List[str]] = {} - self.files_not_matched_by_any_license_tag: Dict[str, List[str]] = {} - self.files_with_inofficial_tag: Dict[str, List[str]] = {} - - def _check(self, package: Package): - if len(package.license_tags) == 0: - self._failed('No license tag defined.') - return - self.declared_licenses = package.license_tags - self._check_license_files(package) - self._evaluate_result(package) - - def _check_license_files(self, package: Package) -> None: - for fname, found_licenses in package.found_files_w_licenses.items(): - if fname in package.get_license_files(): - # the actual license text files are not relevant for this - continue - found_licenses_str = found_licenses[ - 'detected_license_expression_spdx'] - if not found_licenses_str: - continue - licenses = found_licenses_str.split(' AND ') - for license_str in licenses: - if license_str not in self.declared_licenses: - # this license has an inofficial tag - inofficial_licenses = { - lic_tag.id_from_license_text: key - for key, lic_tag in package.license_tags.items() - if lic_tag.id_from_license_text != ''} - if license_str in inofficial_licenses.keys(): - if fname not in self.files_with_inofficial_tag: - self.files_with_inofficial_tag[fname] = [] - self.files_with_inofficial_tag[fname].append( - license_str) - self.files_with_inofficial_tag[fname].append( - inofficial_licenses[license_str]) - continue - # this license is not declared by any license tag - if fname not in self.files_with_uncovered_licenses: - self.files_with_uncovered_licenses[fname] = [] - self.files_with_uncovered_licenses[fname].append( - license_str) - continue - if fname not in self.declared_licenses[ - license_str].source_files: - # this license is declared by a license tag but the file - # is not listed in the source files of the license tag - if fname not in self.files_not_matched_by_any_license_tag: - self.files_not_matched_by_any_license_tag[fname] = [] - self.files_not_matched_by_any_license_tag[fname].append( - license_str) - continue - - def _evaluate_result(self, package: Package) -> None: - if self.files_with_uncovered_licenses: - info_str = '' - info_str += '\nThe following files contain licenses that ' +\ - 'are not covered by any license tag:\n' + '\n'.join( - [f" '{x[0]}': {x[1]}" for x in - self.files_with_uncovered_licenses.items()]) - self._print_info( - info_str, - self.files_with_uncovered_licenses, - self.files_not_matched_by_any_license_tag, - package, - ) - elif len(self.files_not_matched_by_any_license_tag) > 0: - info_str = '' - info_str += '\nThe following files contain licenses that ' +\ - 'are covered by a license tag but are not listed in ' +\ - 'the source files of the license tag:\n' + '\n'.join( - [f" '{x[0]}': {x[1]}" for x in - self.files_not_matched_by_any_license_tag.items()]) - self._print_info( - info_str, - self.files_with_uncovered_licenses, - self.files_not_matched_by_any_license_tag, - package, - ) - elif self.files_with_inofficial_tag: - info_str = '' - info_str += 'For the following files, please change the ' +\ - 'License Tag in the package file to SPDX format:\n' +\ - '\n'.join( - [f" '{x[0]}' is of {x[1][0]} but its Tag is {x[1][1]}." - for x in self.files_with_inofficial_tag.items()]) - self._warning(info_str) - else: - self._success('All licenses found in the code are covered by a ' - 'license declaration.') - - def _print_info(self, info_str, files_with_uncovered_licenses, - files_not_matched_by_any_license_tag, package): - assert info_str != '' - self._failed(info_str) - self.verbose_output = red( - '\n Relevant scan results:\n' + pformat( - list(filter( - lambda x: x[0] in files_with_uncovered_licenses or ( - x[0] in files_not_matched_by_any_license_tag), - package.found_files_w_licenses.items())))) - - -class LicenseFilesReferencedCheck(Check): - """Check if all found License file have a reference in package.xml.""" - - def _check(self, package: Package): - not_covered_texts: Dict[str, str] = {} - inofficial_covered_texts: Dict[str, List[str]] = {} - for filename, license_text in package.found_license_texts.items(): - # skipping all declarations above the package - if not is_in_package(package, filename): - continue - if 'detected_license_expression_spdx' in license_text and \ - license_text['detected_license_expression_spdx'] not in \ - package.license_tags: - spdx_expression = license_text[ - 'detected_license_expression_spdx'] - inofficial_licenses = { - lic_tag.id_from_license_text: key - for key, lic_tag in package.license_tags.items() - if lic_tag.id_from_license_text != ''} - if spdx_expression in inofficial_licenses: - inofficial_covered_texts[filename] = \ - [spdx_expression, - inofficial_licenses[spdx_expression]] - else: - not_covered_texts[filename] = \ - spdx_expression - if not_covered_texts: - info_str = '' - info_str += 'The following license files are not' +\ - ' mentioned by any tag:\n' +\ - '\n'.join( - [f" '{x[0]}' is of {x[1]}." - for x in not_covered_texts.items()]) - self._failed(info_str) - elif inofficial_covered_texts: - info_str = '' - info_str += 'The following license files are not' +\ - ' mentioned by any tag:\n' +\ - '\n'.join( - [f" '{x[0]}' is of {x[1][0]} but its tag is {x[1][1]}." - for x in inofficial_covered_texts.items()]) - self._warning(info_str) - else: - self._success("All license declaration are referenced by a tag.") - - -def is_in_package(package: Package, file: str) -> bool: - """Return TRUE if the file is underneath the absolute package path. - Return FALSE if file is located above package.""" - parent = os.path.abspath(package.abspath) - child = os.path.abspath(package.abspath + '/' + file) - - comm_parent = os.path.commonpath([parent]) - comm_child_parent = os.path.commonpath([parent, child]) - return comm_parent == comm_child_parent diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index 6cd04f9..ae11d0a 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -24,12 +24,17 @@ import timeit from typing import Optional, Sequence -from ros_license_toolkit.checks import (LicenseFilesReferencedCheck, - LicensesInCodeCheck, - LicenseTagExistsCheck, - LicenseTagIsInSpdxListCheck, - LicenseTextExistsCheck, SchemaCheck, - Status) +from ros_license_toolkit.checks import SchemaCheck, Status +from ros_license_toolkit.license_checks.license_file_referenced_check import \ + LicenseFilesReferencedCheck +from ros_license_toolkit.license_checks.license_in_code_check import \ + LicensesInCodeCheck +from ros_license_toolkit.license_checks.license_tag_exists_check import \ + LicenseTagExistsCheck +from ros_license_toolkit.license_checks.license_tag_is_spdx import \ + LicenseTagIsInSpdxListCheck +from ros_license_toolkit.license_checks.license_text_exists_check import \ + LicenseTextExistsCheck from ros_license_toolkit.package import get_packages_in_path from ros_license_toolkit.ui_elements import (FAILURE_STR, SUCCESS_STR, WARNING_STR, Verbosity, major_sep, From 9dc47619bedd560ff25ae86b4afa835586fd3a8f Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Thu, 21 Nov 2024 10:45:10 +0100 Subject: [PATCH 10/21] moved check to extra file Signed-off-by: Anton Utz --- src/ros_license_toolkit/checks.py | 61 +------------- .../license_checks/schema_check.py | 80 +++++++++++++++++++ src/ros_license_toolkit/main.py | 3 +- 3 files changed, 83 insertions(+), 61 deletions(-) create mode 100644 src/ros_license_toolkit/license_checks/schema_check.py diff --git a/src/ros_license_toolkit/checks.py b/src/ros_license_toolkit/checks.py index 2d52012..cc5ed04 100644 --- a/src/ros_license_toolkit/checks.py +++ b/src/ros_license_toolkit/checks.py @@ -18,9 +18,6 @@ from enum import IntEnum -from typing import Tuple - -from lxml import etree from ros_license_toolkit.package import Package, PackageException from ros_license_toolkit.ui_elements import NO_REASON_STR, green, red, yellow @@ -110,60 +107,4 @@ def _check(self, package: Package): raise NotImplementedError("Overwrite this") -class SchemaCheck(Check): - """This checks the xml scheme and returns the version number.""" - def __init__(self): - super().__init__() - xml_schema_3_parsed = etree.parse('./schemas/package_format3.xsd') - self.xml_schema_3 = etree.XMLSchema(xml_schema_3_parsed) - xml_schema_2_parsed = etree.parse('./schemas/package_format3.xsd') - self.xml_schema_2 = etree.XMLSchema(xml_schema_2_parsed) - xml_schema_1_parsed = etree.parse('./schemas/package_format1.xsd') - self.xml_schema_1 = etree.XMLSchema(xml_schema_1_parsed) - - def _check(self, package): - version, status = self.validate(package.abspath + "/package.xml") - if status: - self._success(f"Detected package.xml version {version}, " - "validation of scheme successful.") - else: - if version == -1: - self._failed("package.xml does not contain correct package " - "format number. Please use a real version. " - "(e.g. )") - elif version == 1: - self._failed( - "package.xml contains errors: " - f"{self.xml_schema_1.error_log.last_error.message}") - elif version == 2: - self._failed( - "package.xml contains errors: " - f"{self.xml_schema_2.error_log.last_error.message}") - elif version == 3: - self._failed( - "package.xml contains errors: " - f"{self.xml_schema_3.error_log.last_error.message}") - - def validate(self, pck_xml_path: str) -> Tuple[int, bool]: - """This is validating package.xml file from given path. - Automatically detects version number and validates - it with corresponding scheme, e.g. format 3. - If everything is correct, returns format number, else -1.""" - xml_doc_parsed: etree._ElementTree = etree.parse(pck_xml_path) - for element in xml_doc_parsed.getiterator(): - if element.tag == 'package' and 'format' in element.attrib: - version = element.attrib['format'] - status_check = False - if version == '3': - version = 3 - status_check = self.xml_schema_3.validate(xml_doc_parsed) - elif version == '2': - version = 2 - status_check = self.xml_schema_2.validate(xml_doc_parsed) - elif version == '1': - version = 1 - status_check = self.xml_schema_1.validate(xml_doc_parsed) - else: - version = -1 - return (version, status_check) - return (-1, False) + diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py new file mode 100644 index 0000000..e037bc0 --- /dev/null +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -0,0 +1,80 @@ +# Copyright (c) 2024 - for information on the respective copyright owner +# see the NOTICE file and/or the repository +# https://github.com/boschresearch/ros_license_toolkit + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""This Module contains SchemaCheck, which implements Check.""" + +from lxml import etree +from typing import Tuple +from ros_license_toolkit.checks import Check + + +class SchemaCheck(Check): + """This checks the xml scheme and returns the version number.""" + def __init__(self): + super().__init__() + xml_schema_3_parsed = etree.parse('./schemas/package_format3.xsd') + self.xml_schema_3 = etree.XMLSchema(xml_schema_3_parsed) + xml_schema_2_parsed = etree.parse('./schemas/package_format3.xsd') + self.xml_schema_2 = etree.XMLSchema(xml_schema_2_parsed) + xml_schema_1_parsed = etree.parse('./schemas/package_format1.xsd') + self.xml_schema_1 = etree.XMLSchema(xml_schema_1_parsed) + + def _check(self, package): + version, status = self.validate(package.abspath + "/package.xml") + if status: + self._success(f"Detected package.xml version {version}, " + "validation of scheme successful.") + else: + if version == -1: + self._failed("package.xml does not contain correct package " + "format number. Please use a real version. " + "(e.g. )") + elif version == 1: + self._failed( + "package.xml contains errors: " + f"{self.xml_schema_1.error_log.last_error.message}") + elif version == 2: + self._failed( + "package.xml contains errors: " + f"{self.xml_schema_2.error_log.last_error.message}") + elif version == 3: + self._failed( + "package.xml contains errors: " + f"{self.xml_schema_3.error_log.last_error.message}") + + def validate(self, pck_xml_path: str) -> Tuple[int, bool]: + """This is validating package.xml file from given path. + Automatically detects version number and validates + it with corresponding scheme, e.g. format 3. + If everything is correct, returns format number, else -1.""" + xml_doc_parsed = etree.parse(pck_xml_path) + for element in xml_doc_parsed.getiterator(): + if element.tag == 'package' and 'format' in element.attrib: + version = element.attrib['format'] + status_check = False + if version == '3': + version = 3 + status_check = self.xml_schema_3.validate(xml_doc_parsed) + elif version == '2': + version = 2 + status_check = self.xml_schema_2.validate(xml_doc_parsed) + elif version == '1': + version = 1 + status_check = self.xml_schema_1.validate(xml_doc_parsed) + else: + version = -1 + return (version, status_check) + return (-1, False) diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index ae11d0a..ebe3d99 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -24,7 +24,8 @@ import timeit from typing import Optional, Sequence -from ros_license_toolkit.checks import SchemaCheck, Status +from ros_license_toolkit.checks import Status +from ros_license_toolkit.license_checks.schema_check import SchemaCheck from ros_license_toolkit.license_checks.license_file_referenced_check import \ LicenseFilesReferencedCheck from ros_license_toolkit.license_checks.license_in_code_check import \ From abf0c4d506c69ec6fcdde94449bb5da0ef38d697 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Thu, 21 Nov 2024 10:47:31 +0100 Subject: [PATCH 11/21] fixed imports and whitespaces Signed-off-by: Anton Utz --- src/ros_license_toolkit/checks.py | 3 --- src/ros_license_toolkit/license_checks/schema_check.py | 4 +++- src/ros_license_toolkit/main.py | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ros_license_toolkit/checks.py b/src/ros_license_toolkit/checks.py index cc5ed04..3036d84 100644 --- a/src/ros_license_toolkit/checks.py +++ b/src/ros_license_toolkit/checks.py @@ -105,6 +105,3 @@ def check(self, package: Package): def _check(self, package: Package): """Check `package`. To be overwritten by subclasses.""" raise NotImplementedError("Overwrite this") - - - diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py index e037bc0..65d19c3 100644 --- a/src/ros_license_toolkit/license_checks/schema_check.py +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -16,8 +16,10 @@ """This Module contains SchemaCheck, which implements Check.""" -from lxml import etree from typing import Tuple + +from lxml import etree + from ros_license_toolkit.checks import Check diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index ebe3d99..f10c061 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -25,7 +25,6 @@ from typing import Optional, Sequence from ros_license_toolkit.checks import Status -from ros_license_toolkit.license_checks.schema_check import SchemaCheck from ros_license_toolkit.license_checks.license_file_referenced_check import \ LicenseFilesReferencedCheck from ros_license_toolkit.license_checks.license_in_code_check import \ @@ -36,6 +35,7 @@ LicenseTagIsInSpdxListCheck from ros_license_toolkit.license_checks.license_text_exists_check import \ LicenseTextExistsCheck +from ros_license_toolkit.license_checks.schema_check import SchemaCheck from ros_license_toolkit.package import get_packages_in_path from ros_license_toolkit.ui_elements import (FAILURE_STR, SUCCESS_STR, WARNING_STR, Verbosity, major_sep, From 8687856bc5ec8266200c72ed3a5b2c2e89ab4fe1 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Fri, 22 Nov 2024 09:37:33 +0100 Subject: [PATCH 12/21] changed if structure of _check method Signed-off-by: Anton Utz --- .../license_checks/schema_check.py | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py index 65d19c3..56af0ad 100644 --- a/src/ros_license_toolkit/license_checks/schema_check.py +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -40,22 +40,21 @@ def _check(self, package): self._success(f"Detected package.xml version {version}, " "validation of scheme successful.") else: - if version == -1: - self._failed("package.xml does not contain correct package " - "format number. Please use a real version. " - "(e.g. )") - elif version == 1: - self._failed( - "package.xml contains errors: " - f"{self.xml_schema_1.error_log.last_error.message}") + reason = '' + if version == 1: + reason = "package.xml contains errors: " +\ + f"{self.xml_schema_1.error_log.last_error.message}" elif version == 2: - self._failed( - "package.xml contains errors: " - f"{self.xml_schema_2.error_log.last_error.message}") + reason = "package.xml contains errors: " +\ + f"{self.xml_schema_2.error_log.last_error.message}" elif version == 3: - self._failed( - "package.xml contains errors: " - f"{self.xml_schema_3.error_log.last_error.message}") + reason = "package.xml contains errors: " +\ + f"{self.xml_schema_3.error_log.last_error.message}" + elif version == -1: + reason = "package.xml does not contain correct package " +\ + "format number. Please use a real version. " +\ + "(e.g. )" + self._failed(reason) def validate(self, pck_xml_path: str) -> Tuple[int, bool]: """This is validating package.xml file from given path. From 3d57102a60e72eb1244324d961fa7a7ff7ef5480 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 25 Nov 2024 11:41:55 +0100 Subject: [PATCH 13/21] changed tests to all fullfill scheme version Signed-off-by: Anton Utz --- .../license_checks/license_in_code_check.py | 1 + src/ros_license_toolkit/main.py | 3 ++- .../test_deep_package_folder/deeper/test_pkg_deep/package.xml | 3 +++ test/_test_data/test_pkg_both_tags_not_spdx/package.xml | 3 +++ .../test_pkg_both_tags_not_spdx_one_file_own/package.xml | 3 +++ test/_test_data/test_pkg_code_has_no_license/package.xml | 3 +++ test/_test_data/test_pkg_has_code_disjoint/package.xml | 3 +++ .../test_pkg_has_code_of_different_license/package.xml | 3 +++ .../test_pkg_has_code_of_different_license_and_tag/package.xml | 3 +++ .../package.xml | 3 +++ test/_test_data/test_pkg_ignore_readme_contents/package.xml | 3 +++ test/_test_data/test_pkg_name_not_in_spdx/package.xml | 3 +++ test/_test_data/test_pkg_no_file_attribute/package.xml | 3 +++ test/_test_data/test_pkg_no_license/package.xml | 3 +++ test/_test_data/test_pkg_no_license_file/package.xml | 3 +++ .../test_pkg_one_correct_one_license_file_missing/package.xml | 3 +++ test/_test_data/test_pkg_spdx_tag/package.xml | 3 +++ test/_test_data/test_pkg_tag_not_spdx/package.xml | 3 +++ test/_test_data/test_pkg_too_many_license_files/package.xml | 3 +++ test/_test_data/test_pkg_unknown_license/package.xml | 3 +++ .../test_pkg_unknown_license_missing_file/package.xml | 3 +++ test/_test_data/test_pkg_with_license_and_file/package.xml | 3 +++ .../package.xml | 3 +++ .../package.xml | 3 +++ test/_test_data/test_pkg_wrong_license_file/package.xml | 3 +++ test/_test_data/test_repo_bsd3/pkg_with_bsd3_a/package.xml | 3 +++ test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml | 3 +++ test/_test_data/test_repo_bsd3/test_repo_bsd3/package.xml | 3 +++ test/_test_data/test_repo_mit/pkg_with_mit_a/package.xml | 3 +++ test/_test_data/test_repo_mit/pkg_with_mit_b/package.xml | 3 +++ 30 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/ros_license_toolkit/license_checks/license_in_code_check.py b/src/ros_license_toolkit/license_checks/license_in_code_check.py index 5af49f3..f73bc39 100644 --- a/src/ros_license_toolkit/license_checks/license_in_code_check.py +++ b/src/ros_license_toolkit/license_checks/license_in_code_check.py @@ -30,6 +30,7 @@ class LicensesInCodeCheck(Check): def __init__(self: 'LicensesInCodeCheck'): Check.__init__(self) + # self.package_xml_vers = package_xml_version self.declared_licenses: Dict[str, LicenseTag] = {} self.files_with_uncovered_licenses: Dict[str, List[str]] = {} self.files_not_matched_by_any_license_tag: Dict[str, List[str]] = {} diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index f10c061..009e822 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -125,7 +125,7 @@ def main(args: Optional[Sequence[str]] = None) -> int: rll_print(f"All packages:\n {WARNING_STR}", Verbosity.QUIET) return os.EX_OK rll_print(f"All packages:\n {FAILURE_STR}", Verbosity.QUIET) - return os.EX_DATAERR + return os.EX_DATAERR # TODO change this to critical error def generate_copyright_file(packages, rll_print): @@ -169,6 +169,7 @@ def process_one_pkg(rll_print, package): rll_print(check) rll_print(check.verbose(), Verbosity.VERBOSE) + checks_to_perform.append(schema_check) rll_print(minor_sep()) # Every check is successful, no warning if max(check.status for check in checks_to_perform) == Status.SUCCESS: diff --git a/test/_test_data/test_deep_package_folder/deeper/test_pkg_deep/package.xml b/test/_test_data/test_deep_package_folder/deeper/test_pkg_deep/package.xml index 6cd795d..daee247 100644 --- a/test/_test_data/test_deep_package_folder/deeper/test_pkg_deep/package.xml +++ b/test/_test_data/test_deep_package_folder/deeper/test_pkg_deep/package.xml @@ -2,5 +2,8 @@ test_pkg_deep + 1.2.3 + describing words + BSD diff --git a/test/_test_data/test_pkg_both_tags_not_spdx/package.xml b/test/_test_data/test_pkg_both_tags_not_spdx/package.xml index f304be0..b181d2f 100644 --- a/test/_test_data/test_pkg_both_tags_not_spdx/package.xml +++ b/test/_test_data/test_pkg_both_tags_not_spdx/package.xml @@ -2,6 +2,9 @@ test_pkg_both_tags_not_spdx + 1.2.3 + describing words + BSD MPL diff --git a/test/_test_data/test_pkg_both_tags_not_spdx_one_file_own/package.xml b/test/_test_data/test_pkg_both_tags_not_spdx_one_file_own/package.xml index ee09880..1bdc20b 100644 --- a/test/_test_data/test_pkg_both_tags_not_spdx_one_file_own/package.xml +++ b/test/_test_data/test_pkg_both_tags_not_spdx_one_file_own/package.xml @@ -2,6 +2,9 @@ test_pkg_both_tags_not_spdx_one_file_own + 3.2.1 + very suggestive description + Self Created License MPL diff --git a/test/_test_data/test_pkg_code_has_no_license/package.xml b/test/_test_data/test_pkg_code_has_no_license/package.xml index 68cc407..a4d1eb7 100644 --- a/test/_test_data/test_pkg_code_has_no_license/package.xml +++ b/test/_test_data/test_pkg_code_has_no_license/package.xml @@ -2,5 +2,8 @@ test_pkg_code_has_no_license + 1.2.3 + describing words + Apache-2.0 diff --git a/test/_test_data/test_pkg_has_code_disjoint/package.xml b/test/_test_data/test_pkg_has_code_disjoint/package.xml index 7c931f5..232b8c7 100644 --- a/test/_test_data/test_pkg_has_code_disjoint/package.xml +++ b/test/_test_data/test_pkg_has_code_disjoint/package.xml @@ -2,6 +2,9 @@ test_pkg_has_code_disjoint + 1.2.3 + describing words + BSD-3-Clause Apache-2.0 diff --git a/test/_test_data/test_pkg_has_code_of_different_license/package.xml b/test/_test_data/test_pkg_has_code_of_different_license/package.xml index 80dfe16..78cc972 100644 --- a/test/_test_data/test_pkg_has_code_of_different_license/package.xml +++ b/test/_test_data/test_pkg_has_code_of_different_license/package.xml @@ -2,5 +2,8 @@ test_pkg_has_code_of_different_license + 1.2.3 + describing words + BSD-3-Clause diff --git a/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml b/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml index 5deff85..95c06b8 100644 --- a/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml +++ b/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml @@ -2,6 +2,9 @@ test_pkg_has_code_of_different_license_and_tag + 1.2.3 + describing words + BSD-3-Clause LGPL-2.1-only diff --git a/test/_test_data/test_pkg_has_code_of_different_license_and_wrong_tag/package.xml b/test/_test_data/test_pkg_has_code_of_different_license_and_wrong_tag/package.xml index ff024c2..8b48738 100644 --- a/test/_test_data/test_pkg_has_code_of_different_license_and_wrong_tag/package.xml +++ b/test/_test_data/test_pkg_has_code_of_different_license_and_wrong_tag/package.xml @@ -2,6 +2,9 @@ test_pkg_has_code_of_different_license_and_wrong_tag + 3.2.1 + very suggestive description + BSD-3-Clause LGPL-2.1-or-later diff --git a/test/_test_data/test_pkg_ignore_readme_contents/package.xml b/test/_test_data/test_pkg_ignore_readme_contents/package.xml index c03fd28..348cd78 100644 --- a/test/_test_data/test_pkg_ignore_readme_contents/package.xml +++ b/test/_test_data/test_pkg_ignore_readme_contents/package.xml @@ -2,5 +2,8 @@ test_pkg_ignore_readme_contents + 3.2.1 + very suggestive description + Apache-2.0 diff --git a/test/_test_data/test_pkg_name_not_in_spdx/package.xml b/test/_test_data/test_pkg_name_not_in_spdx/package.xml index 0e44c2a..0efe8fb 100644 --- a/test/_test_data/test_pkg_name_not_in_spdx/package.xml +++ b/test/_test_data/test_pkg_name_not_in_spdx/package.xml @@ -2,5 +2,8 @@ test_pkg_name_not_in_spdx + 8.4.2 + even more suggestive description + BSD diff --git a/test/_test_data/test_pkg_no_file_attribute/package.xml b/test/_test_data/test_pkg_no_file_attribute/package.xml index 9d26842..cb88138 100644 --- a/test/_test_data/test_pkg_no_file_attribute/package.xml +++ b/test/_test_data/test_pkg_no_file_attribute/package.xml @@ -2,5 +2,8 @@ test_pkg_no_file_attribute + 8.4.2 + even more suggestive description + BSD-3-Clause diff --git a/test/_test_data/test_pkg_no_license/package.xml b/test/_test_data/test_pkg_no_license/package.xml index a725822..43f1f5c 100644 --- a/test/_test_data/test_pkg_no_license/package.xml +++ b/test/_test_data/test_pkg_no_license/package.xml @@ -2,5 +2,8 @@ test_pkg_no_license + 8.4.2 + even more suggestive description + diff --git a/test/_test_data/test_pkg_no_license_file/package.xml b/test/_test_data/test_pkg_no_license_file/package.xml index 14087ef..ded1b9c 100644 --- a/test/_test_data/test_pkg_no_license_file/package.xml +++ b/test/_test_data/test_pkg_no_license_file/package.xml @@ -2,5 +2,8 @@ test_pkg_no_license_file + 8.4.2 + even more suggestive description + BSD-3-Clause diff --git a/test/_test_data/test_pkg_one_correct_one_license_file_missing/package.xml b/test/_test_data/test_pkg_one_correct_one_license_file_missing/package.xml index 4d3351e..1d7200c 100644 --- a/test/_test_data/test_pkg_one_correct_one_license_file_missing/package.xml +++ b/test/_test_data/test_pkg_one_correct_one_license_file_missing/package.xml @@ -2,6 +2,9 @@ test_pkg_one_correct_one_license_file_missing + 8.4.2 + even more suggestive description + BSD-3-Clause not-in-spdx-license diff --git a/test/_test_data/test_pkg_spdx_tag/package.xml b/test/_test_data/test_pkg_spdx_tag/package.xml index 120799a..84110aa 100644 --- a/test/_test_data/test_pkg_spdx_tag/package.xml +++ b/test/_test_data/test_pkg_spdx_tag/package.xml @@ -2,6 +2,9 @@ test_pkg_spdx_tag + 8.4.2 + even more suggestive description + AFL-2.0 diff --git a/test/_test_data/test_pkg_tag_not_spdx/package.xml b/test/_test_data/test_pkg_tag_not_spdx/package.xml index 45efa83..285d6cb 100644 --- a/test/_test_data/test_pkg_tag_not_spdx/package.xml +++ b/test/_test_data/test_pkg_tag_not_spdx/package.xml @@ -2,6 +2,9 @@ test_pkg_tag_not_spdx + 8.4.2 + even more suggestive description + AFL diff --git a/test/_test_data/test_pkg_too_many_license_files/package.xml b/test/_test_data/test_pkg_too_many_license_files/package.xml index 7524c26..fac87b6 100644 --- a/test/_test_data/test_pkg_too_many_license_files/package.xml +++ b/test/_test_data/test_pkg_too_many_license_files/package.xml @@ -2,5 +2,8 @@ test_pkg_too_many_license_files + 8.4.2 + even more suggestive description + AFL-2.0 diff --git a/test/_test_data/test_pkg_unknown_license/package.xml b/test/_test_data/test_pkg_unknown_license/package.xml index f792424..00a6443 100644 --- a/test/_test_data/test_pkg_unknown_license/package.xml +++ b/test/_test_data/test_pkg_unknown_license/package.xml @@ -2,5 +2,8 @@ test_pkg_unknown_license + 1.0.0 + description of a description + my own fancy license 1.0 diff --git a/test/_test_data/test_pkg_unknown_license_missing_file/package.xml b/test/_test_data/test_pkg_unknown_license_missing_file/package.xml index 7fcbc95..c4ca161 100644 --- a/test/_test_data/test_pkg_unknown_license_missing_file/package.xml +++ b/test/_test_data/test_pkg_unknown_license_missing_file/package.xml @@ -2,5 +2,8 @@ test_pkg_unknown_license_missing_file + 1.0.0 + description of a description + my own fancy license 1.0 diff --git a/test/_test_data/test_pkg_with_license_and_file/package.xml b/test/_test_data/test_pkg_with_license_and_file/package.xml index 8040115..e7a6f05 100644 --- a/test/_test_data/test_pkg_with_license_and_file/package.xml +++ b/test/_test_data/test_pkg_with_license_and_file/package.xml @@ -2,5 +2,8 @@ test_pkg_with_license_and_file + 1.0.0 + description of a description + BSD-3-Clause diff --git a/test/_test_data/test_pkg_with_multiple_licenses_no_source_files_tag/package.xml b/test/_test_data/test_pkg_with_multiple_licenses_no_source_files_tag/package.xml index 2f6a519..862d869 100644 --- a/test/_test_data/test_pkg_with_multiple_licenses_no_source_files_tag/package.xml +++ b/test/_test_data/test_pkg_with_multiple_licenses_no_source_files_tag/package.xml @@ -2,6 +2,9 @@ test_pkg_with_multiple_licenses_no_source_files_tag + 1.0.0 + description of a description + BSD-3-Clause MPL-1.0 diff --git a/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml b/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml index 7b95237..1127101 100644 --- a/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml +++ b/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml @@ -2,6 +2,9 @@ test_pkg_with_multiple_licenses_one_referenced_incorrect + 1.0.0 + description of a description + BSD MPL-1.0 diff --git a/test/_test_data/test_pkg_wrong_license_file/package.xml b/test/_test_data/test_pkg_wrong_license_file/package.xml index 5439e59..dbfd487 100644 --- a/test/_test_data/test_pkg_wrong_license_file/package.xml +++ b/test/_test_data/test_pkg_wrong_license_file/package.xml @@ -2,5 +2,8 @@ test_pkg_wrong_license_file + 1.0.0 + description of a description + s BSD-3-Clause diff --git a/test/_test_data/test_repo_bsd3/pkg_with_bsd3_a/package.xml b/test/_test_data/test_repo_bsd3/pkg_with_bsd3_a/package.xml index 64dc5b1..60a6212 100644 --- a/test/_test_data/test_repo_bsd3/pkg_with_bsd3_a/package.xml +++ b/test/_test_data/test_repo_bsd3/pkg_with_bsd3_a/package.xml @@ -2,5 +2,8 @@ pkg_with_bsd3_a + 1.0.0 + description of a description + BSD-3-Clause diff --git a/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml b/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml index 4789f09..44b42b6 100644 --- a/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml +++ b/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml @@ -2,5 +2,8 @@ pkg_with_bsd3_b + 1.0.0 + description of a description + s BSD-3-Clause diff --git a/test/_test_data/test_repo_bsd3/test_repo_bsd3/package.xml b/test/_test_data/test_repo_bsd3/test_repo_bsd3/package.xml index 76e29b5..14dc50d 100644 --- a/test/_test_data/test_repo_bsd3/test_repo_bsd3/package.xml +++ b/test/_test_data/test_repo_bsd3/test_repo_bsd3/package.xml @@ -3,5 +3,8 @@ test_repo_bsd3 + 1.0.0 + description of a description + BSD-3-Clause diff --git a/test/_test_data/test_repo_mit/pkg_with_mit_a/package.xml b/test/_test_data/test_repo_mit/pkg_with_mit_a/package.xml index 4e1a46d..c641526 100644 --- a/test/_test_data/test_repo_mit/pkg_with_mit_a/package.xml +++ b/test/_test_data/test_repo_mit/pkg_with_mit_a/package.xml @@ -2,5 +2,8 @@ pkg_with_mit_a + 1.0.0 + description of a description + MIT diff --git a/test/_test_data/test_repo_mit/pkg_with_mit_b/package.xml b/test/_test_data/test_repo_mit/pkg_with_mit_b/package.xml index 5bdaf76..48adb01 100644 --- a/test/_test_data/test_repo_mit/pkg_with_mit_b/package.xml +++ b/test/_test_data/test_repo_mit/pkg_with_mit_b/package.xml @@ -2,5 +2,8 @@ pkg_with_mit_b + 1.0.0 + description of a description + MIT From dbef93610f8682f3c030a9d64833f6f753a3d054 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 25 Nov 2024 12:33:54 +0100 Subject: [PATCH 14/21] changed test data with source-file attribute to version 4 Signed-off-by: Anton Utz --- test/_test_data/test_pkg_both_tags_not_spdx/package.xml | 4 ++-- test/_test_data/test_pkg_has_code_disjoint/package.xml | 4 ++-- .../package.xml | 2 +- test/_test_data/test_pkg_ignore_readme_contents/package.xml | 4 ++-- .../package.xml | 4 ++-- test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/_test_data/test_pkg_both_tags_not_spdx/package.xml b/test/_test_data/test_pkg_both_tags_not_spdx/package.xml index b181d2f..18d5628 100644 --- a/test/_test_data/test_pkg_both_tags_not_spdx/package.xml +++ b/test/_test_data/test_pkg_both_tags_not_spdx/package.xml @@ -1,6 +1,6 @@ - - + + test_pkg_both_tags_not_spdx 1.2.3 describing words diff --git a/test/_test_data/test_pkg_has_code_disjoint/package.xml b/test/_test_data/test_pkg_has_code_disjoint/package.xml index 232b8c7..c63cec3 100644 --- a/test/_test_data/test_pkg_has_code_disjoint/package.xml +++ b/test/_test_data/test_pkg_has_code_disjoint/package.xml @@ -1,6 +1,6 @@ - - + + test_pkg_has_code_disjoint 1.2.3 describing words diff --git a/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml b/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml index 95c06b8..ef095fd 100644 --- a/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml +++ b/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml @@ -1,5 +1,5 @@ - + test_pkg_has_code_of_different_license_and_tag 1.2.3 diff --git a/test/_test_data/test_pkg_ignore_readme_contents/package.xml b/test/_test_data/test_pkg_ignore_readme_contents/package.xml index 348cd78..fc39525 100644 --- a/test/_test_data/test_pkg_ignore_readme_contents/package.xml +++ b/test/_test_data/test_pkg_ignore_readme_contents/package.xml @@ -1,6 +1,6 @@ - - + + test_pkg_ignore_readme_contents 3.2.1 very suggestive description diff --git a/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml b/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml index 1127101..ca12aa0 100644 --- a/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml +++ b/test/_test_data/test_pkg_with_multiple_licenses_one_referenced_incorrect/package.xml @@ -1,6 +1,6 @@ - - + + test_pkg_with_multiple_licenses_one_referenced_incorrect 1.0.0 description of a description diff --git a/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml b/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml index 44b42b6..ca1fe40 100644 --- a/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml +++ b/test/_test_data/test_repo_bsd3/pkg_with_bsd3_b/package.xml @@ -4,6 +4,6 @@ pkg_with_bsd3_b 1.0.0 description of a description - s + BSD-3-Clause From cd379e5ec6bf7111bf311c33e5ffda2c5b540dd9 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Thu, 28 Nov 2024 09:10:08 +0100 Subject: [PATCH 15/21] added case for package.xml version 4 Signed-off-by: Anton Utz --- src/ros_license_toolkit/license_checks/schema_check.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py index 56af0ad..685f946 100644 --- a/src/ros_license_toolkit/license_checks/schema_check.py +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -50,6 +50,12 @@ def _check(self, package): elif version == 3: reason = "package.xml contains errors: " +\ f"{self.xml_schema_3.error_log.last_error.message}" + # Temporary workaround for not implemented version 4 + elif version == 4: + reason = "couldn't check package.xml scheme. Version 4 is " +\ + "not available right now" + self._warning(reason) + return elif version == -1: reason = "package.xml does not contain correct package " +\ "format number. Please use a real version. " +\ @@ -75,6 +81,8 @@ def validate(self, pck_xml_path: str) -> Tuple[int, bool]: elif version == '1': version = 1 status_check = self.xml_schema_1.validate(xml_doc_parsed) + elif version == '4': # Work around: 4 not available right now + version = 4 else: version = -1 return (version, status_check) From 9330eb2876693d88e2e5efb90730986915c77712 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Thu, 28 Nov 2024 09:16:38 +0100 Subject: [PATCH 16/21] removed todo Signed-off-by: Anton Utz --- src/ros_license_toolkit/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index 009e822..f5a04a1 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -125,7 +125,7 @@ def main(args: Optional[Sequence[str]] = None) -> int: rll_print(f"All packages:\n {WARNING_STR}", Verbosity.QUIET) return os.EX_OK rll_print(f"All packages:\n {FAILURE_STR}", Verbosity.QUIET) - return os.EX_DATAERR # TODO change this to critical error + return os.EX_DATAERR def generate_copyright_file(packages, rll_print): From 3ece02364d175a57008d46f1bccf4d79256e92a4 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Thu, 28 Nov 2024 09:48:13 +0100 Subject: [PATCH 17/21] adapted wrong test Signed-off-by: Anton Utz --- .../test_pkg_has_code_of_different_license_and_tag/package.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml b/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml index ef095fd..e3e42b5 100644 --- a/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml +++ b/test/_test_data/test_pkg_has_code_of_different_license_and_tag/package.xml @@ -1,6 +1,6 @@ - + test_pkg_has_code_of_different_license_and_tag 1.2.3 describing words From bc4fabf0d10e14057d6dbd05b32b4474ab164a2a Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Fri, 29 Nov 2024 14:22:59 +0100 Subject: [PATCH 18/21] moved version checking and xml tree conversion to package.py Signed-off-by: Anton Utz --- .../license_checks/schema_check.py | 61 ++++++++----------- src/ros_license_toolkit/package.py | 33 ++++++++++ 2 files changed, 59 insertions(+), 35 deletions(-) diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py index 685f946..7f9232d 100644 --- a/src/ros_license_toolkit/license_checks/schema_check.py +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -21,6 +21,7 @@ from lxml import etree from ros_license_toolkit.checks import Check +from ros_license_toolkit.package import Package class SchemaCheck(Check): @@ -34,56 +35,46 @@ def __init__(self): xml_schema_1_parsed = etree.parse('./schemas/package_format1.xsd') self.xml_schema_1 = etree.XMLSchema(xml_schema_1_parsed) - def _check(self, package): - version, status = self.validate(package.abspath + "/package.xml") + def _check(self, package: Package): + status, message = self.validate(package) + version: int = package.package_xml_format_ver if status: self._success(f"Detected package.xml version {version}, " "validation of scheme successful.") else: - reason = '' - if version == 1: - reason = "package.xml contains errors: " +\ - f"{self.xml_schema_1.error_log.last_error.message}" - elif version == 2: - reason = "package.xml contains errors: " +\ - f"{self.xml_schema_2.error_log.last_error.message}" - elif version == 3: - reason = "package.xml contains errors: " +\ - f"{self.xml_schema_3.error_log.last_error.message}" # Temporary workaround for not implemented version 4 - elif version == 4: + if version == 4: reason = "couldn't check package.xml scheme. Version 4 is " +\ "not available right now" self._warning(reason) return - elif version == -1: + + if message != '': + reason = f"package.xml contains errors: {message}" + else: reason = "package.xml does not contain correct package " +\ "format number. Please use a real version. " +\ "(e.g. )" self._failed(reason) - def validate(self, pck_xml_path: str) -> Tuple[int, bool]: + def validate(self, package: Package) -> Tuple[bool, str]: """This is validating package.xml file from given path. Automatically detects version number and validates it with corresponding scheme, e.g. format 3. If everything is correct, returns format number, else -1.""" - xml_doc_parsed = etree.parse(pck_xml_path) - for element in xml_doc_parsed.getiterator(): - if element.tag == 'package' and 'format' in element.attrib: - version = element.attrib['format'] - status_check = False - if version == '3': - version = 3 - status_check = self.xml_schema_3.validate(xml_doc_parsed) - elif version == '2': - version = 2 - status_check = self.xml_schema_2.validate(xml_doc_parsed) - elif version == '1': - version = 1 - status_check = self.xml_schema_1.validate(xml_doc_parsed) - elif version == '4': # Work around: 4 not available right now - version = 4 - else: - version = -1 - return (version, status_check) - return (-1, False) + version = package.package_xml_format_ver + xml_doc = package.parsed_package_xml + validation_schema: etree.XMLSchema = False + if version == 3: + validation_schema = self.xml_schema_3 + elif version == 2: + validation_schema = self.xml_schema_2 + elif version == 1: + validation_schema = self.xml_schema_1 + if validation_schema: + status_check = validation_schema.validate(xml_doc) + message = '' + if not status_check: + message = validation_schema.error_log.last_error_message + return status_check, message + return False, message diff --git a/src/ros_license_toolkit/package.py b/src/ros_license_toolkit/package.py index 61d741d..daeae20 100644 --- a/src/ros_license_toolkit/package.py +++ b/src/ros_license_toolkit/package.py @@ -22,6 +22,7 @@ import xml.etree.ElementTree as ET from typing import Any, Dict, List, Optional +from lxml import etree from rospkg import RosPack, list_by_path from rospkg.common import PACKAGE_FILE from scancode.api import get_licenses @@ -83,6 +84,12 @@ def __init__(self, path: str, repo: Optional[Repo] = None): # All ignored files and folders self._ignored_content: List[str] = get_ignored_content(self.abspath) + # The package.xml file, parsed as etree + self._parsed_package_xml: etree = None + + # The package.xml version as set in + self._package_xml_format_ver: int = 0 + def _get_path_relative_to_pkg(self, path: str) -> str: """Get path relative to pkg root""" return os.path.relpath(path, self.abspath) @@ -230,6 +237,32 @@ def license_tags(self) -> Dict[str, LicenseTag]: return self._license_tags + @property + def package_xml_format_ver(self) -> int: + """Returns version of package.xml format as seen in + . If Version is not valid, -1 is returned.""" + if self._package_xml_format_ver is None: + root = self.parsed_package_xml.getroot() + if root.tag == 'package': + if 'format' in root.attrib: + version = root.attrib['format'] + try: + self._package_xml_format_ver = int(version) + except ValueError: + self._package_xml_format_ver = -1 + return self._package_xml_format_ver + self._package_xml_format_ver = -1 + return self._package_xml_format_ver + + @property + def parsed_package_xml(self) -> etree: + """Returns the package.xml content parsed as etree.""" + if self._parsed_package_xml == 0: + path = self.abspath + "/package.xml" + assert os.path.exists(path), f'Path {path} does not exist.' + self._parsed_package_xml = etree.parse(path) + return self._parsed_package_xml + @property def repo_url(self) -> Optional[str]: """Get the url of the repo this package is in if the package is in a From 971663954cc45e2c8366eecf2a0125b6792d76d5 Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 2 Dec 2024 11:51:57 +0100 Subject: [PATCH 19/21] refactor of schema check Signed-off-by: Anton Utz --- .../license_checks/schema_check.py | 33 +++++++------------ src/ros_license_toolkit/package.py | 13 +++++--- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py index 7f9232d..557a2ca 100644 --- a/src/ros_license_toolkit/license_checks/schema_check.py +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -16,7 +16,7 @@ """This Module contains SchemaCheck, which implements Check.""" -from typing import Tuple +from typing import Dict, Tuple from lxml import etree @@ -28,12 +28,10 @@ class SchemaCheck(Check): """This checks the xml scheme and returns the version number.""" def __init__(self): super().__init__() - xml_schema_3_parsed = etree.parse('./schemas/package_format3.xsd') - self.xml_schema_3 = etree.XMLSchema(xml_schema_3_parsed) - xml_schema_2_parsed = etree.parse('./schemas/package_format3.xsd') - self.xml_schema_2 = etree.XMLSchema(xml_schema_2_parsed) - xml_schema_1_parsed = etree.parse('./schemas/package_format1.xsd') - self.xml_schema_1 = etree.XMLSchema(xml_schema_1_parsed) + self.xml_schemas: Dict[int, etree] = {} + for i in range(1, 4): + xml_schema_parsed = etree.parse(f'./schemas/package_format{i}.xsd') + self.xml_schemas[i] = etree.XMLSchema(xml_schema_parsed) def _check(self, package: Package): status, message = self.validate(package) @@ -63,18 +61,11 @@ def validate(self, package: Package) -> Tuple[bool, str]: it with corresponding scheme, e.g. format 3. If everything is correct, returns format number, else -1.""" version = package.package_xml_format_ver - xml_doc = package.parsed_package_xml - validation_schema: etree.XMLSchema = False - if version == 3: - validation_schema = self.xml_schema_3 - elif version == 2: - validation_schema = self.xml_schema_2 - elif version == 1: - validation_schema = self.xml_schema_1 - if validation_schema: - status_check = validation_schema.validate(xml_doc) - message = '' - if not status_check: - message = validation_schema.error_log.last_error_message - return status_check, message + message = '' + if version in self.xml_schemas: + schema = self.xml_schemas[version] + result = schema.validate(package.parsed_package_xml) + if not result: + message = schema.error_log.last_error_message + return result, message return False, message diff --git a/src/ros_license_toolkit/package.py b/src/ros_license_toolkit/package.py index daeae20..febd180 100644 --- a/src/ros_license_toolkit/package.py +++ b/src/ros_license_toolkit/package.py @@ -33,6 +33,8 @@ from ros_license_toolkit.license_tag import LicenseTag from ros_license_toolkit.repo import NotARepoError, Repo +INVALID = -1 + class PackageException(Exception): """Exception raised when a package is invalid.""" @@ -240,8 +242,9 @@ def license_tags(self) -> Dict[str, LicenseTag]: @property def package_xml_format_ver(self) -> int: """Returns version of package.xml format as seen in - . If Version is not valid, -1 is returned.""" - if self._package_xml_format_ver is None: + . If Version is not valid, + INVALID (-1) is returned.""" + if self._package_xml_format_ver == 0: root = self.parsed_package_xml.getroot() if root.tag == 'package': if 'format' in root.attrib: @@ -249,15 +252,15 @@ def package_xml_format_ver(self) -> int: try: self._package_xml_format_ver = int(version) except ValueError: - self._package_xml_format_ver = -1 + self._package_xml_format_ver = INVALID return self._package_xml_format_ver - self._package_xml_format_ver = -1 + self._package_xml_format_ver = INVALID return self._package_xml_format_ver @property def parsed_package_xml(self) -> etree: """Returns the package.xml content parsed as etree.""" - if self._parsed_package_xml == 0: + if self._parsed_package_xml is None: path = self.abspath + "/package.xml" assert os.path.exists(path), f'Path {path} does not exist.' self._parsed_package_xml = etree.parse(path) From bab6691a54824a985e115968ee9d1d967f88cf3d Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 2 Dec 2024 14:00:01 +0100 Subject: [PATCH 20/21] fixed bug Signed-off-by: Anton Utz --- src/ros_license_toolkit/license_checks/schema_check.py | 2 +- src/ros_license_toolkit/package.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py index 557a2ca..99a6792 100644 --- a/src/ros_license_toolkit/license_checks/schema_check.py +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -66,6 +66,6 @@ def validate(self, package: Package) -> Tuple[bool, str]: schema = self.xml_schemas[version] result = schema.validate(package.parsed_package_xml) if not result: - message = schema.error_log.last_error_message + message = schema.error_log.last_error return result, message return False, message diff --git a/src/ros_license_toolkit/package.py b/src/ros_license_toolkit/package.py index febd180..787110c 100644 --- a/src/ros_license_toolkit/package.py +++ b/src/ros_license_toolkit/package.py @@ -254,7 +254,7 @@ def package_xml_format_ver(self) -> int: except ValueError: self._package_xml_format_ver = INVALID return self._package_xml_format_ver - self._package_xml_format_ver = INVALID + self._package_xml_format_ver = INVALID return self._package_xml_format_ver @property From 55962ce1883bc528601005c8590b000908e0c85e Mon Sep 17 00:00:00 2001 From: Anton Utz Date: Mon, 9 Dec 2024 13:32:02 +0100 Subject: [PATCH 21/21] implemented suggestions Signed-off-by: Anton Utz --- src/ros_license_toolkit/checks.py | 1 - .../license_checks/license_in_code_check.py | 23 +++++++++---------- .../license_checks/schema_check.py | 2 ++ src/ros_license_toolkit/main.py | 7 +----- .../test_pkg_wrong_license_file/package.xml | 2 +- 5 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/ros_license_toolkit/checks.py b/src/ros_license_toolkit/checks.py index 3036d84..1c542e9 100644 --- a/src/ros_license_toolkit/checks.py +++ b/src/ros_license_toolkit/checks.py @@ -16,7 +16,6 @@ """This module contains the checks for the linter.""" - from enum import IntEnum from ros_license_toolkit.package import Package, PackageException diff --git a/src/ros_license_toolkit/license_checks/license_in_code_check.py b/src/ros_license_toolkit/license_checks/license_in_code_check.py index f73bc39..c0fccaa 100644 --- a/src/ros_license_toolkit/license_checks/license_in_code_check.py +++ b/src/ros_license_toolkit/license_checks/license_in_code_check.py @@ -30,11 +30,10 @@ class LicensesInCodeCheck(Check): def __init__(self: 'LicensesInCodeCheck'): Check.__init__(self) - # self.package_xml_vers = package_xml_version self.declared_licenses: Dict[str, LicenseTag] = {} self.files_with_uncovered_licenses: Dict[str, List[str]] = {} self.files_not_matched_by_any_license_tag: Dict[str, List[str]] = {} - self.files_with_inofficial_tag: Dict[str, List[str]] = {} + self.files_with_unofficial_tag: Dict[str, List[str]] = {} def _check(self, package: Package): if len(package.license_tags) == 0: @@ -56,18 +55,18 @@ def _check_license_files(self, package: Package) -> None: licenses = found_licenses_str.split(' AND ') for license_str in licenses: if license_str not in self.declared_licenses: - # this license has an inofficial tag - inofficial_licenses = { + # this license has an unofficial tag + unofficial_licenses = { lic_tag.id_from_license_text: key for key, lic_tag in package.license_tags.items() if lic_tag.id_from_license_text != ''} - if license_str in inofficial_licenses.keys(): - if fname not in self.files_with_inofficial_tag: - self.files_with_inofficial_tag[fname] = [] - self.files_with_inofficial_tag[fname].append( + if license_str in unofficial_licenses.keys(): + if fname not in self.files_with_unofficial_tag: + self.files_with_unofficial_tag[fname] = [] + self.files_with_unofficial_tag[fname].append( license_str) - self.files_with_inofficial_tag[fname].append( - inofficial_licenses[license_str]) + self.files_with_unofficial_tag[fname].append( + unofficial_licenses[license_str]) continue # this license is not declared by any license tag if fname not in self.files_with_uncovered_licenses: @@ -98,13 +97,13 @@ def _evaluate_result(self, package: Package) -> None: self.files_not_matched_by_any_license_tag, package, ) - elif self.files_with_inofficial_tag: + elif self.files_with_unofficial_tag: info_str = '' info_str += 'For the following files, please change the ' +\ 'License Tag in the package file to SPDX format:\n' +\ '\n'.join( [f" '{x[0]}' is of {x[1][0]} but its Tag is {x[1][1]}." - for x in self.files_with_inofficial_tag.items()]) + for x in self.files_with_unofficial_tag.items()]) self._warning(info_str) elif len(self.files_not_matched_by_any_license_tag) > 0: info_str = '' diff --git a/src/ros_license_toolkit/license_checks/schema_check.py b/src/ros_license_toolkit/license_checks/schema_check.py index 99a6792..b4277a2 100644 --- a/src/ros_license_toolkit/license_checks/schema_check.py +++ b/src/ros_license_toolkit/license_checks/schema_check.py @@ -34,6 +34,8 @@ def __init__(self): self.xml_schemas[i] = etree.XMLSchema(xml_schema_parsed) def _check(self, package: Package): + """Checks via schema validation if the package.xml has the correct + format, dependant on the version it is in.""" status, message = self.validate(package) version: int = package.package_xml_format_ver if status: diff --git a/src/ros_license_toolkit/main.py b/src/ros_license_toolkit/main.py index f5a04a1..9bac322 100644 --- a/src/ros_license_toolkit/main.py +++ b/src/ros_license_toolkit/main.py @@ -152,12 +152,8 @@ def process_one_pkg(rll_print, package): rll_print( f'git hash of ({package.repo.get_path()}):' f' {package.repo.get_hash()}') - # check for scheme here, then insert scheme version in constructor - schema_check = SchemaCheck() - schema_check.check(package) - rll_print(schema_check) - rll_print(schema_check.verbose(), Verbosity.VERBOSE) checks_to_perform = [ + SchemaCheck(), LicenseTagExistsCheck(), LicenseTagIsInSpdxListCheck(), LicenseTextExistsCheck(), @@ -169,7 +165,6 @@ def process_one_pkg(rll_print, package): rll_print(check) rll_print(check.verbose(), Verbosity.VERBOSE) - checks_to_perform.append(schema_check) rll_print(minor_sep()) # Every check is successful, no warning if max(check.status for check in checks_to_perform) == Status.SUCCESS: diff --git a/test/_test_data/test_pkg_wrong_license_file/package.xml b/test/_test_data/test_pkg_wrong_license_file/package.xml index dbfd487..9ddcde1 100644 --- a/test/_test_data/test_pkg_wrong_license_file/package.xml +++ b/test/_test_data/test_pkg_wrong_license_file/package.xml @@ -4,6 +4,6 @@ test_pkg_wrong_license_file 1.0.0 description of a description - s + BSD-3-Clause