Adds basic license check #258
Conversation
| log "Checking that the SPDX license in the manifest corresponds to the one in the package" | ||
| licenseFromLicensee <- liftAff $ Licensee.detect absoluteFolderPath | ||
| case licenseFromLicensee of | ||
| Left err -> throwWithComment $ "Could not find a license in the package: " <> err | ||
| Right li -> when (Array.notElem (SPDX.print manifest.license) li) (throwWithComment $ "License from the manifest does not match the license detected by licensee. In manifest: " <> SPDX.print manifest.license <> ". Deteceted in package: " <> show li <> ".") |
There was a problem hiding this comment.
I added a comment in the other PR to the same effect, but this isn't quite enough -- licensee will detect from the LICENSE file, but we want to also pull the licenses out of the manifest files present in the repository in general, all of which is included in the toManifestFields function in the LegacyImport module.
We want to check the Bowerfile, Spago file, package.json file, and the LICENSE file and make sure they all match with the purs.json (manifest) file, but here we're only checking the LICENSE file.
The work to check all those files is done in the legacy import tool already so it's just a matter of pulling it out.
There was a problem hiding this comment.
I think I understand. Lemme write it up here and you can tell me if it is correct or not.
Currently, the API.purs file is looking for a file called purs.json with various bits of information including a license. In addition to this file, we want to parse other manifests which may also contain a license. Currently, the two additional manifest files we will parse are bower.json and spago.dhall + packages.dhall. So, for example, if we were writing it up in a README, it'd look something like this:
License
Packages added to the registry must have a valid SPDX license, and that license must be indicated in a manifest file. We determine license validity using the following steps:
- We look for, and if present parse, the following three manifest files to determine a package's license:
purs.jsonspago.dhallbower.json
- We then use
licenseeto find license files in the package. - We cross-reference the licenses found by
licenseewith the licenses specified in a package's manifest file(s). If there is a mismatch (ie too many licenses, too few licenses or an incorrect license) we will reject the package.
There was a problem hiding this comment.
@thomashoneyman ping regarding this just to make sure I'm on the right track - thanks!
There was a problem hiding this comment.
Yes! Two clarifications:
- We check the manifest file for the license for the project (the purs.json file). We then cross-reference against the other manifest files. In practice this is basically the same as what you’ve written, but until we fully decide what to do about mismatches I think it’s worth calling out that in our minds the purs.json manifest is the “true” one
- Licensee checks the package.json file (if I remember correctly), so we’re checking that too.
There was a problem hiding this comment.
Would it be ok if I added the following?
In the future, we will be using
purs.jsonas the single source of truth for licenses for PureScript packages and will be deprecatingbower.json,package.json, andspago.dhallsupport. We thus encourage all package authors to include their licenses in apurs.jsonfile so as not to be dropped from a future edition of the registry.
There was a problem hiding this comment.
I’m not sure about that — we may always check the other manifests, but in the future you will have to have a purs.json file to publish to the registry. We’re only auto-importing legacy packages
There was a problem hiding this comment.
Packages added to the registry must have a valid SPDX license
Scenario: I have a lawyer and I have a new open source license type (not "UNLICENSED"). I have it documented in a LICENSE file so consumers understand how to comply. Would this make my package unregisterable? Or do I need to petition to get it in SPDX to be valid? Is creating my own license a good practice or idea? Probably not, but should it straight up not be able to be handled? I could see an argument where it should be a Either SPDX CustomAndIKnowWhatImDoing.
There was a problem hiding this comment.
The issue is that we (the registry trustees) also would need to review the license to be sure that it is usable for open source — for example, open enough that we can host the code in the registry backend. Given that our bandwidth is already stretched it’s not something we’d like to take on if we can defer to a widely used standard (SPDX).
If a package does have this requirement in the future, then I’d encourage them to reach out to the registry trustees to see if this restriction can be changed at that time, or just not register the package. Package managers will still let you use the package even if it isn’t registered.
|
One of the recommendations in #251 is the following:
..which means that we'd still need something like this check, right? |
|
Yea, we still want to have a check in the package upload pipeline that just verifies that the SPDX identifier you've asserted in your package manifest checks out against other manifest files / LICENSE files present. We won't fix it for you, but we'll reject packages that have an identifier in their |
@thomashoneyman I have a draft PR with something along the lines of what we talked about in the registry meeting, but as I was putting it together I realized that the solution may be more simple than what we had discussed. This PR is my attempt to do that simple thing. If it is missing something, then let me know and I can add more logic.