Skip to content
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

Proposal: Reimplement 'purs publish' in the registry #525

Open
thomashoneyman opened this issue Sep 29, 2022 · 7 comments
Open

Proposal: Reimplement 'purs publish' in the registry #525

thomashoneyman opened this issue Sep 29, 2022 · 7 comments
Labels

Comments

@thomashoneyman
Copy link
Member

thomashoneyman commented Sep 29, 2022

I think we should stop using purs publish and instead move that functionality into the registry. purs publish has many restrictions that prevent the registry from working properly, such as having to have a SemVer-compliant Git tag checked out with a clean source tree (except for the purs.json file). To support monorepos or non-GitHub locations we have to lift these restrictions. Rather than iterate on the compiler I think we should just do this in the registry, because then we can report better errors and not be tied in with compiler releases.

purs publish can stay in the compiler for non-registry users – the Pursuit API isn't changing – but I think eventually we will want to deprecate it altogether and have the registry be the direct pipeline to Pursuit.

We can do this because we already have, in the API pipeline:

  1. The dependency resolutions for the package
  2. The purs.json or spago.yaml manifest for the package
  3. The output directory produced by the compiler; we can also generate docs if need be.
@f-f
Copy link
Member

f-f commented Mar 15, 2023

From today's discussion in the Registry Weekly: the side of the compiler that deals with publishing is adjacent to the docs, and that's a big chunk of code that might not be straightforward to port. One of the possibilities that we discussed was to sidestep purs publish entirely, and change Pursuit instead.

@thomashoneyman
Copy link
Member Author

thomashoneyman commented Nov 12, 2023

To re-implement purs publish in the registry all we have to do is construct the Package type used by the compiler and Pursuit. I've explored this in #328 and it's mostly straightforward. I've added the type and JSON encoding here:

-- | The definition of a 'Package' according to Pursuit
--
-- Taken originally from the compiler repository, which is reused in Pursuit:
-- https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/src/Language/PureScript/Docs/Types.hs#L51
--
-- This should be considered a legacy format; we must follow Pursuit's publishing
-- rules for now, but eventually we should relax requirements such as being
-- GitHub-only, or being associated with a specific Git tag on a repository with
-- a README in root (which forbids monorepos).
type PursuitPackage =
{ uploader :: String
, packageMeta :: PursuitPackageMeta
, tagTime :: String
, modules :: Array PursDocs
-- Package names must include the 'purescript' prefix.
, resolvedDependencies :: Map RawPackageName RawVersion
, version :: RawVersion
-- The package ref, usually the version prefixed with 'v' (but not always).
, versionTag :: String
-- Manifest.location (only GitHub allowed) in two parts: [ owner, repo ]
, github :: Array String
}

I omitted fields that aren't actually used by Pursuit, include modulesMap. If that turns out to be used we can easily add it, since we already do this via associateModules in the publish pipeline.

The only type I didn't implement is that of the modules key, which contains the docs.json for each module the package includes or depends on. Ideally we wouldn't have to actually type the entire docs.json format because it's a behemoth — we would type it as Json and just read the file in from disk (the output directory for the package we're publishing). Alas, the docs.json file always sets reExports to an empty array, and those re-exports are only associated during purs publish via this function:

https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/src/Language/PureScript/Docs/Collect.hs#L43

Thus the only real blocker to completing this work is figuring out how to produce the correct list of re-exports. Presumably that means reimplementing collectDocs from the compiler, ideally without typing the whole docs.json format, but perhaps there are other options.

Work is started in https://github.com/purescript/registry-dev/tree/trh/purs-publish. Once we figure out the collectDocs issue we can proceed with producing the PursuitPackage type during the publish pipeline. We have all the other information we need.

@f-f
Copy link
Member

f-f commented Nov 13, 2023

Thanks @thomashoneyman for digging into this! It seems problematic to me that the reExport list is always empty - surely the compiler should know what is being re-exported! I have a strong suspicion that this is the same issue that things like purescript/purescript#4477 are trying to fix (i.e. have each module know that their full external interface is), so if that's the case we should go for fixing the root cause rather than trying to route around it

@thomashoneyman
Copy link
Member Author

thomashoneyman commented Nov 13, 2023

@f-f that would mean not being able to publish packages prior to whatever compiler version contains that fix, though, so we'd have to either institute a cutoff (docs are only published with compiler 0.15.x and afterwards) or route around the issue. I agree it'd be better for the docs files to just have the re-exports associated right away; I don't know the reasoning for it being a later-on fixup step.

@f-f
Copy link
Member

f-f commented Nov 13, 2023

that would mean not being able to publish packages prior to whatever compiler version contains that fix

That's ok? If you are publishing a package you're targeting the latest version of the compiler in the vast majority of cases, so I don't think it would be an issue. Docs that already exist do not need to be generated again either.

I don't know the reasoning for it being a later-on fixup step.

I suspect it was done to route around the issue as well 😄

@thomashoneyman
Copy link
Member Author

Yea, I realize now that we already can't publish packages pre-0.14.7, because that's when I added support for purs.json files to purs publish and relaxed the 'clean working tree' restriction for that file. Bumping that up to a recent compiler is potentially fine, though it might be a surprise for someone on a relatively recent compiler like 0.15.12.

@f-f
Copy link
Member

f-f commented Nov 13, 2023

might be a surprise for someone on a relatively recent compiler

This is currently the first item on a list of things that we need to change to allow the new style of publishing (not bound to GitHub, not bound to git tags, and monorepos) so hopefully by the time we get there it won't be a very recent version anymore 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants