Don't error, just print a warning if the found license is invalid

Also, remove -l CLI option to provide the license manually.
Whoever needs that, can edit the resulting specfile.

Fixes: #47

pyp2spec/conf2spec.py

@@ -93,20 +93,33 @@ def python3_pkgversion_or_3(config):
     return "%{python3_pkgversion}" if config.get_string("python_alt_version") else "3"
 
 
+def get_license_string(config):
+    none_notice = "# No license information obtained, it's up to the packager to fill it in"
+    detected_notice = ("# Check if the automatically generated License and its "
+        "spelling is correct for Fedora\n"
+        "# https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/")
+    if (license := config.get_string("license")):
+        return (license, detected_notice)
+    return ("...", none_notice)
+
+
 def fill_in_template(config):
     """Return template rendered with data from config file."""
 
     with (files("pyp2spec") / TEMPLATE_FILENAME).open("r", encoding="utf-8") as f:
         spec_template = Template(f.read())
 
+    license, license_notice = get_license_string(config)
+
     result = spec_template.render(
         additional_build_requires=list_additional_build_requires(config),
         archful=config.get_bool("archful"),
         archive_name=config.get_string("archive_name"),
         automode=config.get_bool("automode"),
         description=wrap_description(config),
         extras=",".join(config.get_list("extras")),
-        license=config.get_string("license"),
+        license=license,
+        license_notice=license_notice,
         name=config.get_string("pypi_name"),
         python_name=config.get_string("python_name"),
         pypi_version=config.get_string("pypi_version"),

pyp2spec/license_processor.py

@@ -15,9 +15,6 @@ class NoSuchClassifierError(Pyp2specError):
     """Raised when the detected license classifier doesn't exist in pyp2spec's data"""
 
 
-class InvalidSPDXExpressionError(Pyp2specError):
-    """Raised when the assumed SPDX expression is not valid"""
-
 
 def _load_package_resource(filename):
     with (files("pyp2spec") / filename).open("r", encoding="utf-8") as f:
@@ -69,9 +66,12 @@ def classifiers_to_spdx_identifiers(classifiers):
 def license_keyword_to_spdx_identifiers(license_keyword):
     """Return a sorted list of SPDX identifiers extracted from the license_keyword
     
-    If no identifiers were parsed out of the license_keyword, return None.
-    Raise ValueError if the license_keyword isn't a valid SPDX expression.
+    If no identifiers were parsed out of the license_keyword or
+    the license_keyword is unparseable, return None.
     """
+    # nothing to transform, ergo no identifiers
+    if not license_keyword:
+        return None
 
     # `Artistic-1.0-Perl` alone is forbidden in Fedora, but the combination is allowed
     # These expressions may be a part of even longer license strings, which is impossible to cover
@@ -90,12 +90,11 @@ def license_keyword_to_spdx_identifiers(license_keyword):
     licensing = get_spdx_licensing()
     try:
         parsed_license = licensing.parse(license_keyword, validate=True)
-        if parsed_license is None:  # license keyword is probably empty
-            return None
         # The objects are stored in sets, sort and return as a list
         return sorted(parsed_license.objects)
     except ExpressionError as err:
-        raise InvalidSPDXExpressionError(f"Invalid SPDX expression: {license_keyword}") from err
+        # Don't bubble the error up, the calling function will handle the invalid result
+        return None
 
 
 def _load_fedora_licenses(source_path=None, session=None):

pyp2spec/pyp2conf.py

@@ -169,7 +169,6 @@ def transform_to_spdx(self):
         package license metadata (classifiers or license keyword).
         
         If multiple identifiers are found, create an expression that's the safest option (with AND as joining operator).
-        Raise NoLicenseDetectedError if no valid SPDX identifiers are found.
         """
 
         if (license_classifiers := self.filter_license_classifiers()):
@@ -181,9 +180,6 @@ def transform_to_spdx(self):
         license_keyword = self.package_data["info"]["license"]
         identifiers = license_keyword_to_spdx_identifiers(license_keyword)
 
-        # No more options to detect licenses left, hence explicit fail
-        if not identifiers:
-            raise NoLicenseDetectedError("No valid license detected.")
         return (identifiers, license_keyword)
 
     def license(self, *, check_compliance=False, licenses_dict=None):
@@ -193,18 +189,25 @@ def license(self, *, check_compliance=False, licenses_dict=None):
         SPDX identifiers against the Fedora allowed licenses.
         This isn't a 100% reliable solution and in case of ambiguous results,
         the license is discarded as invalid.
-        If the compliant license can't be determined, raise NoLicenseDetectedError.
         """
 
         identifiers, expression = self.transform_to_spdx()
+        if not identifiers:
+            inspect_notice = "Inspect the project manually to find the license."
+            if expression:
+                err_string = f"WARNING: The found license `{expression}` is not a valid SPDX expression. " + inspect_notice
+                click.secho(err_string, fg="red")
+            else:
+                err_string = "WARNING: No license found. " + inspect_notice
+                click.secho(err_string, fg="red")
+            return None
         if check_compliance:
             is_compliant, bad_identifiers = good_for_fedora(identifiers, session=self._session, licenses_dict=licenses_dict)
             if not is_compliant:
                 if bad_identifiers:
                     err_string = "The detected licenses: `{0}` aren't allowed in Fedora."
                     click.secho(err_string.format(", ".join(bad_identifiers), fg="red"))
-                raise NoLicenseDetectedError("Could not create a compliant license field.")
-
+                return None
         return expression
 
     def summary(self):
@@ -349,7 +352,6 @@ def create_config_contents(
     package,
     version=None,
     session=None,
-    license=None,
     compliant=False,
     python_alt_version=None,
     automode=False,
@@ -371,9 +373,8 @@ def create_config_contents(
     if version is None:
         click.secho(f"Assuming PyPI --version={pkg.version}", fg="yellow")
 
-    if license is None:
-        license = pkg.license(check_compliance=compliant)
-        click.secho(f"Assuming --license={license}", fg="yellow")
+    if (license := pkg.license(check_compliance=compliant)) is not None:
+        contents["license"] = license
 
     if automode:
         contents["automode"] = True
@@ -390,7 +391,6 @@ def create_config_contents(
     contents["summary"] = pkg.summary()
     contents["version"] = convert_version_to_rpm_scheme(pkg.version)
     contents["pypi_version"] = pkg.pypi_version_or_macro()
-    contents["license"] = license
     contents["pypi_name"] = pkg.pypi_name
     contents["python_name"] = pkg.python_name(python_alt_version=python_alt_version)
     contents["url"] = pkg.project_url()
@@ -422,7 +422,6 @@ def create_config(options):
     contents = create_config_contents(
         options["package"],
         version=options["version"],
-        license=options["license"],
         compliant=options["fedora_compliant"],
         python_alt_version=options["python_alt_version"],
         automode=options["automode"]
@@ -440,10 +439,6 @@ def pypconf_args(func):
         "--version", "-v",
         help="Provide package version to query PyPI for, default: latest",
     )
-    @click.option(
-        "--license", "-l",
-        help="Provide license name",
-    )
     @click.option(
         "--fedora-compliant", is_flag=True,
         help="Check whether license is compliant with Fedora",

pyp2spec/template.spec

@@ -8,8 +8,7 @@ Release:        %autorelease
 # Fill in the actual package summary to submit package to Fedora
 Summary:        {{summary}}
 
-# Check if the automatically generated License and its spelling is correct for Fedora
-# https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/
+{{ license_notice }}
 License:        {{license}}
 URL:            {{url}}
 Source:         {{source}}

tests/expected_specfiles/python-sphinx.spec

@@ -4,9 +4,8 @@ Release:        %autorelease
 # Fill in the actual package summary to submit package to Fedora
 Summary:        Python documentation generator
 
-# Check if the automatically generated License and its spelling is correct for Fedora
-# https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/
-License:        fake-license
+# No license information obtained, it's up to the packager to fill it in
+License:        ...
 URL:            https://www.sphinx-doc.org/
 Source:         %{pypi_source sphinx}
 

tests/test_configs/customized_python-sphinx.conf

@@ -2,8 +2,6 @@ description = "This is package 'sphinx' generated automatically by pyp2spec."
 summary = "Python documentation generator"
 version = "7.2.6"
 pypi_version = "%{version}"
-# customized: license (the upstream metadata contain garbage)
-license = "fake-license"
 pypi_name = "sphinx"
 python_name = "python-sphinx"
 url = "https://www.sphinx-doc.org/"

tests/test_license_processor.py

@@ -2,7 +2,7 @@
 
 from pyp2spec.license_processor import classifiers_to_spdx_identifiers, license_keyword_to_spdx_identifiers
 from pyp2spec.license_processor import _is_compliant_with_fedora, good_for_fedora
-from pyp2spec.license_processor import InvalidSPDXExpressionError, NoSuchClassifierError
+from pyp2spec.license_processor import NoSuchClassifierError
 
 
 @pytest.mark.parametrize(
@@ -61,9 +61,8 @@ def test_perl_license_raises_exception():
         ("string"),
     ),
 )
-def test_invalid_expressions_raise_errors(expression):
-    with pytest.raises(InvalidSPDXExpressionError):
-        license_keyword_to_spdx_identifiers(expression)
+def test_invalid_expressions_return_None(expression):
+    assert license_keyword_to_spdx_identifiers(expression) is None
 
 
 

tests/test_pyp2conf.py

@@ -85,7 +85,6 @@ def test_archful_package(betamax_session):
     package = "numpy"
     config = create_config_contents(
         package=package,
-        license="fake-string",
         version="1.25.2",
         automode=True,
         session=betamax_session,
@@ -101,7 +100,6 @@ def test_package_with_extras(betamax_session):
     package = "sphinx"
     config = create_config_contents(
         package=package,
-        license="fake-license",
         session=betamax_session,
     )
 
@@ -131,8 +129,7 @@ def test_license_classifier_read_correctly():
 def test_no_license_classifiers_and_no_license_keyword():
     pkg = PypiPackage("_", version="1.2.3", package_metadata={"info":{"classifiers": [], "license": ""}})
     assert pkg.filter_license_classifiers() == []
-    with pytest.raises(NoLicenseDetectedError):
-        pkg.license()
+    assert pkg.license() is None
 
 
 def test_compliant_license_is_returned(fake_fedora_licenses):
@@ -155,8 +152,7 @@ def test_bad_license_fails_compliance_check(fake_fedora_licenses):
     ]}}
     pkg = PypiPackage("_", version="1.2.3", package_metadata=fake_pkg_data)
 
-    with pytest.raises(NoLicenseDetectedError):
-        pkg.license(check_compliance=True, licenses_dict=fake_fedora_licenses)
+    assert pkg.license(check_compliance=True, licenses_dict=fake_fedora_licenses) is None
 
 
 def test_bad_license_without_compliance_check_is_returned():
@@ -174,8 +170,7 @@ def test_mix_good_bad_licenses_fail_compliance_check(fake_fedora_licenses):
         "License :: OSI Approved :: MIT License",
     ]}}
     pkg = PypiPackage("_", version="1.2.3", package_metadata=fake_pkg_data)
-    with pytest.raises(NoLicenseDetectedError):
-        pkg.license(check_compliance=True, licenses_dict=fake_fedora_licenses)
+    assert pkg.license(check_compliance=True, licenses_dict=fake_fedora_licenses) is None
 
 
 def test_mix_good_bad_licenses_without_compliance_check_are_returned():
@@ -196,16 +191,14 @@ def test_license_keyword_without_compliance_check():
 def test_license_keyword_with_compliance_check(fake_fedora_licenses):
     fake_pkg_data = {"info": {"license": "RSCPL", "classifiers": []}}
     pkg = PypiPackage("_", version="1.2.3", package_metadata=fake_pkg_data)
-    with pytest.raises(NoLicenseDetectedError):
-        pkg.license(check_compliance=True, licenses_dict=fake_fedora_licenses)
+    assert pkg.license(check_compliance=True, licenses_dict=fake_fedora_licenses) is None
 
 
 @pytest.mark.parametrize("compliant", (True, False))
 def test_empty_license_keyword_fails(compliant, fake_fedora_licenses):
     fake_pkg_data = {"info": {"license": "", "classifiers": []}}
     pkg = PypiPackage("_", version="1.2.3", package_metadata=fake_pkg_data)
-    with pytest.raises(NoLicenseDetectedError):
-        pkg.license(check_compliance=compliant, licenses_dict=fake_fedora_licenses)
+    assert pkg.license(check_compliance=compliant, licenses_dict=fake_fedora_licenses) is None
 
 
 def test_zip_sdist_is_added_to_source_macro():