Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
thomashoneyman opened this issue Nov 26, 2023 · 0 comments · Fixed by #676
Closed
Labels
alpha bug Something isn't working help wanted Extra attention is needed

Comments

@thomashoneyman
Copy link
Member

thomashoneyman commented Nov 26, 2023

See purescript/registry#422 (comment) 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:

Manifest manifest <-

Later, we check if the manifest package / version has been published to the registry, but the docs haven't been published to pursuit:

for_ (Operation.Validation.isNotPublished (Manifest manifest) (Metadata metadata)) \info -> do

If so, we eventually move forward with publishing to Pursuit:

publishToPursuit { packageSourceDir: packageDirectory, compiler: payload.compiler, resolutions: verifiedResolutions, dependenciesDir }

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:

else if hasSpagoYaml then do
-- We need to write the generated purs.json file, but because spago-next
-- already does unused dependency checks and supports explicit test-only
-- dependencies we can skip those checks.
Run.liftAff $ writeJsonFile Manifest.codec packagePursJson (Manifest manifest)
publishRegistry
{ source
, manifest: Manifest manifest
, metadata: Metadata metadata
, payload
, publishedTime
, tmp
, packageDirectory
}

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:

Spec.describe "API pipelines run correctly" $ Spec.around withCleanEnv do

@thomashoneyman thomashoneyman added bug Something isn't working help wanted Extra attention is needed alpha labels Nov 26, 2023
@f-f f-f closed this as completed in #676 Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant