From a4131b94475c8c023304993678b6f7d0e347fe0c Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 21 Jan 2025 19:56:12 -0800 Subject: [PATCH] Silence WITH_PERMISSION_ONLY warnings Currently using a license in the WITH_PERMISSION_ONLY list will create a warning. By making an ALL_LICENSE list including this list and also allowing it when checking for open source compatible licenses, it will no longer create warnings. This will enable us to change the current warnings into errors. Bug: b/388620886 Change-Id: I883a3d3c825f0f1903b62d0b93810218b1f42bb9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6188501 Commit-Queue: Jordan Brown Reviewed-by: Rachael Newitt --- metadata/fields/custom/license.py | 11 +++++----- metadata/fields/custom/license_allowlist.py | 1 + .../README.chromium.test.restricted-license | 22 +++++++++++++++++++ metadata/tests/dependency_metadata_test.py | 13 +++++++++-- metadata/tests/validate_test.py | 18 +++++++++++++++ 5 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 metadata/tests/data/README.chromium.test.restricted-license diff --git a/metadata/fields/custom/license.py b/metadata/fields/custom/license.py index 5661e92af..ae60734b2 100644 --- a/metadata/fields/custom/license.py +++ b/metadata/fields/custom/license.py @@ -21,7 +21,7 @@ sys.path.insert(0, _ROOT_DIR) import metadata.fields.field_types as field_types import metadata.fields.util as util import metadata.validation_result as vr -from metadata.fields.custom.license_allowlist import ALLOWED_LICENSES, ALLOWED_OPEN_SOURCE_LICENSES +from metadata.fields.custom.license_allowlist import ALLOWED_LICENSES, ALLOWED_OPEN_SOURCE_LICENSES, ALL_LICENSES, WITH_PERMISSION_ONLY def process_license_value(value: str, @@ -60,16 +60,17 @@ def process_license_value(value: str, def is_license_valid(value: str) -> bool: - """Returns whether the value is in the allowlist for license - types. + """Returns whether the value is a valid license type. """ - # The open source allowlist is the most permissive. - return value in ALLOWED_OPEN_SOURCE_LICENSES + return value in ALL_LICENSES def is_license_allowlisted(value: str, is_open_source_project: bool = False) -> bool: """Returns whether the value is in the allowlist for license types. """ + # Restricted licenses are not enforced by presubmits, see b/388620886 😢. + if value in WITH_PERMISSION_ONLY: + return True if is_open_source_project: return value in ALLOWED_OPEN_SOURCE_LICENSES return value in ALLOWED_LICENSES diff --git a/metadata/fields/custom/license_allowlist.py b/metadata/fields/custom/license_allowlist.py index 43e7c0a51..629dd9da4 100644 --- a/metadata/fields/custom/license_allowlist.py +++ b/metadata/fields/custom/license_allowlist.py @@ -199,3 +199,4 @@ WITH_PERMISSION_ONLY = frozenset([ ALLOWED_LICENSES = ALLOWED_SPDX_LICENSES | EXTENDED_LICENSE_CLASSIFIERS ALLOWED_OPEN_SOURCE_LICENSES = ALLOWED_LICENSES | OPEN_SOURCE_SPDX_LICENSES +ALL_LICENSES = ALLOWED_OPEN_SOURCE_LICENSES | WITH_PERMISSION_ONLY \ No newline at end of file diff --git a/metadata/tests/data/README.chromium.test.restricted-license b/metadata/tests/data/README.chromium.test.restricted-license new file mode 100644 index 000000000..b032509e0 --- /dev/null +++ b/metadata/tests/data/README.chromium.test.restricted-license @@ -0,0 +1,22 @@ +Name: Dependency using a restricted license +Short Name: restricted-license-dependency +URL: https://www.example.com/metadata, + https://www.example.com/parser +Unknown Field: Should be extracted into a field, because the preceding URL + field is structured, thus terminated by another field-like + line, even if the field name isn't well known to us. +Version: 1.0.12 +Date: 2020-12-03 +License: GPL-2.0 +License File: LICENSE +Security Critical: yes +Shipped: yes +CPEPrefix: unknown +This line should be ignored because CPEPrefix is a one-liner field. +Description: +This is a dependency that uses a restricted license. + +Local Modifications: +None. + +Additional paragraphs describing the package, which are not part of Description. \ No newline at end of file diff --git a/metadata/tests/dependency_metadata_test.py b/metadata/tests/dependency_metadata_test.py index bec793315..546d4f1e7 100644 --- a/metadata/tests/dependency_metadata_test.py +++ b/metadata/tests/dependency_metadata_test.py @@ -372,21 +372,30 @@ class DependencyValidationTest(unittest.TestCase): def test_all_licenses_allowlisted(self): """Test that a single allowlisted license returns True.""" dependency = dm.DependencyMetadata() - # "MPL-2.0" is a reciprocal license, i.e. only allowed in open source projects. self.assertTrue(dependency.all_licenses_allowlisted("MIT", False)) + self.assertTrue(dependency.all_licenses_allowlisted("MIT, GPL-2.0", False)) self.assertTrue(dependency.all_licenses_allowlisted("MIT, Apache-2.0", False)) - self.assertTrue(dependency.all_licenses_allowlisted("MPL-2.0", True)) self.assertFalse(dependency.all_licenses_allowlisted("InvalidLicense", False)) self.assertFalse(dependency.all_licenses_allowlisted("MIT, InvalidLicense", False)) self.assertFalse(dependency.all_licenses_allowlisted("", False)) + + # "MPL-2.0" is a reciprocal license, i.e. only allowed in open source projects. + self.assertTrue(dependency.all_licenses_allowlisted("MPL-2.0", True)) self.assertFalse(dependency.all_licenses_allowlisted("MPL-2.0", False)) + # Restricted licenses are treated the same as other license types, until + # the exception and enforcement is resourced. + self.assertTrue(dependency.all_licenses_allowlisted("GPL-2.0", False)) + self.assertTrue(dependency.all_licenses_allowlisted("GPL-2.0", True)) + self.assertFalse(dependency.all_licenses_allowlisted("MPL-2.0, GPL-2.0", False)) + def test_only_open_source_licenses(self): """Test that only open source licenses are returned.""" dependency = dm.DependencyMetadata() self.assertEqual(dependency.only_open_source_licenses(""), []) self.assertEqual(dependency.only_open_source_licenses("MIT"), []) + self.assertEqual(dependency.only_open_source_licenses("GPL-2.0"), []) self.assertEqual(dependency.only_open_source_licenses("MPL-2.0"), ["MPL-2.0"]) result = dependency.only_open_source_licenses("MIT, MPL-2.0") self.assertEqual(result, ["MPL-2.0"]) diff --git a/metadata/tests/validate_test.py b/metadata/tests/validate_test.py index ef1c39d06..3ab622382 100644 --- a/metadata/tests/validate_test.py +++ b/metadata/tests/validate_test.py @@ -255,5 +255,23 @@ class ValidateReciprocalLicenseTest(unittest.TestCase): self.assertEqual(len(license_errors), 0, "Should not create an error when a reciprocal license is used in an open source project") + +class ValidateRestrictedLicenseTest(unittest.TestCase): + """Tests that validate_content handles allowing restricted licenses correctly.""" + + # TODO(b/388620886): Warn when changing to a restricted license. + def test_restricted_licenses(self): + # Test content with a restricted license (GPL-2.0). + restricted_license_metadata_filepath = os.path.join(_THIS_DIR, "data", + "README.chromium.test.restricted-license") + results = metadata.validate.validate_content( + content=gclient_utils.FileRead(restricted_license_metadata_filepath), + source_file_dir=_SOURCE_FILE_DIR, + repo_root_dir=_THIS_DIR, + is_open_source_project=False + ) + + self.assertEqual(len(results), 0, "Should not create an error when a restricted license is used") + if __name__ == "__main__": unittest.main()