search

spdx / spdx-spec

The SPDX specification in MarkDown and HTML formats.
https://spdx.github.io/spdx-spec/
Other
274 stars 133 forks source link

allow `and`, `or` and `with` operators #876

Closed xsuchy closed 4 months ago

xsuchy commented 5 months ago

This is follow up of discussion https://lists.spdx.org/g/spdx/message/1798 where this was found as good compromise https://lists.spdx.org/g/spdx/message/1817

zvr commented 5 months ago

I assume this should be oriented towards the development/v3.0 branch.

maxhbr commented 4 months ago

I assume this should be oriented towards the development/v3.0 branch.

I think that this is also a fix for 2.3, but would need new release to be affective

goneall commented 4 months ago

I added PR #877 which adds this file to the development/v3.0 branch.

Once that is merged, let's create a PR to add this change into v3.0 and leave this PR open and/or merge it in case we do a dot release for v2.3 (note: no additional releases are currently planned).

pombredanne commented 4 months ago

SPDX license ids are unique ignoring case in practice and the case of operators does not matter and does not change the meaning of anything, the case should not matter.

Therefore, my 2 cents would be to:

  1. Not care about the case at all at parsing time, be case insensitive.
  2. Have a canonical representation (say all uppercase, OR all lowercase, OR mixed uppercase for operators and lowercase for ids, or anything that is simple and foolproof).

If you want to make things more practical for adopters, you should consider these changes in 3.0. The proposed change here for 2.3 is neither bad, nor great and it creates unnecessary churn for all implementers: it would be best to do better and settle the case issue once for all to avoid further churn.

Being strict on which character case to use for SPDX operators and SPDX license ids has always been something that I found to be problematic with no actual value except making the experience of adopters and implementers more complex and error prone, and is something that I would qualify as a self-inflicted wound.

pombredanne commented 4 months ago

Just to add another squeak to the wheel... this will likely make many tools stop working when they validate such expression, so this is likely an API-breaking change.

maxhbr commented 4 months ago

Just to add another squeak to the wheel... this will likely make many tools stop working when they validate such expression, so this is likely an API-breaking change.

Agreed, this is a breaking change for consumers.

goneall commented 4 months ago

Thanks @pombredanne and @maxhbr for pointing out the potential for breaking changes.

The parsing algorithms I build are case insensitive, so I kind of missed this in my reviews.

Two points:

xsuchy commented 4 months ago

I agree, closing here. I will open new PR against 3.0 when #877 will be merged.