Decide if nested optional and alt tags should be allowed and update schema and/or tools appropriately #462
Comments
|
On Fri, Oct 27, 2017 at 07:24:41PM +0000, goneall wrote:
This should be disallowed.
Previous discussion in #393. To take HPND (#457) as an explicit
example, the current [1]:
<optional>, and that the name <optional>of</optional>
<alt match=".*" name="copyrightHolder0"><copyright holder></alt>
<optional>
<alt match=".*" name="orRelated"><or related entities></alt>
</optional>
not be used in advertising or publicity pertaining to distribution of the software without specific, written
prior permission</optional>.
would need to be rephrased to be a series of <alt> blocks (or a single
<alt> with a fairly complicated regex, or something). If we decide to
go with the no-nesting approach anyway, I'd rather see #457 addressed
before adjusting the schema, to make sure we know what we're getting
into ;).
[1]: https://github.com/spdx/license-list-XML/blob/e5da40e25becb0aa7626d3f62649d2387284a623/src/HPND.xml
|
|
Although the nested alt and optional tags was part of the discussion for issue #393 , I would like to separate out the issue of allowing formatting and allowing nested optional and alt tags. I do not understand the expected behavior of the above example. What is the expected behavior of a nested alt text inside an optional block? How is it to be used in a matching scenario.? |
|
On Sat, Oct 28, 2017 at 12:29:20AM +0000, goneall wrote:
What is the expected behavior of a nested alt text inside an
optional block? How is it to be used in a matching scenario.?
Lets simplify the HPND case to:
Permission to use this software is hereby granted, provided that the
above copyright notice appear<optional> and that the name
<optional>of</optional> <alt match=".*"
name="copyrightHolder0"><copyright holder></alt> not be
used</optional>.
That will match:
Permission to use this software is hereby granted, provided that the
above copyright notice appear.
if the outer optional block is skipped entirely. It will also match:
Permission to use this software is hereby granted, provided that the
above copyright notice appear and that the name Alice not be used.
If the inner optional “of” is skipped and “Alice” is used for
copyrightHolder0. It will also match:
Permission to use this software is hereby granted, provided that the
above copyright notice appear and that the name of Bob not be used.
If the inner optional “of” is included and “Bob” is used for
copyrightHolder0. It will not match:
Permission to use this software is hereby granted, provided that the
above copyright notice appear and that you mail me $5.
where “and that you mail me $5” would be substantive text not covered
by the pattern. To cover the same case without nesting, you'd need
something like:
Permission to use this software is hereby granted, provided that the
above copyright notice appear<alt match="…"
name="optionalNameBlock"> and that the name of <copyright
holder> not be used</alt>.
with something pretty hairy in that ‘match’ attribute.
|
|
@wking Thanks for the description. Makes sense. Challenging to implement in the tools, however. I'll investigate the effort to support, but I believe it will be difficult to add support in the SPDX tools prior to release. Of course, contributions to the SPDX tools to implement are welcome. |
goneall
changed the title
|
The tools implementation to support nesting of optional and var would be quite involved and may even be impractical. The challenge is that most of the regexes are "greedy" and would normally consume the rest of the license text. To solve for this, in the SPDX tools, I search for remaining text that matches the license template and narrow down the text used in the regex to only the code not already matched. Adding nested optional and var tags would significantly complicate this and make some pretty complex and somewhat ugly code even uglier. |
By differentiating between formattedAltText..., which may contain <alt> and <optional>, and formattedFixedText..., which may not. The optionalType is using formattedAltTextGroup, because nesting variable content inside an optional element can be useful [1,2]. The <alt> element, on the other hand, specifies all the variable options in its match attribute, so there's no need for nesting variable elements inside the alt body. [1]: spdx#393 [2]: spdx#462
|
On Sun, Oct 29, 2017 at 07:45:42PM -0700, goneall wrote:
To solve for this, in the SPDX tools, I search for remaining text
that matches the license template and narrow down the text used in
the regex to only the code not already matched.
This sounds like the best approach to me.
Adding nested optional and var tags would significantly complicate
this and make some pretty complex and somewhat ugly code even
uglier.
Can you provide more details on this? It seems like you could
approach this recursively by replacing any optional element with a .*
regexp, regardless of whether an optional tag contained optional or
alt children. In my above example [1], that would be:
Permission to use this software is hereby granted, provided that the above copyright notice appear.*\.
Then if that matches the target text, you can recurse into the
optional element, matching against the empty string (<optional> text
not used) or:
and that the name.* .* not be used
If *that* matches, then recursing into the optional element, we'd
match the first .* against the empty string or ‘of ’.
Or something like that ;).
[1]: #462 (comment)
|
|
We also need to update the template language to support any nesting we agree on. Adding nesting to the The example above has nesting inside the |
I don't think so, and this is what #475 is about. |
By differentiating between formattedAltText..., which may contain <alt> and <optional>, and formattedFixedText..., which may not. The optionalType is using formattedAltTextGroup, because nesting variable content inside an optional element can be useful [1,2]. The <alt> element, on the other hand, specifies all the variable options in its match attribute, so there's no need for nesting variable elements inside the alt body. Also require fixed text for <notes>, since those aren't templates. And drop the unused notesType, since they're vanilla formatted text. We almost certainly want to drop titleText and copyrightText fro the formatted*TextGroup choices, but I'm leaving that for [3]. [1]: spdx#393 [2]: spdx#462 [3]: spdx#452
Doing a recursive matching would be a good approach, but would require some significant design and code changes in the current implementation. The current matching code can be found at https://github.com/spdx/tools/blob/a1d1fb79e78ca4ee737b104e9f8174908e99d0fb/src/org/spdx/compare/LicenseCompareHelper.java#L508 The license template parser is implemented using an interface ILicenseTemplateOutputHandler. This interface assumes that optional text is just text and does not have an internal structure. If we changed the design of the interface, all other implementations of this interface would also have to change. This type of change is certainly possible, but I would estimate about a week's worth of work to make the changes and properly test. |
|
The SPDX tools have now been updated to support nested optional. Based on the discussions and email threads, no one seems to object to changing the spec to allow var and optional tags inside optional tags. Pull request made to the spec Allow embedded rules for rule type optional for license templates to make this change. Closing this as resolved. |
Now that this is settled, we can probably remove the “Nested Optional” label from our label list. That makes it easier to find the label you need when labelling new issues. |
Good suggestion - it is now removed |
As noted by @wking in issue #457 (comment), the schema currently allows for nested optional tags. The SPDX tools currently disallows any nesting of alt or optional tags.
Either the schema or the tools should be updated once we decide if this should be allowed.
The text was updated successfully, but these errors were encountered: