purescript / registry-dev

Development work related to the PureScript Registry
https://github.com/purescript/registry
95 stars 80 forks source link

Write purs.json before calling Pursuit publish on already published package version #675

Closed thomashoneyman closed 9 months ago

thomashoneyman commented 9 months ago

See https://github.com/purescript/registry/issues/422#issuecomment-1825929900 for an example of this error.

In #667 we started to prune unused dependencies from manifests. Accordingly, we stopped immediately writing manifests and instead read them so they can (potentially) be manipulated before they are written. Here's where we read the manifest and previously wrote it: https://github.com/purescript/registry-dev/blob/1d9e28b36964fbbb3e7918ee486adbd0ac0e6a96/app/src/App/API.purs#L402

Later, we check if the manifest package / version has been published to the registry, but the docs haven't been published to pursuit: https://github.com/purescript/registry-dev/blob/1d9e28b36964fbbb3e7918ee486adbd0ac0e6a96/app/src/App/API.purs#L498

If so, we eventually move forward with publishing to Pursuit: https://github.com/purescript/registry-dev/blob/1d9e28b36964fbbb3e7918ee486adbd0ac0e6a96/app/src/App/API.purs#L520

However, at this time, we haven't actually attempted to change the manifest and haven't written it to the package source directory. That doesn't happen until later — for example: https://github.com/purescript/registry-dev/blob/1d9e28b36964fbbb3e7918ee486adbd0ac0e6a96/app/src/App/API.purs#L550-L563

This causes Pursuit publishing to fail in the case there was no purs.json file, as purs publish will exit. To fix this, we need to either:

  1. Move the 'already published' check to the end of the pipeline, after we have potentially modified and written the new purs.json file, or
  2. If we have already published the package version, skip any further processing and read the manifest we previously produced from the registry index, aborting if it isn't there.

The advantage of (1) is that it's very easy to implement. The disadvantage is that if a package is published at one time, and then the registry checks or modifications change, then if the package docs are attempted later on then it's possible the package manifest used for publishing is different (or fails altogether) vs. the original we packaged.

The advantage of (2) is that we skip directly to reusing the manifest we already computed and publish that — no chance of a mismatch, and more efficient. If we're OK with just reading from the registry index then it's also quite easy to implement. If we want to handle the possibility that the manifest is missing in the registry index then it's a little more work (we'd have to download and unpack the tarball we published to get the manifest out of it).

My suggestion is that we use approach (2), reading the already-published manifest from the registry index (unless the source code uses a purs.json, in which case we can just use that). If the manifest cannot be found in the index, abort with an error.

As a stretch goal we should add to the "integration" test in the PureScript tests such that we verify this code path: https://github.com/purescript/registry-dev/blob/1d9e28b36964fbbb3e7918ee486adbd0ac0e6a96/app/test/App/API.purs#L71