Update Individual values for NONE and NOASSERTION licenses #456

goneall · 2023-07-31T23:24:58Z

This PR implements NONE and NOASSERTION licenses as individuals.

It follows the recommended format documented in issue #299

Note: It may require changes to the spec-parser before merging.

goneall · 2023-07-31T23:48:19Z

Fixes #204

goneall · 2023-08-01T00:10:42Z

Resolves #76

maxhbr · 2023-08-01T05:16:28Z

model/ExpandedLicensing/Vocabularies/NoAssertionLicense.md

+## Description
+
+NoAssertion should be used if the SPDX creator has attempted to but cannot reach a reasonable objective determination;
+the SPDX creator has made no attempt to determine this field; or


as discussed in #289, compared to SPDX2 we have also null (or not including any licensing information) as an value and it should be clearly distinct from NoAssertion. I think I understood that NoAssertion should only be valid if and only if "the SPDX creator has intentionally provided no information".

I think we need to align this with #289 (comment)

To my understanding, Null is the same as NONE, so not sure why null was introduced.

To my understanding, Null is the same as NONE, so not sure why null was introduced.

that would mean that not saying anything is equivalent to explicitly saying "I checked and there is no license". But that is probably a discussion for #289.

The comment implies changing the definitions for the properties, not the NoneLicense and NoAssertionLicense.

@maxhbr if you agree, I can add a suggested text change to the properties along with this PR.

@kestewart - see the comments in issue #289 for more context

@maxhbr if you agree, I can add a suggested text change to the properties along with this PR.

yes, that would match what was discussed (although I am still not happy with it).

Since the 2.3 Noassertion now maps to two values (not present and noassertion in 3.0), we need to do a choice regarding migration. Maybe that should also be added to the description as a comment.

@maxhbr @kestewart I added proposed text to concludedLicense.md and declaredLicense.md - please review

model/ExpandedLicensing/Vocabularies/NoAssertionLicense.md

maxhbr · 2023-08-01T05:25:12Z

model/ExpandedLicensing/Vocabularies/NoAssertionLicense.md

+## Metadata
+
+- name: NoAssertionLicense
+- type: License


I think this should extend AnyLicenseInfo or maybe ExtendableLicense. It then would also not have a licenseText, that would need to be overwritten with weird text.

Agree - I'll include a fix in the next update

zvr · 2023-08-05T07:19:30Z

@goneall why are the files in the Vocabularies directory and not in the Individuals as they already are?

goneall · 2023-08-05T17:10:33Z

@goneall why are the files in the Vocabularies directory and not in the Individuals as they already are?

@zvr - Individuals? Is this a separate directory? If you prefer me to create a new directory called Individuals, I can move them there. I'll move them to wherever the spec-parser expects to see them. Just confirm.

BTW - I think of the enumerations also as individuals, hence the choice of putting them in the Vocabularies directory.

zvr · 2023-08-06T08:43:23Z

@goneall, I mean, it's already there in the main branch: https://github.com/spdx/spdx-3-model/tree/main/model/ExpandedLicensing/Individuals

Yes, Vocabularies also generate individuals, but that's one for each list item, while these are more fleshed out... Let's not mix our parsing, please.

goneall · 2023-08-06T17:40:36Z

I mean, it's already there in the main branch

I missed that - looks like @armintaenzertng already had the individuals added

I'll more these changes over to the individuals folder

goneall · 2023-08-06T17:50:45Z

I moved the Individual definitions over to the Individuals folder.

Here's a summary of the differences from @armintaenzertng original:

Updated descriptions - I copied these from some of @swinslow original descriptions and from the SPDX 2.3 spec with appropriate modifications
Change metadata id to name - name is consistently used through other metadata (e.g. for Classes, Properties, and Vocabularies)
Replaced the property text with property name - AnyLicenseInfo does not have a text property whereas name is inherited from Element

goneall · 2023-08-06T18:13:27Z

@maxhbr @zvr @armintaenzertng @swinslow - I just created a profile Licensing which just contains the descriptions for concluded and declared licenses in this draft PR.

Once we figure out how to define restrictions on relationships, we can add them in this profile.

Please let me know if you disagree with this approach.

maxhbr · 2023-08-16T16:32:06Z

model/Licensing/Licensing.md

+* the SPDX data creator has intentionally provided no information (no meaning
+  should be implied by doing so).
+
+If a declaredLicense is not present or a null value, no conclusion can be drawn.


How would we migrate a null in SPDX2.3 Json? It would need to be mapped to NOASSERTION, right?

@maxhbr - I would leave it as null - I don't think we're changing the semantics?

for 2.3 NOASSERTION is equal to null, since there was no null in the model. We are introducing it with 3.0. So we should treat an explicit NOASSERTION equal to the shorthand null.

for 2.3 NOASSERTION is equal to null, since there was no null in the model.

In 2.3 we made the license properties optional, so I believe they can be null. That is how I'm interpreting a missing value in the JSON file.

If we have a JSON file where a license field is not present in 2.3, we should likewise have it not present in 3.0. I don't see a need to convert it to a NOASSERTION.

Is the issue with the text or a null value? If so, we can remove it.

Sorry for going in circles here. In 2.3 the properties were optional (cardinality 0 to 1) but the following text was added to the description:

If the Declared License field is not present for a package, it implies an equivalent meaning to NOASSERTION.

this effectively collapses the the "not present" and the "NOASSERTION" case again, for 2.3. In my recollection this was purely added to allow for a shorter serialization and the Intend was not to make the model more expressive.

Got it.

I don't know if we intentionally changed the semantics of declared license. I still think if there is no value in 2.3, we should create a NoAssertionLicense relationship in 3.0.

@swinslow - any thoughts?

agree, mapping "not present" and the "NOASSERTION" both to the NoAssertionLicense Relationship is consistent for me.

Note that in 3.0 the author of SPDX data can still choose not to have a declared license, so not mapping it to a NoAssertionLicense would be OK too. We just need to decide what to document in the migration guide.

I have a call with @swinslow this afternoon to sync up - I'll check with Steve and update the issue.

@maxhbr - Since this is specific to 2.3 migration and would only be documented in the migration guide IMO, can you approve this PR?

This raises a question: I'm not sure what "not present" means, now that Declared License is a Relationship rather than a property. "Not present" in this particular SPDX document, BOM, bundle, namespace, ...? (Apologies as I'm not certain which term to use here!)

I believe the purpose of decoupling license values from the underlying software artifacts was in part to make it so that anyone could more easily create licensing values (via Relationships) for software artifacts that someone else had defined as SPDX Elements previously.

Given that, I'm not sure that a "null value" really has relevance anymore, unless I'm misunderstanding. Perhaps this line should just be deleted.

This raises a question: I'm not sure what "not present" means, now that Declared License is a Relationship rather than a property. "Not present" in this particular SPDX document, BOM, bundle, namespace, ...? (

Couple of thoughts:

Not present would just indicate that there is no declaredLicense relationship type found from a SoftwareArtifact to a license - no matter what was serialized - if you don't find it, it isn't present.

There is no null value in the model itself - either a relationship is present or it isn't. There is a null value if we treat it as a property.

I'll remove the null value reference.

swinslow

@goneall Thanks! I've included a few comments inline in this PR.

swinslow · 2023-08-17T20:11:52Z

model/ExpandedLicensing/Individuals/NoAssertionLicense.md

@@ -3,19 +3,21 @@ SPDX-License-Identifier: Community-Spec-1.0
 # NoAssertionLicense

 ## Summary
+An Individual Value for License when no assertion can be made about it's actual value.


minor nit: "it's" => "its"

Updated per suggestion

swinslow · 2023-08-17T20:12:43Z

model/ExpandedLicensing/Individuals/NoAssertionLicense.md


 ## Description

-A NoAssertionLicense is the value that is used to indicate that the data creator
-is making no assertion about the license information for the corresponding software artifact.
+NoAssertion should be used if the SPDX creator has attempted to but cannot reach a reasonable objective determination;


For the first word, "NoAssertion" or "NoAssertionLicense" or "NOASSERTION"?

I'd think "NoAssertionLicense" if it's referring to the Individual's title, or "NOASSERTION" if it's referring to the concrete string value.

Updated to use NoAssertionLicense

swinslow · 2023-08-17T20:13:35Z

model/ExpandedLicensing/Individuals/NoneLicense.md

@@ -3,19 +3,18 @@ SPDX-License-Identifier: Community-Spec-1.0
 # NoneLicense

 ## Summary
+An Individual Value for License no license is present.


I think there's a missing bit: e.g. "for License where the SPDX data creator determines that no license is present"

Updated - along the same lines as the suggestion

swinslow · 2023-08-17T20:14:48Z

model/ExpandedLicensing/Individuals/NoneLicense.md


 ## Description

-A NoneLicense is the value that is used to indicate
-the absence of license information from a software artifact.
+The SPDX document creator concludes there is no license available for this package


SPDX document creator, or SPDX data creator? I'm assuming "data" creator since I gather for 3.0 this might not be in the context of an SPDX document.

I would probably change "package" to "software artifact" since this could also be used for Files or Snippets

Updated per suggestions

swinslow · 2023-08-17T20:16:25Z

model/Licensing/Licensing.md

+
+## Description
+
+The Licensing profile only contains additional restrictions.


What is this meant to be saying?

I probably wouldn't use this phrasing -- "additional restrictions" in the open source licensing context is often used to mean e.g. provisions in license texts that impose additional requirements or prohibitions.

I clarified this to be related to the model

swinslow · 2023-08-17T20:20:49Z

model/Licensing/Licensing.md

+(Classes and Properties associated with string license expressions) and in the ExpandedLicensingProfile (Classes and Properties used for a
+fully parsed syntax tree of license expressions).
+
+There are 2 relationships which are required for any Software Artifact - declaredLicense and concludedLicense.


This isn't what we had decided would be required for the Licensing profile as a requirement during the 2020/2021 discussions. Only concludedLicense was required.

That said, I'm not clear what it means for it to be "required" in any case where concludedLicense is a relationship now, and where I don't think documents are required?

I'm not sure how other profiles are handling requirements like this, where relationships might exist anywhere in SPDX data not tied to a particular document. Are other profiles scoping it to e.g. "in order to conform with a profile, there must be an SPDX Document which contains a relationship for each of these items"?

And sorry, just to clarify -- only concludedLicense was required for the SPDX data to conform to the Licensing profile.

On your first point - I fixed it to only state the requirement was on the concluded license.

On you second point, I opened issue #463 to track. This is targeted to be resolved after RC2 whereas these descriptions should be updated prior to release of RC2.

swinslow · 2023-08-17T20:22:33Z

model/Licensing/Licensing.md

+
+There are 2 relationships which are required for any Software Artifact - declaredLicense and concludedLicense.
+
+A declaredLicense is the license identified in text in the software package,


I think this is missing a pretty key piece from the "summary" part of the prior text, which isn't there now but is visible in the PR that removed it, at https://github.com/spdx/spdx-3-model/pull/448/files#diff-7c90a0506e9d8122276df9c2e1d92a0115bc693c4f2cc53c95fab28c7be943f1L7-L8:

A declaredLicense "[i]dentifies the license information actually found in the software Package, File or Snippet, for example as detected by use of automated tooling."

The "as detected by use of automated tooling" is pretty important to make it clear that this is where information about detected licenses should be expressed.

Updated per suggestion.

goneall · 2023-08-17T23:21:46Z

Thanks @swinslow for the reviews.

I think I updated everything per review comments and then some. It would be worth another look since there were a lot of changes and I may have missed something.

Also - based on one of your comments, I wonder if I copy/pasted from the wrong source on the Licensing.md file. Let me know if you see anything else that needs to be fixed.

outdated review

goneall · 2023-08-27T18:34:38Z

@swinslow @kestewart @maxhbr - I updated the PR with all the suggestions - if you could review and, assuming you don't find any new issues, I'll merge.

maxhbr

LGTM

Signed-off-by: Gary O'Neall <[email protected]>

Co-authored-by: Maximilian Huber <[email protected]>

Signed-off-by: Gary O'Neall <[email protected]>

goneall requested review from zvr and swinslow July 31, 2023 23:24

goneall marked this pull request as draft July 31, 2023 23:25

goneall mentioned this pull request Jul 31, 2023

Licensing: Where should contextual meanings of NONE / NOASSERTION be stated? #204

Closed

goneall mentioned this pull request Aug 1, 2023

Handling NONE and NOASSERTION values in a consistent manner #76

Closed

maxhbr previously requested changes Aug 1, 2023

View reviewed changes

maxhbr reviewed Aug 1, 2023

View reviewed changes

model/ExpandedLicensing/Vocabularies/NoAssertionLicense.md Outdated Show resolved Hide resolved

maxhbr reviewed Aug 1, 2023

View reviewed changes

kestewart added the Profile:Licensing Licensing Profile and related matters label Aug 1, 2023

kestewart added this to the 3.0-rc2 milestone Aug 1, 2023

goneall changed the title ~~Add Individual values for NONE and NOASSERTION licenses~~ Update Individual values for NONE and NOASSERTION licenses Aug 6, 2023

goneall marked this pull request as ready for review August 12, 2023 18:22

maxhbr reviewed Aug 16, 2023

View reviewed changes

swinslow reviewed Aug 17, 2023

View reviewed changes

jlovejoy mentioned this pull request Aug 17, 2023

Reinserting deleted text for concludedLicense and declaredLicense #466

Closed

maxhbr approved these changes Aug 27, 2023

View reviewed changes

goneall and others added 3 commits August 27, 2023 11:50

Add Individual values for NONE and NOASSERTION licenses

2c2c064

Signed-off-by: Gary O'Neall <[email protected]>

Change type of None and NoAssertion License to AnyLicenseInfo

fe6eaa0

Signed-off-by: Gary O'Neall <[email protected]>

Update model/ExpandedLicensing/Vocabularies/NoAssertionLicense.md

b6a691d

Co-authored-by: Maximilian Huber <[email protected]>

goneall added 2 commits August 27, 2023 11:54

Move individuals back to individuals directory

1c5c6a4

Signed-off-by: Gary O'Neall <[email protected]>

Move the concluded and declared license doc to Licensing profile

e5aeae9

Signed-off-by: Gary O'Neall <[email protected]>

goneall force-pushed the issue299 branch from c51ded7 to e5aeae9 Compare August 27, 2023 19:00

goneall merged commit 00a8448 into main Sep 1, 2023
1 check passed

goneall deleted the issue299 branch September 1, 2023 22:04

goneall mentioned this pull request Sep 1, 2023

Proposal: Add definitions for Individuals #299

Closed

swinslow mentioned this pull request Mar 19, 2024

If license information is optional, we do not need NoAssertion in the model #289

Closed


		## Description

		The Licensing profile only contains additional restrictions.


		There are 2 relationships which are required for any Software Artifact - declaredLicense and concludedLicense.

		A declaredLicense is the license identified in text in the software package,

Update Individual values for NONE and NOASSERTION licenses #456

Update Individual values for NONE and NOASSERTION licenses #456

Conversation

goneall commented Jul 31, 2023

goneall commented Jul 31, 2023 • edited Loading

goneall commented Aug 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zvr commented Aug 5, 2023

goneall commented Aug 5, 2023

zvr commented Aug 6, 2023

goneall commented Aug 6, 2023

goneall commented Aug 6, 2023

goneall commented Aug 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swinslow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goneall commented Aug 17, 2023

goneall commented Aug 27, 2023

maxhbr left a comment

Choose a reason for hiding this comment

goneall commented Jul 31, 2023 •

edited

Loading