keep manifest decoding backward-compatible

Read stored manifests leniently again so historical SPDX spellings in the registry and registry-index still decode, but keep canonical-only decoding for new manifest input in publish and legacy import paths. Add direct regressions for the historical manifest/index case and document the boundary in the spec and public modules.

Author: thomashoneyman

SPEC.md

@@ -361,7 +361,7 @@ Alternately, this can be written without an id:
 **[Source](./lib/src/Registry/License.purs)**
 **[Spec](./types/v1/License.dhall)**
 
-All packages in the registry must have a license that grants permission for redistribution of the source code. Concretely, the registry requires that all packages use an SPDX license and specify an [SPDX license identifier](https://spdx.dev/ids/). `AND` and `OR` conjunctions are allowed, and licenses can contain exceptions using the `WITH` preposition. The SPDX specification describes [how licenses can be combined and exceptions applied](https://spdx.dev/ids#how).
+All packages in the registry must have a license that grants permission for redistribution of the source code. Concretely, the registry requires that all packages use an SPDX license and specify an [SPDX license identifier](https://spdx.dev/ids/). `AND` and `OR` conjunctions are allowed, and licenses can contain exceptions using the `WITH` preposition. The SPDX specification describes [how licenses can be combined and exceptions applied](https://spdx.dev/ids#how). Newly submitted manifests must use current canonical SPDX identifiers, but registry readers should remain backward-compatible with historical stored manifests that still use deprecated SPDX spellings.
 
 A `License` is represented as a string, which must be a valid SPDX identifier. For example:
 

app/src/App/API.purs

@@ -436,7 +436,7 @@ parseSourceManifest { packageDir, name, version, ref, location } = do
         Left error -> do
           Log.error $ "Manifest does not typecheck: " <> error
           Except.throw $ "Found a valid purs.json file in the package source, but it does not typecheck."
-        Right _ -> case parseJson Manifest.codec string of
+        Right _ -> case parseJson Manifest.canonicalCodec string of
           Left err -> do
             Log.error $ "Failed to parse manifest: " <> CJ.DecodeError.print err
             Except.throw $ "Found a purs.json file in the package source, but it could not be decoded."
@@ -1347,7 +1347,7 @@ validateLicense packageDir manifestLicense = do
           detectedStrings # Array.mapMaybe \detectedLicense ->
             hush do
               canonicalized <- License.canonicalizeDetected detectedLicense
-              License.parse canonicalized
+              License.parseCanonical canonicalized
 
       Log.debug $ "Detected licenses: " <> String.joinWith ", " detectedStrings
 

app/src/App/Legacy/Manifest.purs

@@ -77,7 +77,7 @@ bowerfileToPursJson
 bowerfileToPursJson (Bowerfile { description, dependencies, license }) = do
   let
     -- Best effort: keep any licenses that parse cleanly and drop the rest.
-    validLicenses = Array.mapMaybe (hush <<< License.parse) license
+    validLicenses = Array.mapMaybe (hush <<< License.parseCanonical) license
 
   parsedLicense <-
     case NonEmptyArray.fromArray validLicenses of
@@ -140,7 +140,7 @@ spagoDhallToPursJson
 spagoDhallToPursJson (SpagoDhallJson { license, dependencies, packages }) = do
   parsedLicense <- case license of
     Nothing -> Left "No license found in spago.dhall"
-    Just lic -> case License.parse (NonEmptyString.toString lic) of
+    Just lic -> case License.parseCanonical (NonEmptyString.toString lic) of
       Left _ -> Left $ "Invalid SPDX license in spago.dhall: " <> NonEmptyString.toString lic
       Right l -> Right l
 

lib/src/License.purs

@@ -6,18 +6,22 @@
 -- | must install if you are using parsing code from this module. Please see the
 -- | package.json file for exact versions.
 -- |
--- | `parse` is strict and intended for user-authored SPDX expressions in
--- | registry manifests. `canonicalizeDetected` is intended for SPDX identifiers
--- | emitted by external tooling like `licensee`, and only rewrites deprecated
--- | identifiers when SPDX provides a deterministic canonical replacement.
+-- | `parse` accepts canonical SPDX expressions and historical deprecated SPDX
+-- | expressions when SPDX provides a deterministic canonical replacement. Use
+-- | `parseCanonical` when validating new user-authored manifest input.
+-- | `canonicalizeDetected` is intended for SPDX identifiers emitted by
+-- | external tooling like `licensee`, and only rewrites deprecated identifiers
+-- | when SPDX provides a deterministic canonical replacement.
 module Registry.License
   ( License
   , SPDXConjunction(..)
+  , canonicalCodec
   , canonicalizeDetected
   , codec
   , extractIds
   , joinWith
   , parse
+  , parseCanonical
   , print
   ) where
 
@@ -46,12 +50,22 @@ newtype License = License LicenseTree
 
 derive newtype instance Eq License
 
--- | A codec for encoding and decoding a `License` as JSON
+-- | A codec for encoding and decoding a `License` as JSON.
+-- | This decoder is backward-compatible with historical manifest data.
 codec :: CJ.Codec License
-codec = CJ.named "License" $ Codec.codec' decode encode
+codec = licenseCodec parse
+
+-- | A codec for encoding and decoding a `License` as JSON.
+-- | This decoder accepts only canonical SPDX identifiers and is intended for
+-- | validating newly authored manifest input.
+canonicalCodec :: CJ.Codec License
+canonicalCodec = licenseCodec parseCanonical
+
+licenseCodec :: (String -> Either String License) -> CJ.Codec License
+licenseCodec parseLicense = CJ.named "License" $ Codec.codec' decode encode
   where
   decode :: JSON -> Except CJ.DecodeError License
-  decode = except <<< lmap CJ.DecodeError.basic <<< parse <=< Codec.decode CJ.string
+  decode = except <<< lmap CJ.DecodeError.basic <<< parseLicense <=< Codec.decode CJ.string
 
   encode :: License -> JSON
   encode = print >>> CJ.encode CJ.string
@@ -93,9 +107,19 @@ foreign import currentIds :: Array String
 foreign import deprecatedIds :: Array String
 
 -- | Parse a string as a SPDX license identifier.
--- | This is strict and accepts only current canonical SPDX identifiers.
+-- | This is backward-compatible with historical registry manifests and accepts
+-- | deprecated SPDX identifiers when the canonical replacement is
+-- | deterministic.
 parse :: String -> Either String License
 parse input = do
+  parsedTree <- parseExpressionTree input
+  canonicalTree <- canonicalizeParsedTree canonicalizeDetectedLeaf parsedTree
+  pure $ License canonicalTree
+
+-- | Parse a string as a canonical SPDX license identifier.
+-- | This is intended for validating newly authored manifest input.
+parseCanonical :: String -> Either String License
+parseCanonical input = do
   parsedTree <- parseExpressionTree input
   canonicalTree <- canonicalizeParsedTree canonicalizeStrictLeaf parsedTree
   pure $ License canonicalTree

lib/src/Manifest.purs

@@ -13,6 +13,7 @@
 -- | https://github.com/purescript/registry-index
 module Registry.Manifest
   ( Manifest(..)
+  , canonicalCodec
   , codec
   ) where
 
@@ -70,12 +71,22 @@ instance Ord Manifest where
 
 -- | A codec for encoding and decoding a `Manifest` as JSON. Represented as a
 -- | JSON object. The implementation uses explicitly ordered keys instead of
--- | record sugar.
+-- | record sugar. This decoder remains backward-compatible with historical
+-- | manifests stored in the registry and manifest index.
 codec :: CJ.Codec Manifest
-codec = Profunctor.wrapIso Manifest $ CJ.named "Manifest" $ CJ.object
+codec = manifestCodec License.codec
+
+-- | A codec for encoding and decoding a `Manifest` as JSON that requires
+-- | canonical SPDX identifiers in the license field. This is intended for
+-- | newly authored manifests submitted for publishing.
+canonicalCodec :: CJ.Codec Manifest
+canonicalCodec = manifestCodec License.canonicalCodec
+
+manifestCodec :: CJ.Codec License -> CJ.Codec Manifest
+manifestCodec licenseCodec = Profunctor.wrapIso Manifest $ CJ.named "Manifest" $ CJ.object
   $ CJ.recordProp @"name" PackageName.codec
   $ CJ.recordProp @"version" Version.codec
-  $ CJ.recordProp @"license" License.codec
+  $ CJ.recordProp @"license" licenseCodec
   $ CJ.recordPropOptional @"description" (Internal.Codec.limitedString 300)
   $ CJ.recordProp @"location" Location.codec
   $ CJ.recordProp @"ref" CJ.string

lib/test/Registry/License.purs

@@ -30,7 +30,24 @@ spec = do
         , Array.foldMap (append "\n  - " <<< License.print) success
         ]
 
-  Spec.it "Strict parsing rejects deprecated SPDX identifiers" do
+  Spec.it "Parses and canonicalizes deterministic deprecated SPDX identifiers" do
+    let
+      cases =
+        [ { input: "AGPL-3.0", output: "AGPL-3.0-only" }
+        , { input: "AGPL-3.0+", output: "AGPL-3.0-or-later" }
+        , { input: "GPL-2.0-with-classpath-exception", output: "GPL-2.0-only WITH Classpath-exception-2.0" }
+        , { input: "GPL-2.0+", output: "GPL-2.0-or-later" }
+        , { input: "GFDL-1.3+", output: "GFDL-1.3-or-later" }
+        ]
+
+    for_ cases \{ input, output } ->
+      case License.parse input of
+        Left err ->
+          Assert.fail $ "Expected parse to succeed for " <> input <> ", but failed with: " <> err
+        Right parsed ->
+          Assert.shouldEqual output (License.print parsed)
+
+  Spec.it "Canonical parsing rejects deprecated SPDX identifiers" do
     let
       rejected =
         [ { input: "AGPL-3.0", expectedError: "AGPL-3.0-only" }
@@ -42,9 +59,9 @@ spec = do
         ]
 
     for_ rejected \{ input, expectedError } ->
-      case License.parse input of
+      case License.parseCanonical input of
         Right parsed ->
-          Assert.fail $ "Expected strict parse to reject " <> input <> ", but parsed as " <> License.print parsed
+          Assert.fail $ "Expected canonical parse to reject " <> input <> ", but parsed as " <> License.print parsed
         Left err ->
           unless (String.contains (Pattern expectedError) err) do
             Assert.fail $ "Expected parse error for " <> input <> " to mention " <> expectedError <> ", but got: " <> err

lib/test/Registry/Manifest.purs

@@ -2,8 +2,12 @@ module Test.Registry.Manifest (spec) where
 
 import Prelude
 
+import Codec.JSON.DecodeError as CJ.DecodeError
+import Data.Codec.JSON as CJ
+import Data.Either (Either(..))
 import Data.String as String
 import Data.Traversable (for)
+import JSON as JSON
 import Node.Encoding (Encoding(..))
 import Node.FS.Aff as FS.Aff
 import Node.Path as Path
@@ -21,3 +25,46 @@ spec = do
       rawManifest <- FS.Aff.readTextFile UTF8 $ Path.concat [ manifestFixturesPath, path ]
       pure { label: path, value: String.trim rawManifest }
     Assert.shouldRoundTrip "Manifest" Manifest.codec fixtures
+
+  Spec.it "Decodes historical manifests with deprecated SPDX identifiers" do
+    let input = historicalManifest
+    case JSON.parse input of
+      Left err ->
+        Assert.fail $ "Failed to parse test JSON: " <> err
+      Right json ->
+        case CJ.decode Manifest.codec json of
+          Left err ->
+            Assert.fail $ "Failed to decode historical manifest: " <> CJ.DecodeError.print err
+          Right _ ->
+            pure unit
+
+  Spec.it "Rejects deprecated SPDX identifiers in canonical manifest decoding" do
+    let input = historicalManifest
+    case JSON.parse input of
+      Left err ->
+        Assert.fail $ "Failed to parse test JSON: " <> err
+      Right json ->
+        case CJ.decode Manifest.canonicalCodec json of
+          Left _ ->
+            pure unit
+          Right _ ->
+            Assert.fail "Expected canonical manifest decoder to reject deprecated SPDX license"
+
+historicalManifest :: String
+historicalManifest =
+  String.trim
+    """
+    {
+      "name": "jarilo",
+      "version": "1.0.1",
+      "license": "AGPL-3.0",
+      "location": {
+        "githubOwner": "bklaric",
+        "githubRepo": "purescript-jarilo"
+      },
+      "ref": "v1.0.1",
+      "dependencies": {
+        "prelude": ">=6.0.0 <7.0.0"
+      }
+    }
+    """

lib/test/Registry/ManifestIndex.purs

@@ -45,6 +45,10 @@ spec = do
     let parsedContext = ManifestIndex.parseEntry contextEntry
     contextEntry `Assert.shouldEqualRight` map (ManifestIndex.printEntry <<< NonEmptySet.fromFoldable1) parsedContext
 
+  Spec.it "Parses historical manifest-index entries with deprecated SPDX identifiers" do
+    let parsedHistorical = ManifestIndex.parseEntry historicalAgplEntry
+    historicalCanonicalEntry `Assert.shouldEqualRight` map (ManifestIndex.printEntry <<< NonEmptySet.fromFoldable1) parsedHistorical
+
   Spec.it "Produces correct entry file paths" do
     let
       entries =
@@ -156,6 +160,16 @@ contextEntry =
 {"name":"context","version":"0.0.3","license":"MIT","location":{"githubOwner":"Fresheyeball","githubRepo":"purescript-owner"},"ref":"v0.0.3","dependencies":{}}
 """
 
+historicalAgplEntry :: String
+historicalAgplEntry =
+  """{"name":"jarilo","version":"1.0.1","license":"AGPL-3.0","location":{"githubOwner":"bklaric","githubRepo":"purescript-jarilo"},"ref":"v1.0.1","dependencies":{"prelude":">=6.0.0 <7.0.0"}}
+"""
+
+historicalCanonicalEntry :: String
+historicalCanonicalEntry =
+  """{"name":"jarilo","version":"1.0.1","license":"AGPL-3.0-only","location":{"githubOwner":"bklaric","githubRepo":"purescript-jarilo"},"ref":"v1.0.1","dependencies":{"prelude":">=6.0.0 <7.0.0"}}
+"""
+
 testIndex
   :: forall m
    . MonadThrow Error m