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

appendix-IV: Refactor the license expression appendix #37

Closed
wants to merge 1 commit into from

Conversation

wking
Copy link
Contributor

@wking wking commented Sep 13, 2017

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
Copy link
Contributor Author

wking commented Oct 12, 2017

Cross linking:

@sschuberth
Copy link
Member

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

@goneall
Copy link
Member

goneall commented Oct 19, 2017

@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.

@wking
Copy link
Contributor Author

wking commented Oct 19, 2017 via email

@sschuberth
Copy link
Member

@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.

@goneall
Copy link
Member

goneall commented Oct 19, 2017

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.

Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

Made a complete pass through and suggested items for FIXME's plus a few additional comments

For instance, the following expression:
### IV.3.1 The `+` Operator <a name="IV.3.1"></a>

FIXME: How are you supposed to cover this in XML?
Copy link
Member

Choose a reason for hiding this comment

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

The following text can be added:
The + operator can be expressed in XML via a <spdx:OrLaterOperator> element, with a single spdx:member property for the license preceeding the + operator.

 <spdx:OrLaterOperator>
      <spdx:member rdf:resource="http://spdx.org/licenses/GPL-2.0"/>
 </spdx:OrLaterOperator>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in cf8decf9c8d189.


A conjunctive license can be expressed in RDF via a `<spdx:ConjunctiveLicenseSet>` element, with an spdx:member property for each element in the conjunctive license. Two or more members are required.
FIXME: I don't think this element makes sense. What you want is a way to represent a `simple-expression` based on `WITH`. This subsection seems to be focused on defining exceptions, but it does not cover *consuming* those license exceptions. And “and exception … to which the exception applies” is really not clear; is it the left or right `WITH` operand? What happens for licenses/exceptions that are not in the SPDX License List?
Copy link
Member

Choose a reason for hiding this comment

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

There is another SPDX class that needs to be described. The With operator:

The WITH operator can be expressed in XML via a <spdx:WithExceptionOperator> element, with an spdx:member property for the license and an spdx:licenseException property for the exception.

 <spdx:WithExceptionOperator>
      <spdx:member rdf:resource="http://spdx.org/licenses/GPL-2.0"/>
      <spdx:licenseException rdf:resource="https://spdx.org/licenses/FLTK-exception">
  </spdx:WithExceptionOperator>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The WITH operator can be expressed in XML via a <spdx:WithExceptionOperator> element…

That makes sense to me. But I expect we want to drop the <spdx:LicenseException> element, because as far as I can tell we provide no other way for authors to define exceptions. They can only use exceptions from our list, or they can define a LicenseID-* that includes both the license and the exception. But I see nowhere outside of this appendix that suggests they can define their own exceptions.

Copy link
Member

Choose a reason for hiding this comment

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

The reference for the <spdx:LicenseException> is still of value for those wanting to parse the details out of the exception. We may want to move this to a different section, however, so we don't confuse the reader as to whether they can define license exceptions. Feel free to remove it for now and we can add it back in when we document the other fields.

simple-expression = license ["+"] [space "WITH" space license-exception-id]
binary-compound-operator = "AND" / "OR"
license-expression = simple-expression
/ license-expression space binary-license-operator space license-expression
Copy link
Member

Choose a reason for hiding this comment

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

binary-license-operator -> binary-compound-operator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

binary-license-operator -> binary-compound-operator

I'll go with binary-expression-operator because the arguments are license-expressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in cf8decf9c8d189.


compound-expression = 1*1(simple-expression /
Tag:value authors SHOULD use `enclosed-license-expression` instead of the more general `license-expression`.
Copy link
Member

Choose a reason for hiding this comment

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

SHOULD -> MUST - this is a rule we agreed to to enable machine parsing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SHOULD -> MUST - this is a rule we agreed to to enable machine parsing

But 2.1 uses:

For the Tag:value format, any license expression that consists of more than one license identifier and/or LicenseRef, should be encapsulated by parentheses

I think making this stricter without a major version bump is a bit dicey, especially with wild exceptions that don't set those enclosing parens.

On the other hand, the tag:value format is currently poorly spec'ed, so I guess we can always tell broken docs and tools “sorry, but you should have known you were making assumptions about the eventual format once it was spec'ed out”.

Copy link
Member

Choose a reason for hiding this comment

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

@wking Good point - I would like to "correct" the 2.1 spec.

This was highly debated at the time, but I believe there was a consensus to require the parenthesis. I do know that my parser would break without the parenthesis and I believe the syntax is ambiguous making it unreliable for any implementation of a parser for tag/value @kestewart - let me know if you agree we can change this to "must".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do know that my parser would break without the parenthesis and I believe the syntax is ambiguous making it unreliable for any implementation of a parser for tag/value…

Parsers can be patched, but the data they're parsing may be historical (like the wild example I link above). I'm interested in having new parsers be able to parse as many existing declarations as is reasonably possible. I'm less worried about breaking existing parsers.

Copy link
Member

Choose a reason for hiding this comment

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

@wking This was discussed and resolved during 2.1 development that tag/value files need parenthesis if the expression is complex. I consider it a defect in the spec that this wasn't worded as a must. The reference to parsers is just pointing out the implications of not documenting what was agreed to.

Since you disagree, why don't you keep it as should and I'll create my own pull request after yours is complete to change it to must. That way it will not appear that you are requesting a change that you disagree with. We can then pick up the debate on the new pull request.

Copy link
Member

Choose a reason for hiding this comment

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

IMHO In most cases parens are not needed and just make the expression heavier and ugly looking for no good reason. I think that rather than mandating them, we should relax their use in upcoming spec versions.

Also in the common case it never makes sense to have multi-line expressions .... so we should IMHO state that expression MUST be a single line. In doing so you make things cleaner and simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMHO In most cases parens are not needed and just make the expression heavier and ugly looking for no good reason.

“We want an unambiguous way to identify the end of this value” is a good reason. Requiring (or at least strongly recommending) enclosing parens is one way to do that. Defining clear folding-whitespace rules (#44, #45) is another way. Forbidding newlines in values is a third way. All of those approaches have pros and cons, especially in the context of (backwards) compatibility.

And debating this particular issue was not a priority for this PR. Just tell me what I need to say here to get this landed, and we can come back and kick this particular point around in follow-up work (e.g. #44 and #45).


### IV.3.3 Conjunctive `AND` Operator <a name="IV.3.3"></a>

A [conjunctive license](#IV.2.3) can be expressed in XML via a `<spdx:ConjunctiveLicenseSet>` element, with an `spdx:member` property for each element in the conjunctive license. Two or more members are required.

<spdx:ConjunctiveLicenseSet>
<spdx:member rdf:resource="http://spdx.org/licenses/GPL-2.0"/>
<spdx:ExtractedLicensingInfo rdf:about="http://example.org#LicenseRef-EternalSurrender">
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a missing spdx:member which should enclose the ExtractedLicensingInfo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in cf8decf9c8d189.

wking added a commit to wking/spdx-spec that referenced this pull request Oct 20, 2017
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 [1].

* I've added Gary's documentation for spdx:OrLaterOperator [2];
  previously there was no way to represent the + operator in RDF/XML.

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 a hole around the WITH operators.
I've left a FIXME where I think we need adjustments.

The HTML comment avoids the ambiguous four-space indent after the
list.  Without the comment, it could be parsed as a code block (which
is what we want) [3] or a second paragraph of the final list entry [4]
(which is not what we want).  The HTML comment closes the list to
resolve the ambiguity.

[1]: spdx#22 (comment)
[2]: spdx#37 (comment)
[3]: https://daringfireball.net/projects/markdown/syntax#precode
     "To produce a code block in Markdown, simply indent every line of
     the block by at least 4 spaces or 1 tab"
[4]: https://daringfireball.net/projects/markdown/syntax#list
     "Each subsequent paragraph in a list item must be indented by
     either 4 spaces or one tab"
@wking
Copy link
Contributor Author

wking commented Oct 20, 2017

I've pushed cf8decf9c8d189 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 added a commit to wking/spdx-spec that referenced this pull request Oct 20, 2017
This is a large diff, but I aimed for restructuring/polishing without
changing the end result (much).  I did make a few 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 [1].

* I've added Gary's documentation for spdx:OrLaterOperator [2];
  previously there was no way to represent the + operator in RDF/XML.

* I've added Gary's documentation for spdx:WithExceptionOperator [3].
  I think it's a bit odd that the XML operator represetation are using
  URLs instead of the SPDX IDs that the license expression syntax
  calls for.  That means you cannot convert between the two
  representations without an ID <-> URL map.  But we can address that
  later.

* I've removed spdx:LicenseException, because we currently provide no
  other way for authors to define license exceptions.  We do define a
  way for them to define their own licenses [4], and currently authors
  have to use that to give a LicenseRef to a license+exception pair if
  their exception is not in our list.  Gary feels like we may return
  to this later (and I'd be happy giving users a way to define their
  own exceptions), but we're removing it for now [5].

The HTML comment avoids the ambiguous four-space indent after the
list.  Without the comment, it could be parsed as a code block (which
is what we want) [6] or a second paragraph of the final list entry [7]
(which is not what we want).  The HTML comment closes the list to
resolve the ambiguity.

[1]: spdx#22 (comment)
[2]: spdx#37 (comment)
[3]: spdx#37 (comment)
[4]: https://github.com/spdx/spdx-spec/blob/cfa1b9d08903befdf03e669da6472707b7b60cb9/chapters/6-other-licensing-information-detected.md#6.1
[5]: spdx#37 (comment)
[6]: https://daringfireball.net/projects/markdown/syntax#precode
     "To produce a code block in Markdown, simply indent every line of
     the block by at least 4 spaces or 1 tab"
[7]: https://daringfireball.net/projects/markdown/syntax#list
     "Each subsequent paragraph in a list item must be indented by
     either 4 spaces or one tab"
@wking
Copy link
Contributor Author

wking commented Oct 20, 2017

I've pushed 9c8d189922031a 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.

@goneall
Copy link
Member

goneall commented Oct 21, 2017

@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.

@pombredanne
Copy link
Member

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?

@wking
Copy link
Contributor Author

wking commented Nov 14, 2017 via email

wking added a commit to wking/spdx-spec that referenced this pull request Dec 28, 2017
This is a large diff, but I aimed for restructuring/polishing without
changing the end result (much).  I did make a few 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 [1].

* I've added Gary's documentation for spdx:OrLaterOperator [2];
  previously there was no way to represent the + operator in RDF/XML.

* I've added Gary's documentation for spdx:WithExceptionOperator [3].
  I think it's a bit odd that the XML operator represetation are using
  URLs instead of the SPDX IDs that the license expression syntax
  calls for.  That means you cannot convert between the two
  representations without an ID <-> URL map.  But we can address that
  later.

* I've removed spdx:LicenseException, because we currently provide no
  other way for authors to define license exceptions.  We do define a
  way for them to define their own licenses [4], and currently authors
  have to use that to give a LicenseRef to a license+exception pair if
  their exception is not in our list.  Gary feels like we may return
  to this later (and I'd be happy giving users a way to define their
  own exceptions), but we're removing it for now [5].

* I've fleshed out the documentation for the + operator to explain how
  it works with the AGPL-1.0.  Without this explaination, I think
  there's a risk that folks misinterpret ${ROOT}-${BASE_VERSION}+ as
  "allows ${ROOT}-${VERSION} for any ${VERSION} >= ${BASE_VERSION}",
  but that's not true.  Instead the proper interpretation is "allows
  ${ROOT}-${VERSION} and any other licenses allowed by ${ROOT-VERSION}
  which are based on 'any later version' grant".  For example, if the
  AGPL-2.0 had not been released, you could distribute AGPL-1.0+ code
  under the GPL-3.0+, but *not* under the AGPL-3.0+.

The HTML comment avoids the ambiguous four-space indent after the
list.  Without the comment, it could be parsed as a code block (which
is what we want) [6] or a second paragraph of the final list entry [7]
(which is not what we want).  The HTML comment closes the list to
resolve the ambiguity.

[1]: spdx#22 (comment)
[2]: spdx#37 (comment)
[3]: spdx#37 (comment)
[4]: https://github.com/spdx/spdx-spec/blob/cfa1b9d08903befdf03e669da6472707b7b60cb9/chapters/6-other-licensing-information-detected.md#6.1
[5]: spdx#37 (comment)
[6]: https://daringfireball.net/projects/markdown/syntax#precode
     "To produce a code block in Markdown, simply indent every line of
     the block by at least 4 spaces or 1 tab"
[7]: https://daringfireball.net/projects/markdown/syntax#list
     "Each subsequent paragraph in a list item must be indented by
     either 4 spaces or one tab"
@wking
Copy link
Contributor Author

wking commented Dec 28, 2017

I've pushed 922031aeafa739 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 added a commit to wking/spdx-spec that referenced this pull request Dec 28, 2017
This is a large diff, but I aimed for restructuring/polishing without
changing the end result (much).  I did make a few 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 [1].

* I've added Gary's documentation for spdx:OrLaterOperator [2];
  previously there was no way to represent the + operator in RDF/XML.

* I've added Gary's documentation for spdx:WithExceptionOperator [3].
  I think it's a bit odd that the XML operator represetation are using
  URLs instead of the SPDX IDs that the license expression syntax
  calls for.  That means you cannot convert between the two
  representations without an ID <-> URL map.  But we can address that
  later.

* I've removed spdx:LicenseException, because we currently provide no
  other way for authors to define license exceptions.  We do define a
  way for them to define their own licenses [4], and currently authors
  have to use that to give a LicenseRef to a license+exception pair if
  their exception is not in our list.  Gary feels like we may return
  to this later (and I'd be happy giving users a way to define their
  own exceptions), but we're removing it for now [5].

* I've fleshed out the documentation for the + operator to explain how
  it works with the AGPL-1.0.  Without this explaination, I think
  there's a risk that folks misinterpret ${ROOT}-${BASE_VERSION}+ as
  "allows ${ROOT}-${VERSION} for any ${VERSION} >= ${BASE_VERSION}",
  but that's not true.  Instead the proper interpretation is "allows
  ${ROOT}-${VERSION} and any other licenses allowed by ${ROOT-VERSION}
  which are based on 'any later version' grant".  For example, if the
  AGPL-2.0 had not been released, you could distribute AGPL-1.0+ code
  under the GPL-3.0+, but *not* under the AGPL-3.0+.

The HTML comment avoids the ambiguous four-space indent after the
list.  Without the comment, it could be parsed as a code block (which
is what we want) [6] or a second paragraph of the final list entry [7]
(which is not what we want).  The HTML comment closes the list to
resolve the ambiguity.

[1]: spdx#22 (comment)
[2]: spdx#37 (comment)
[3]: spdx#37 (comment)
[4]: https://github.com/spdx/spdx-spec/blob/cfa1b9d08903befdf03e669da6472707b7b60cb9/chapters/6-other-licensing-information-detected.md#6.1
[5]: spdx#37 (comment)
[6]: https://daringfireball.net/projects/markdown/syntax#precode
     "To produce a code block in Markdown, simply indent every line of
     the block by at least 4 spaces or 1 tab"
[7]: https://daringfireball.net/projects/markdown/syntax#list
     "Each subsequent paragraph in a list item must be indented by
     either 4 spaces or one tab"
This is a large diff, but I aimed for restructuring/polishing without
changing the end result (much).  I did make a few 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 more-recently-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 a paragraph addressing casing, based on discussion in
  [1].

* 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 [2].

* I've added Gary's documentation for spdx:OrLaterOperator [3];
  previously there was no way to represent the + operator in RDF/XML.

* I've added Gary's documentation for spdx:WithExceptionOperator [4].
  I think it's a bit odd that the XML operator represetation are using
  URLs instead of the SPDX IDs that the license expression syntax
  calls for.  That means you cannot convert between the two
  representations without an ID <-> URL map.  But we can address that
  later.

* I've removed spdx:LicenseException, because we currently provide no
  other way for authors to define license exceptions.  We do define a
  way for them to define their own licenses [5], and currently authors
  have to use that to give a LicenseRef to a license+exception pair if
  their exception is not in our list.  Gary feels like we may return
  to this later (and I'd be happy giving users a way to define their
  own exceptions), but we're removing it for now [6].

* I've fleshed out the documentation for the + operator to explain how
  it works with the AGPL-1.0.  Without this explaination, I think
  there's a risk that folks misinterpret ${ROOT}-${BASE_VERSION}+ as
  "allows ${ROOT}-${VERSION} for any ${VERSION} >= ${BASE_VERSION}",
  but that's not true.  Instead the proper interpretation is "allows
  ${ROOT}-${VERSION} and any other licenses allowed by ${ROOT-VERSION}
  which are based on 'any later version' grant".  For example, if the
  AGPL-2.0 had not been released, you could distribute
  AGPL-1.0-or-later code under the GPL-3.0-or-later, but *not* under
  the AGPL-3.0-or-later.

The HTML comment avoids the ambiguous four-space indent after the
list.  Without the comment, it could be parsed as a code block (which
is what we want) [7] or a second paragraph of the final list entry [8]
(which is not what we want).  The HTML comment closes the list to
resolve the ambiguity.

[1]: spdx#63
[2]: spdx#22 (comment)
[3]: spdx#37 (comment)
[4]: spdx#37 (comment)
[5]: https://github.com/spdx/spdx-spec/blob/cfa1b9d08903befdf03e669da6472707b7b60cb9/chapters/6-other-licensing-information-detected.md#6.1
[6]: spdx#37 (comment)
[7]: https://daringfireball.net/projects/markdown/syntax#precode
     "To produce a code block in Markdown, simply indent every line of
     the block by at least 4 spaces or 1 tab"
[8]: https://daringfireball.net/projects/markdown/syntax#list
     "Each subsequent paragraph in a list item must be indented by
     either 4 spaces or one tab"
@wking wking force-pushed the license-expression-cleanup branch from 649889f to afc5c1e Compare June 5, 2018 19:19
@wking
Copy link
Contributor Author

wking commented Jun 5, 2018

I've pushed 649889f -> afc5c1e, rebasing onto master (around conflicts with eafa739 and b971d8d). 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.

@iamwillbar
Copy link
Collaborator

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

@goneall
Copy link
Member

goneall commented Jan 23, 2020

@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.

@iamwillbar iamwillbar added this to the 3.0 milestone Jan 23, 2020
@wking
Copy link
Contributor Author

wking commented Jan 23, 2020

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

@goneall
Copy link
Member

goneall commented Jan 29, 2020

@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.

@kestewart
Copy link
Contributor

Closing this as stale. If disagree, please reopen.

@goneall
Copy link
Member

goneall commented Apr 3, 2024

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

@goneall goneall closed this Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants