purescript / pursuit

Website for hosting and searching PureScript API documentation
https://pursuit.purescript.org/
Other
170 stars 47 forks source link

Update Pursuit for PureScript 0.14 #428

Closed thomashoneyman closed 3 years ago

thomashoneyman commented 3 years ago

This PR updates Pursuit to compile with PureScript 0.14. There are still some checks that need to happen:

thomashoneyman commented 3 years ago

@hdgarrood I've been able to verify that things look OK with this version of Pursuit. Below are some steps to reproduce the tests I ran through.

Verify existing database

First I verified that the updated Pursuit can load the entire set of Pursuit backups. I cloned the whole database into data/verified and made sure I could click around the modules:

Pursuit loads database

The packages I inspected manually look just fine, but I can check particular ones if you'd like.

Verify publishing with 0.13.8 and 0.14.0 compilers

First, I applied this patch to Pulp to help me test publishing packages to the local instance of Pursuit implemented in this pull request:

diff --git a/src/Pulp/Publish.purs b/src/Pulp/Publish.purs
index 77cd9dd..af5e2d2 100644
--- a/src/Pulp/Publish.purs
+++ b/src/Pulp/Publish.purs
@@ -309,7 +309,7 @@ readTokenFile = do

 pursuitUrl :: String -> Version -> String
 pursuitUrl name vers =
-  "https://pursuit.purescript.org/packages/" <> name <> "/" <> Version.showVersion vers
+  "http://localhost:3000/packages/" <> name <> "/" <> Version.showVersion vers

 uploadPursuitDocs :: Outputter -> String -> Buffer -> Aff Unit
 uploadPursuitDocs out authToken gzippedJson = do
@@ -331,8 +331,9 @@ uploadPursuitDocs out authToken gzippedJson = do

   reqOptions = fold
     [ HTTP.method := "POST"
-    , HTTP.protocol := "https:"
-    , HTTP.hostname := "pursuit.purescript.org"
+    , HTTP.protocol := "http:"
+    , HTTP.hostname := "localhost"
+    , HTTP.port := 3000
     , HTTP.path := "/packages"
     , HTTP.headers := headers
     ]

and ran npm run build to get a new pulp.js I could execute. Then I deleted the prelude package:

Verify prelude is gone

Then I verified I can publish the package with 0.13.8:

λ purs --version
0.13.8

λ node ../pulp.js publish --no-push
Checking your package is registered in purescript/registry... ok
Publishing purescript-prelude at v4.1.2. Is this ok? [y/n] y
* Uploading documentation to Pursuit...
* Done.
* You can view your package's documentation at: http://localhost:3000/packages/purescript-prelude/4.1.2

and made sure that this package ends up in the local /data/verified. It does get recorded with 'compilerVersion: 0.14.0' despite using the 0.13.8 compiler for the upload, but I suppose that's Pursuit annotating based on its library dependency on PureScript.

λ tail -c 27 data/verified/purescript-prelude/4.1.2.json | head -c 26
"compilerVersion":"0.14.0"

Here's the output:

Published with 0 13 8

I can also publish with 0.14.0-rc3:

λ purs --version                   
0.14.0-rc3

λ node ../pulp.js publish --no-push
Checking your package is registered in purescript/registry... ok
Publishing purescript-prelude at v4.1.4. Is this ok? [y/n] y
* Uploading documentation to Pursuit...
* Done.
* You can view your package's documentation at: http://localhost:3000/packages/purescript-prelude/4.1.4

and again verify the package ends up in data/verified.

λ tail -c 27 data/verified/purescript-prelude/4.1.4.json | head -c 26
"compilerVersion":"0.14.0"

Once again, the output:

Published with 0 14 0-rc4

Test possible edge cases

I tested your code snippet here by adding it to a new module in prelude:

module Prelude2 where

class X :: (Type -> Type) -> Constraint
class X a

and it seems to render just fine:

Test class renders correctly

though we're not rendering the kind annotations anywhere -- I assume this would be a nice thing to add, though.

hdgarrood commented 3 years ago

Yes, we should really update the HTML documentation generation inside the compiler soon so that code taking advantage of PolyKinds is documented accurately.

That all sounds great apart from one thing:

and made sure that this package ends up in the local /data/verified. It does get recorded with 'compilerVersion: 0.14.0' despite using the 0.13.8 compiler for the upload, but I suppose that's Pursuit annotating based on its library dependency on PureScript.

Pursuit shouldn’t be doing this, the compiler version in the uploaded JSON should contain the compiler version used to create that JSON originally. Are you absolutely sure you used 0.13.8 to produce that JSON? If Pursuit is overwriting that field then that’s a bug we should fix.

thomashoneyman commented 3 years ago

Pursuit shouldn’t be doing this, the compiler version in the uploaded JSON should contain the compiler version used to create that JSON originally. Are you absolutely sure you used 0.13.8 to produce that JSON? If Pursuit is overwriting that field then that’s a bug we should fix.

I’m sure that the compiler in my PATH is 0.13.8 in the first section (which fails to build prelude on master), and 0.14.0-rc3 in the second section. I don’t have the compilers in the same location, so the actual commands I’m running in the second step first have:

PATH=path/to/purs14:$PATH <command>
hdgarrood commented 3 years ago

Ah, I see what's going on. Currently the ToJSON instance for an uploaded package overwrites the current compiler version when encoding to JSON, see https://github.com/purescript/purescript/blob/d056c120821cc11f654cf15071cc0b43e98463f8/src/Language/PureScript/Docs/Types.hs#L758. This will need to be fixed inside the compiler so it's not worth holding this change up for it, I think.