Skip to content

fix: handle the license with exception case, e.g., llvmlite#142

Closed
joanise wants to merge 1 commit into
FHPythonUtils:masterfrom
joanise:issue-137
Closed

fix: handle the license with exception case, e.g., llvmlite#142
joanise wants to merge 1 commit into
FHPythonUtils:masterfrom
joanise:issue-137

Conversation

@joanise

@joanise joanise commented Jan 26, 2026

Copy link
Copy Markdown

Purpose of This Pull Request

Please check the relevant option by placing an "X" inside the brackets:

  • Documentation update
  • Bug fix
  • New feature
  • Other (please explain):

Overview of Changes

As described in #137, licensecheck currently hits an unhandled exception when faced with a license with exception, such as is found in llvmlite.

The underlying issue is that not all tokens handled on line 121 of packageinfo.py have a key attribute.

My solution is to guard against this problem with getattr(x, "key", str(x)) which fixes the problem.

Related Issue

If this pull request addresses a specific issue, please mention it using the
format "Fixes #IssueNumber"

Fixes #137

Reviewer Focus

It's a pretty simple patch, so nothing specific.

Additional Notes

I wanted to provide unit testing with my PR, but I did not manage to get the existing suite to run correctly on either my Windows or Linux machine. I've added a minimal data file for testing, however:

licensecheck -r tests/data/issue_137.txt

raises an exception on the current master branch but outputs llvmlite │ APACHE-2.0 WITH LLVM-EXCEPTION;; BSD-2-CLAUSE as expected with this PR.

Comment thread licensecheck/packageinfo.py Outdated

tokens = sorted(parsed.literals)
pkg_info.license = ucstr(JOINS.join(x.key for x in tokens))
pkg_info.license = ucstr(JOINS.join(getattr(x, "key", str(x)) for x in tokens))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
pkg_info.license = ucstr(JOINS.join(getattr(x, "key", str(x)) for x in tokens))
final_keys = []
for token in parsed.literals:
final_keys.extend(sub_symbol.key for sub_symbol in token.decompose())
pkg_info.license = ucstr(JOINS.join(finals_keys))

Maybe use the decompose function to get the symbols and not fallback to a simple string representation?

@joanise joanise Mar 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Just tested your suggestion, and I'm not sure what the better answer is. With my code, for the test file I provided, I get:

│ ✔          │ llvmlite │ APACHE-2.0 WITH LLVM-EXCEPTION;; BSD-2-CLAUSE │

with your suggested change, instead, I get the warnings 'LLVM-EXCEPTION' License not identified so falling back to UNKNOWN followed by

│ ✔          │ llvmlite │ APACHE-2.0;; LLVM-EXCEPTION;; BSD-2-CLAUSE │

which is not technically correct: llvmlite does not have Apache-2.0 as a license and LLVM-Exception as a license, rather it has "The Apache License v2.0 with LLVM Exceptions" as a license (see https://github.com/numba/llvmlite/blob/main/LICENSE.thirdparty).

Given that, I believe license_expression="BSD-2-Clause AND Apache-2.0 WITH LLVM-exception", (see https://github.com/numba/llvmlite/blob/20226fe41989cffeea322332bb1356c4b1d5a65d/setup.py#L223) should get rendered as my proposed change does.

Maybe there's a cleaner way to accomplish this than my simple casting to a string? I'm open to suggestions.

joshuanapoli added a commit to CVector-Energy/pyproject-license-check that referenced this pull request Mar 31, 2026
@joanise

joanise commented Jun 16, 2026

Copy link
Copy Markdown
Author

@FredHappyface while you're actively working on this project right now, do you think you could take this PR or find another solution to make llvmlite pass through licensecheck?

@joanise

joanise commented Jun 16, 2026

Copy link
Copy Markdown
Author

Oh, I see you commented on #137 a few days ago saying you would look at the issue. Thank you!

@FredHappyface

Copy link
Copy Markdown
Member

Cheers for the nudge, took me a minute to unpick things as it has been a while since I've visited the project. Absolutely no idea why .key was used in all honesty as it doesn't appear to be part of the boolean.boolean.Expression interface, though applied your fix. Thank you for taking the time to look at this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: AttributeError: 'LicenseWithExceptionSymbol' object has no attribute 'key'

3 participants