diff --git a/metadata/fields/custom/license.py b/metadata/fields/custom/license.py index 68db772af..bd3e2d131 100644 --- a/metadata/fields/custom/license.py +++ b/metadata/fields/custom/license.py @@ -20,13 +20,6 @@ import metadata.fields.util as util import metadata.validation_result as vr from metadata.fields.custom.license_allowlist import ALLOWED_SPDX_LICENSES -_PATTERN_VERBOSE_DELIMITER = re.compile(r" and | or | / ") - -# Split on the canonical delimiter, or any of the non-canonical delimiters. -_PATTERN_SPLIT_LICENSE = re.compile("{}|{}".format( - _PATTERN_VERBOSE_DELIMITER.pattern, - field_types.MetadataField.VALUE_DELIMITER)) - def process_license_value(value: str, atomic_delimiter: str) -> List[Tuple[str, bool]]: @@ -50,18 +43,12 @@ def process_license_value(value: str, return [(value, True)] breakdown = [] - if re.search(_PATTERN_VERBOSE_DELIMITER, value): - # Split using the verbose delimiters. - for component in re.split(_PATTERN_VERBOSE_DELIMITER, value): - breakdown.extend( - process_license_value(component.strip(), atomic_delimiter)) - else: - # Split using the standard value delimiter. This results in - # atomic values; there is no further splitting possible. - for atomic_value in value.split(atomic_delimiter): - atomic_value = atomic_value.strip() - breakdown.append( - (atomic_value, is_license_allowlisted(atomic_value))) + # Split using the standard value delimiter. This results in + # atomic values; there is no further splitting possible. + for atomic_value in value.split(atomic_delimiter): + atomic_value = atomic_value.strip() + breakdown.append( + (atomic_value, is_license_allowlisted(atomic_value))) return breakdown @@ -74,30 +61,30 @@ def is_license_allowlisted(value: str) -> bool: class LicenseField(field_types.SingleLineTextField): - """Custom field for the package's license type(s). + """Custom field for the package's license type(s). e.g. Apache 2.0, MIT, BSD, Public Domain. """ - def __init__(self): - super().__init__(name="License") + def __init__(self): + super().__init__(name="License") - def validate(self, value: str) -> Optional[vr.ValidationResult]: - """Checks the given value consists of recognized license types. + def validate(self, value: str) -> Optional[vr.ValidationResult]: + """Checks the given value consists of recognized license types. Note: this field supports multiple values. """ - not_allowlisted = [] - licenses = process_license_value(value, + not_allowlisted = [] + licenses = process_license_value(value, atomic_delimiter=self.VALUE_DELIMITER) - for license, allowed in licenses: - if util.is_empty(license): - return vr.ValidationError( + for license, allowed in licenses: + if util.is_empty(license): + return vr.ValidationError( reason=f"{self._name} has an empty value.") - if not allowed: - not_allowlisted.append(license) + if not allowed: + not_allowlisted.append(license) - if not_allowlisted: - return vr.ValidationWarning( + if not_allowlisted: + return vr.ValidationWarning( reason=f"{self._name} has a license not in the allowlist." " (see https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:metadata/fields/custom/license_allowlist.py).", additional=[ @@ -105,12 +92,12 @@ class LicenseField(field_types.SingleLineTextField): f"{util.quoted(not_allowlisted)}.", ]) - return None + return None - def narrow_type(self, value: str) -> Optional[List[str]]: - if not value: - # Empty License field is equivalent to "not declared". - return None + def narrow_type(self, value: str) -> Optional[List[str]]: + if not value: + # Empty License field is equivalent to "not declared". + return None - parts = _PATTERN_SPLIT_LICENSE.split(value) - return list(filter(bool, map(lambda str: str.strip(), parts))) + parts = value.split(self.VALUE_DELIMITER) + return list(filter(bool, map(lambda str: str.strip(), parts))) diff --git a/metadata/tests/data/README.chromium.test.multi-invalid b/metadata/tests/data/README.chromium.test.multi-invalid index 5983054f8..d723c953f 100644 --- a/metadata/tests/data/README.chromium.test.multi-invalid +++ b/metadata/tests/data/README.chromium.test.multi-invalid @@ -4,7 +4,7 @@ URL: https://www.example.com/metadata, https://www.example.com/parser Version: 1.0.12 Date: 2020-12-03 -License: Apache-2.0 and MIT +License: Apache-2.0 and MIT and not_an_spdx License File: LICENSE Security Critical: yes Shipped: yes diff --git a/metadata/tests/data/README.chromium.test.multi-valid b/metadata/tests/data/README.chromium.test.multi-valid index 6eb8a1710..b0807474d 100644 --- a/metadata/tests/data/README.chromium.test.multi-valid +++ b/metadata/tests/data/README.chromium.test.multi-valid @@ -4,7 +4,7 @@ URL: https://www.example.com/metadata, https://www.example.com/parser Version: 1.0.12 Date: 2020-12-03 -License: Apache-2.0 and MIT +License: Apache-2.0, MIT License File: LICENSE Security Critical: yes Shipped: yes diff --git a/metadata/tests/fields_test.py b/metadata/tests/fields_test.py index 1cf8cc566..78a8ff323 100644 --- a/metadata/tests/fields_test.py +++ b/metadata/tests/fields_test.py @@ -115,17 +115,19 @@ class FieldValidationTest(unittest.TestCase): self._run_field_validation( field=known_fields.LICENSE, valid_values=[ - "Apache-2.0 / MIT", + "Apache-2.0 , MIT", "Apache-2.0", "BSD-2-Clause", "BSD-2-Clause-FreeBSD", "MIT", - "APSL-2.0 and MIT", + "APSL-2.0, MIT", + "APSL-2.0 ,MIT", ], - error_values=["", "\n", ",", "Apache 2.0 / MIT / "], + error_values=["", "\n", ",", "Apache 2.0 ,"], warning_values=[ "Custom license", "Custom / MIT", + "Custom, MIT", ], ) diff --git a/metadata/tests/parse_test.py b/metadata/tests/parse_test.py index 169d716ec..8131102ee 100644 --- a/metadata/tests/parse_test.py +++ b/metadata/tests/parse_test.py @@ -77,7 +77,7 @@ class ParseTest(unittest.TestCase): " https://www.example.com/parser"), ("Version", "1.0.12"), ("Date", "2020-12-03"), - ("License", "Apache-2.0 and MIT"), + ('License', 'Apache-2.0 and MIT and not_an_spdx'), ("License File", "LICENSE"), ("Security Critical", "yes"), ("Shipped", "yes"), diff --git a/metadata/tests/type_narrowing_test.py b/metadata/tests/type_narrowing_test.py index 7f17c1711..aab4c8d71 100644 --- a/metadata/tests/type_narrowing_test.py +++ b/metadata/tests/type_narrowing_test.py @@ -127,7 +127,7 @@ class FieldValidationTest(unittest.TestCase): expect = self._test_on_field(fields.LICENSE) expect("", None, "treat empty string as None") expect("LICENSE-1", ["LICENSE-1"], "return as a list") - expect("LGPL v2 and BSD", ["LGPL v2", "BSD"], "return as a list") + expect("LGPL v2, BSD", ["LGPL v2", "BSD"], "return as a list") def test_license_file(self): # TODO(b/321154076): Consider excluding files that doesn't exist on diff --git a/metadata/tests/validate_test.py b/metadata/tests/validate_test.py index 420c0a9f3..72e0ba44d 100644 --- a/metadata/tests/validate_test.py +++ b/metadata/tests/validate_test.py @@ -56,7 +56,7 @@ class ValidateContentTest(unittest.TestCase): source_file_dir=_SOURCE_FILE_DIR, repo_root_dir=_THIS_DIR, ) - self.assertEqual(len(results), 9) + self.assertEqual(len(results), 11) error_count = 0 warning_count = 0 for result in results: @@ -64,8 +64,8 @@ class ValidateContentTest(unittest.TestCase): error_count += 1 else: warning_count += 1 - self.assertEqual(error_count, 7) - self.assertEqual(warning_count, 2) + self.assertEqual(error_count, 8) + self.assertEqual(warning_count, 3) class ValidateFileTest(unittest.TestCase): @@ -94,7 +94,7 @@ class ValidateFileTest(unittest.TestCase): filepath=_INVALID_METADATA_FILEPATH, repo_root_dir=_THIS_DIR, ) - self.assertEqual(len(results), 9) + self.assertEqual(len(results), 11) error_count = 0 warning_count = 0 for result in results: @@ -102,8 +102,8 @@ class ValidateFileTest(unittest.TestCase): error_count += 1 else: warning_count += 1 - self.assertEqual(error_count, 7) - self.assertEqual(warning_count, 2) + self.assertEqual(error_count, 8) + self.assertEqual(warning_count, 3) class CheckFileTest(unittest.TestCase): @@ -141,7 +141,7 @@ class CheckFileTest(unittest.TestCase): # self.assertEqual(len(errors), 7) # self.assertEqual(len(warnings), 2) self.assertEqual(len(errors), 0) - self.assertEqual(len(warnings), 9) + self.assertEqual(len(warnings), 11) class ValidationResultTest(unittest.TestCase):