Skip to content
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

spdx-utils: Disambiguate parser rules for ids and exceptions #1498

Closed
wants to merge 1 commit into from

Conversation

sschuberth
Copy link
Member

@sschuberth sschuberth commented May 2, 2019

Previously, an id without the optional "+" was the same as an exception,
and only the rule order determined that an exception was given precedence.
Now, require an exception to actually require the word "exception" as
specified at

https://spdx.github.io/spdx-spec/appendix-I-SPDX-license-list/#i2-exceptions-list

Signed-off-by: Sebastian Schuberth sebastian.schuberth@here.com


This change is Reviewable

Previously, an id without the optional "+" was the same as an exception,
and only the rule order determined that an exception was given precedence.
Now, require an exception to actually require the word "exception" as
specified at

https://spdx.github.io/spdx-spec/appendix-I-SPDX-license-list/#i2-exceptions-list

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@here.com>
LICENSEREFERENCE : ('DocumentRef-' IDSTRING ':')? 'LicenseRef-' IDSTRING ;
IDSTRING : (ALPHA | DIGIT)(ALPHA | DIGIT | '-' | '.')* ;
IDSTRING : (ALPHA | DIGIT)(ALPHA | DIGIT | '-' | '.')* ;
LICENSE_EXCEPTION : (IDSTRING '-exception' | IDSTRING '-exception-' IDSTRING) ;
Copy link
Member

Choose a reason for hiding this comment

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

It's true that all "official" SDPX exceptions contain "-exception-", and that what comes after WITH needs to be in that list, but it does not specifically define that "-exception-" needs to be contained. For our lenient implementation that shall be able to parse also expressions with typos or non-SPDX IDs I would not make that requirement.
Is there any specific problem you want so solve? Otherwise I would not make the grammar more complex than it needs to be.

Copy link
Member Author

Choose a reason for hiding this comment

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

The specific issue I wanted to solve was the bogus line 1:8 no viable alternative at input 'licensea' console output I see when running PackageCurationTest. I noticed the ambiguity between licenseIdExpression and licenseExceptionExpression while looking for the cause, and wanted to address it along the way. In any case, this change does not fix the "no viable alternative at input" issue.

@sschuberth sschuberth closed this May 2, 2019
@sschuberth sschuberth deleted the spdx-exception-lexer branch May 2, 2019 14:51
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.

2 participants