spdx-spec icon indicating copy to clipboard operation
spdx-spec copied to clipboard

appendix-IV: Refactor the license expression appendix

Open wking opened this issue 7 years ago • 18 comments

This is a large diff, but I aimed for restructuring/polishing without changing the end result. I did make a few minor, intentional changes:

  • Extended license-id to include appendix I.3 (deprecated licenses). We don't want folks using these in license expressions (because they're deprecated), but they are valid (or we would have removed them instead of just deprecating them). That means that in some cases the nature of a string is unclear. For example GPL-1.0+ could be the depreacted license-id, or it could be a simple-expression using the non-deprecated GPL-1.0 license-id and the + operator. I don't think that's a problem though, because I can't think of a case where the ambiguity would matter.

  • I've allowed + for license-ref (it used to be only for license-id). There could be external licenses which offer a choice between only-this-version and or-later grants, and allowing + for license-ref makes it easier to support those licenses as they transition into the SPDX License List. This isn't a big deal, but it avoids needing separate license-refs for the only-this-version and or-later grants if you need both.

  • I've added explicit whitespace handling, vs. the previous version which just discussed it in the text. That way the ABNF is the sole source of normative syntax information.

  • I've added enclosed-license-expression, so consumers like the tag:value format can suggest/require it. This allows for more precision in consumers (e.g. appendix V should be updated to require enclosed-license-expression), but I've left those other sections alone for this commit. Ideally the tag:value line would be moved to a separate section that defined the tag-value format, but we don't have such a section yet.

Also, it seems odd that there's no way to define an external license exception (LicenseExceptionRef?). But I haven't touched that in this commit.

The RDF/XML bindings seem to have some holes around the + and WITH operators. I've left some FIXMEs where I think we need adjustments.

wking avatar Sep 13 '17 22:09 wking

Cross linking:

  • An old spdx Bugzilla issue with ABNF discussion.
  • jslicense/spdx-expression-parse.js#20 with ABNF discussion.

wking avatar Oct 12 '17 23:10 wking

FYI, I already had implemented LicenseExpressionParser: Allow "+" as part of license-refs as part of https://github.com/spdx/tools/pull/66, but back then we couldn't reach a consensus about that on the mailing list IIRC, so I closed my PR unmerged.

sschuberth avatar Oct 16 '17 07:10 sschuberth

@sschuberth - Do you recall where we left the conversation? I couldn't easily find the email thread. In reviewing your pull request, the tool changes look reasonable but I don't recall if there was a concern raised earlier.

goneall avatar Oct 19 '17 16:10 goneall

Some previous discussion in [1,2,3,4]. [4] looks like #16. But I haven't gone through those threads carefully enough to post a summary yet.

Subject: Is "+" a valid character of a LicenseRef idstring? Date: Wed Oct 28 09:28:20 UTC 2015 [2]: https://lists.spdx.org/pipermail/spdx-tech/2015-November/002874.html Subject: Is "+" a valid character of a LicenseRef idstring? Date: Mon Nov 2 09:56:47 UTC 2015 [3]: https://lists.spdx.org/pipermail/spdx-tech/2015-December/002966.html Subject: [Bug 1333] New: SPDX SPEC fails to discuss whitespace Date: Tue Dec 8 16:54:54 UTC 2015 [4]: https://lists.spdx.org/pipermail/spdx-tech/2015-December/002986.html Subject: [Bug 1326] Inconsistency whether "+" is a valid character in LicenseRef ID strings Date: Tue Dec 22 18:17:39 UTC 2015

wking avatar Oct 19 '17 16:10 wking

@goneall, I also couldn't find the email thread again, but AFAIR my PR would need to take the SPDX version into account, which would have made the already quite ugly code even more ugly, so I decided to not further work on this.

sschuberth avatar Oct 19 '17 16:10 sschuberth

After reading through the history, it looks like we decided to deprecate the + in the license ID's which would allow for it to be used in license-ref's for version 2.1 or later.

I'll think about the code a bit more - I agree with @sschuberth that it would be quite messy to check for the version. That being said, we should update the tools to handle this case.

goneall avatar Oct 19 '17 21:10 goneall

I've pushed cf8decf → 9c8d189 addressing some of @goneall's points and including a few more copy-edits. Outstanding issues are:

  • Whether we want to keep <spdx:LicenseException> now that we're adding <spdx:WithExceptionOperator> (thread).

  • Whether we want SHOULD or MUST for tag:values using enclosed-license-expression (thread).

wking avatar Oct 20 '17 19:10 wking

I've pushed 9c8d189 → 922031a dropping <spdx:LicenseException> and adding <spdx:WithExceptionOperator> now that @goneall gave that a green light. We may come back to external extention definitions in follow-up work.

We've also agreed to punt the tag:value enclosed-license-expression SHOULD/MUST to follow-up work.

I'm not aware of any outstanding change requests for this PR.

wking avatar Oct 20 '17 23:10 wking

@wking thanks for all the updates. I like the current state of the files.

@kestewart if you get a chance - could you review as well? I would like to merge this in so that I can re-raise the must/should parenthesis discussions.

goneall avatar Oct 21 '17 00:10 goneall

I've allowed + for license-ref (it used to be only for license-id). There could be external licenses which offer a choice between only-this-version and or-later grants, and allowing + for license-ref makes it easier to support those licenses as they transition into the SPDX License List. This isn't a big deal, but it avoids needing separate license-refs for the only-this-version and or-later grants if you need both.

This does not make sense to me at all. A ref is a ref. This is not an ID. This is easy to implement but just has no clear basis. And it opens a can of worms that make things complicated otherwise. Should you have one text for each of a + and base ref version or only one?

pombredanne avatar Nov 14 '17 08:11 pombredanne

On Tue, Nov 14, 2017 at 08:29:58AM +0000, Philippe Ombredanne wrote:

I've allowed + for license-ref (it used to be only for license-id)…

This does not make sense to me at all. A ref is a ref. This is not an ID.

I see very little space between license identifiers defined on the license list (e.g. ‘GPL-3.0’) and a license identifier defined by a third party (e.g. ‘LicenseRef-MIT-Style-1’) besides whether you have commit rights to the SPDX license list. In both cases, there is a license, and somebody is giving it a handful of characters as an identifier. I don't see a reason to support operators for identifiers coined by the SPDX but then to not support those same operators for identifiers coined by someone else.

And it opens a can of worms that make things complicated otherwise. Should you have one text for each of a + and base ref version or only one?

Only one. This is somewhat up in the air with 1's hard-coded approach which removes the + operator entirely (or maybe just in some cases?). But regardless of how that shakes out, the motivation here is to treat SPDX-created IDs and third-party-created IDs with the same logic. So however many copies of the GPL 2.0 the SPDX wants to keep to support its various versioned grants, third parties should be able to keep that many as well, and use the same operators to craft grants based on their LicenseRef identifiers.

wking avatar Nov 14 '17 09:11 wking

I've pushed 922031a → eafa739 which restores semantic docs for + (which I'd accidentally dropped with 922031a) and adds a new explanation of the AGPL-1.0+ based on this reasoning.

wking avatar Dec 28 '17 17:12 wking

I've pushed 649889f -> afc5c1e, rebasing onto master (around conflicts with eafa739 and b971d8d6). I've also adjusted to the new -only and -or-later forms which happend in license list v3.0 and added a paragraph addressing case-senstivity for #63. We should get spdx/license-list-XML#651 landed an a list release cut so we have a nice place to link from the spec when discussing case-uniqueness commitments. Once that happens, I'll rebase this PR again to pick up the nicer link.

wking avatar Jun 05 '18 19:06 wking

@goneall and @kestewart, what are your thoughts on this PR for 2.2 vs 3.0?

iamwillbar avatar Jan 22 '20 19:01 iamwillbar

@iamwillbar @kestewart I'm thinking 3.0. There are a large number of issues mixed into this one PR. Resolving all of these will likely take a few tech calls.

goneall avatar Jan 23 '20 00:01 goneall

I'm happy to split this up if you want to lay out how you want it divided :)

wking avatar Jan 23 '20 03:01 wking

@wking Thanks for the offer to split it up. I think the controversial items to be excluded would be the + operator changes for licenseRef's and the parenthesis "MUST" requirement.

The fixes to the RDF would be nice to get into the 2.2 release.

@kestewart Can you take a look and let us know if you agree with that split.

goneall avatar Jan 29 '20 23:01 goneall

Closing this as stale. If disagree, please reopen.

kestewart avatar Feb 05 '24 17:02 kestewart

This PR is rather stale - closing. If we want to refactor, we should refactor the current version in the development/v3.0 branch.

goneall avatar Apr 03 '24 21:04 goneall