libpng-1.6.35: fix missing paren in name

[swinslow]

Just fixing a missing right parenthesis in the license name field.

Signed-off-by: Steve Winslow [email protected]

[xsuchy]

+1

[xsuchy]

I would merge, but I am confused by the failed test.

[swinslow]

Thanks @xsuchy -- same here. I've been staring at it and trying to dig into the license list publisher tool to understand the cause, but unfortunately I don't.

@zvr This is the one that I'm going to reach out to you about momentarily in a separate email. Grateful for any thoughts you might have here...

[swinslow]

Noting for the record that this appears to be the same issue from #2729. Though I think that the specific problem there (with libpng-2.0) was resolved in that thread, in order to get that PR to a working state to merge.

[zvr]

@swinslow that is... tricky. I can confirm that it has nothing to do with the parenthesis you're adding.

I think the issue is, after the license generation, when the license list is being generated, a check for the title is happening...

After a lot of testing, I have a minimal example that also fails:

<?xml version="1.0" encoding="UTF-8"?>
<SPDXLicenseCollection xmlns="http://www.spdx.org/license">
   <license licenseId="myid" name="My License">
      <text>
         <titleText>
            PNG Reference Library License version 789
         </titleText>
         <p>
		 Some text.
         </p>
      </text>
   </license>
</SPDXLicenseCollection>

If you delete or change any word in the titleText (besides changing the version number), no error is produced!!!

For example:

titleText	Result
PNG Reference Library License version 4	FAIL
PNG Reference Library License version 789	FAIL
Reference Library License version 789	No problem
PNG Reference Library License	No problem

I am completely baffled.

For future reference, among the false paths I went down to:

• listVersionAdded is too new (nope)
• the crossRef points to pure text (no matter)
• titleText should have text inside a <p> (some do; some don't)

I would need to check the LicenseListPublisher code (or the SPDX Java lib code, for comparison utilities) to dig deeper into what is happening -- but I'm not a Java programmer.

@goneall Does the above point to anything?

[swinslow]

Thank you @zvr for digging into this!

An additional thing I'm noting, though not sure what to do with it yet:

Looking at the checks, I see that the 'Validate canonical match' task failed for the "pull_request" event for this PR; but it passed for the "push" event. (edited because I got this backwards in my original message)

From the YAML file for this GitHub Action, it looks like the two are handled differently in how they set BASE_REF, and then how they fetch the diff of what's changed.

I haven't sorted through the implications of this yet, but just putting it here so I don't forget as I'm continuing to look into this...

[swinslow]

Tracing through a few things in the workflow logs:

• for the "push" workflow which passes: $BASE_REF gets set to b8019ac4903f931fecd0d75306737c5f06fa0af3
• for the "pull_request" workflow which fails: $BASE_REF gets set to origin/main

This is visible via the "Fetch changes for git diff" and "Run echo ..." drop-downs. (I note that there's a deprecation warning for "set-output" as well, though I'm guessing that may not be related to the present issue.)

The main purpose for these steps appears to be setting the BASE_REF environment variable, which appears to get passed to the Makefile as INPUT_BASE_REF.

This value is then used in the Makefile for a few variables relating to identifying what's changed in the source files. From what I can see, these are in turn only used to create the LICENSE_SOURCE variable, which is then actually passed to the license list publisher command line.

I realize this is just tracing through the GitHub Actions and Makefile (both of which I should probably understand better than I do!), but it may help point towards what specifically is causing this to pass for the "push" action with BASE_REF as the named commit, but fail for the "pull_request" action with BASE_REF as just origin/main.

[goneall]

Back online - I'll see if I can duplicate this on my local machine. There is definitely an issue with the publisher in that it should not generate an NPE.

This may be related to spdx/Spdx-Java-Library#334

[goneall]

I'm able to duplicate the problem - so I should be able to find a solution in the next day or so

[goneall]

I fixed the current issue - there is still a failure caused by a duplicate license. @swinslow I think this is the LGPL duplicate license you already fixed.

This issue was rather complex. There were 2 issues involved.

There's a bug in the license list publisher where an empty optional text will cause an NPE. This is fixed in spdx/Spdx-Java-Library#337.

The reason there is an empty optional text is there is a nested alt text inside the title for the libpng-2.0.xml file. Note this is not the file being edited - it just happened to match enough of the libpng-2.0.xml file to go down the path where the NPE occurs.

This has been fixed in the XML files in this repo. However, the CI will test for duplicates against the published licenses when only changing a single license. This is for performance reasons - we don't have to recompile every XML file to check for duplicates.

I copied over the fixed libpng-2.0.json and libpng-2.0.json-ld files to the website and we now advance to having the duplicate license warnings.

@swinslow - back to you if you can (re) fix the duplicate license issue.