-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Release 4.3.4 breaking change with SPDX expressions #792
Comments
This also broke our usage of this action -- our allow-licenses: (Apache-2.0 OR ... OR MIT OR ... OR MPL-2.0) On v4.3.3 this was working fine, but now this fails on a license that should pass the check: Edit: Here's a repo with a minimal reproduction of the issue, showing the same rule working on 4.3.3 but failing on 4.3.4: https://github.com/kade-robertson/dra-repro |
Thanks for the report. We'll take a look. |
Hi folks, this was not intended to be a breaking change at all, so I do apologize for that. I've spent some time investigating this and it appears that our new underlying SPDX parser does work a little bit differently here. When parsing I think we can try to support more detailed expressions here in the future, but for now I believe the solution is to use only an array of license identifiers in |
@juxtin it seems like it's also failing with a single license
when we have |
Using expressions directly in the From the changelog of Is there a limitation somewhere on why it does not work with |
@lucacome I believe the problem is not just limited to the individual expression, but it may break license matching altogether. We may be able to improve our config validation to surface this in a clearer way. In the meantime, does that still work when you remove lines like these that still have expressions? |
The PR you're referring to was originally intended to fix just that kind of behavior by switching to a library that had better support for SPDX expression parsing. However, this didn't give us the kinds of easy wins that we'd hoped. Instead, I tried to make it clear in the changelog that this was mainly a refactor and wasn't expected to address many, if any, of the outstanding issues around improper handling of SPDX expressions. That said, those issues are still on our radar and we do plan to address them in a future update. |
@juxtin removing those lines resolved the problem for us, thanks! |
I'll go ahead and close this issue for now. Again, we are hoping to expand our support for these kinds of expressions in the future, but for now the fix is to use only license identifiers in |
The github dependency review action no longer allows to pass in logically connected licenses. So if there is a `License-A AND License-B` in the list of allowed licenses, the action will always fail if a dependency has only `License-A` or `License B`. For reference see: actions/dependency-review-action#792 This results in false positives for dependencies with something like `License-A AND License-B`, however as this is much rarer than e.g. `MIT` license, it is preferable to pass on all the single licenses.
The github dependency review action no longer allows to pass in logically connected licenses. So if there is a `License-A AND License-B` in the list of allowed licenses, the action will always fail if a dependency has only `License-A` or `License B`. For reference see: actions/dependency-review-action#792 This results in false positives for dependencies with something like `License-A AND License-B`, however as this is much rarer than e.g. `MIT` license, it is preferable to pass on all the single licenses.
The github dependency review action no longer allows to pass in logically connected licenses. So if there is a `License-A AND License-B` in the list of allowed licenses, the action will always fail if a dependency has only `License-A` or `License B`. For reference see: actions/dependency-review-action#792 This results in false positives for dependencies with something like `License-A AND License-B`, however as this is much rarer than e.g. `MIT` license, it is preferable to pass on all the single licenses.
The github dependency review action no longer allows to pass in logically connected licenses. So if there is a `License-A AND License-B` in the list of allowed licenses, the action will always fail if a dependency has only `License-A` or `License B`. For reference see: actions/dependency-review-action#792 This results in false positives for dependencies with something like `License-A AND License-B`, however as this is much rarer than e.g. `MIT` license, it is preferable to pass on all the single licenses.
Describe the bug
Release 4.3.4 changes how license list is parsed and having SPDX expression like
Apache-2.0 AND MIT
now causes all license checks to fail. But also does not seem to work as intended (qustion mark?).To Reproduce
Apache-2.0 AND MIT
expression in theallow-licenses
input, like so:Apache-2.0 AND MIT
license expression (like morelinq 4.2.0)Apache-2.0
orMIT
licensesExpected behavior
Apache-2.0 AND MIT
should be able to pass.Screenshots
![image](https://private-user-images.githubusercontent.com/59015010/348239328-3fe01745-d79a-4d6d-bb24-e47906b80d76.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjQ0MzIxODksIm5iZiI6MTcyNDQzMTg4OSwicGF0aCI6Ii81OTAxNTAxMC8zNDgyMzkzMjgtM2ZlMDE3NDUtZDc5YS00ZDZkLWJiMjQtZTQ3OTA2YjgwZDc2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MjMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODIzVDE2NTEyOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWM2M2I5YTgxZmZhNjE5ZWZhNDk1YzI2ZmFlN2IwYjEyNGIzMTRmNjRkM2E5OWY5YjI2N2U0YzFjN2E0M2EzNjQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.mtVwKlwYFDfFfDThcOVuAXoGlcN-oWMIgmX-_ove0Eo)
v4.3.4 with
Apache-2.0 AND MIT
inallow-licenses
input:v4.3.4 without
![image](https://private-user-images.githubusercontent.com/59015010/348239236-40eb3d18-002f-4874-8c0f-08fef36ca6db.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjQ0MzIxODksIm5iZiI6MTcyNDQzMTg4OSwicGF0aCI6Ii81OTAxNTAxMC8zNDgyMzkyMzYtNDBlYjNkMTgtMDAyZi00ODc0LThjMGYtMDhmZWYzNmNhNmRiLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MjMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODIzVDE2NTEyOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWNlNzEwY2U0OTYzNjc1OWZhOTA2OGQzMjMzZTk3NDYwMzEyZDY0OGUwY2FkMWI3M2IwZmE3ZGU4MDZkMDk2YzEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.Etrd0bsG-6UlNYdEN4F1w06VZ6tqhO9ICdB3wGurqr4)
Apache-2.0 AND MIT
:v4.3.3 with
Apache-2.0 AND MIT
(no screenshot as it passes)Action version
4.3.4
Examples
Project file:
Action:
Additional context
None.
The text was updated successfully, but these errors were encountered: