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

New exception: SHL-2.1 #1028

Merged
merged 10 commits into from
May 15, 2020
Merged

New exception: SHL-2.1 #1028

merged 10 commits into from
May 15, 2020

Conversation

jlovejoy
Copy link
Member

xml file for SHL-2.1

@jlovejoy
Copy link
Member Author

hmmm, thought I add .txt file to PR, but not seeing it...

@jlovejoy jlovejoy self-assigned this May 13, 2020
@jlovejoy jlovejoy added this to the 3.9 release milestone May 13, 2020
@jlovejoy jlovejoy requested a review from swinslow May 13, 2020 18:03
@jlovejoy
Copy link
Member Author

both files there now, but still failing - sigh

@swinslow
Copy link
Member

I'm on calls for next couple of hours, but I'll take a look :) Thanks for getting these going!

optional tag and standard header tag
@jlovejoy
Copy link
Member Author

@swinslow - it seems to think there's no .txt file and its looking in the exceptions folder - but we've added other exceptions and passed tests?
@goneall might have an insight

@swinslow
Copy link
Member

looking at the test log, in lines 222 and 223 it looks like it's having issues with parsing the XML file as invalid:

Processing License List19:32:45.022 [main] ERROR org.spdx.licensexml.LicenseXmlDocument - Invalid license XML file SHL-2.1.xml
org.xml.sax.SAXParseException: cvc-complex-type.2.4.a: Invalid content was found starting with element '{"http://www.spdx.org/license":standardLicenseHeader}'. One of '{"http://www.spdx.org/license":p, "http://www.spdx.org/license":bullet, "http://www.spdx.org/license":list, "http://www.spdx.org/license":optional, "http://www.spdx.org/license":alt, "http://www.spdx.org/license":br, "http://www.spdx.org/license":titleText, "http://www.spdx.org/license":copyrightText}' is expected.

I'm taking a closer look, will see if I can tell why it might be failing...

@swinslow
Copy link
Member

Ah, I think it might be because the <standardLicenseHeader> section is appearing within the <text> section. Looking at another example I think it might need to be outside. I'm jumping on another call but will experiment afterwards to see if that fixes it...

@jlovejoy
Copy link
Member Author

Ah, I think it might be because the <standardLicenseHeader> section is appearing within the <text> section. Looking at another example I think it might need to be outside.

I don't think it's that, as I have it the same way as in Apache-2.0

@zvr
Copy link
Member

zvr commented May 14, 2020

@jlovejoy @swinslow the two </br> in lines 38 and 39 should probably be <br/> 😉

@swinslow
Copy link
Member

I just pushed another commit which should fix it (at least it's working on the test on my local machine) -- two changes:

  • fixed the br tags -- actually just removed and replaced them with p tags
  • removed the standardLicenseHeader tags

My best guess is that there's something about using standardLicenseHeader in an exception that is causing it to break. From a quick search it doesn't look like any of the other exceptions use this tag. Will wait to confirm that it passes...

Note also that I gather there have been some updates to the upstream text (per discussion in #1027) so we should make sure to capture those too before merging.

Signed-off-by: Steve Winslow <[email protected]>
Copy link
Member

@swinslow swinslow left a comment

Choose a reason for hiding this comment

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

Passing now, hooray :) but marking as needs changes b/c we need to update based on edits from @andrewjskatz. I'm preparing those updates now.

@swinslow
Copy link
Member

Good to go now. @andrewjskatz, could you take a look at https://raw.githubusercontent.com/spdx/license-list-XML/d2397ad0e0dac0c225bceccd65833705853cd3ec/test/simpleTestForGenerator/SHL-2.1.txt and confirm that you are good with locking it in as the 2.1 text?

@andrewjskatz
Copy link

andrewjskatz commented May 14, 2020 via email

@swinslow
Copy link
Member

Looks like we're good to go -- I'm merging this. Thank you @jlovejoy @andrewjskatz @zvr!

@swinslow swinslow merged commit 2d40fed into spdx:master May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants