Reimplement spago to manifest code

[JordanMartinez]

Fixes the first part of purescript/spago#1122 (comment):

1. Reimplement spago.yaml parsing in the registry (just the sections we care about)

[JordanMartinez]

I'm guessing I need to add yaml as an NPM dependency somewhere, but not sure how to do that.

[thomashoneyman]

Thanks for taking this on, @JordanMartinez! I have a few comments, and @f-f should take a look, but on the whole I think it's a good direction. I already commented on some of this, but we should add tests to the app test that parse yaml strings into the spago.yaml config format so we have some verification that our parsing and errors are good. We could also consider adding tests to the foreign workspace that just verify we're parsing YAML strings correctly so that if we update the yaml library in the future we've got assurances it's all still working.

[JordanMartinez]

Since I opened this PR before I read over #672, I'm actually thinking this is the wrong approach to take and my idea about storing purs.json file within the git tag's annotation would be a better solution.

The concerns I have here are:

• needing to depend on a backend-specific library (e.g. yaml).
• out-of-sync issues with the package manager's config file. While I copied over this code and refactored it a bit here, this could still be slightly different than what Spago does.

I'd rather have the package managers consume their own config files and produce a purs.json file for us via some api subcommand (e.g. spago api spago-to-purs). However, that requires us to depend on (possibly multiple versions of) different package managers. So, this is also not ideal.

[f-f]

I'm actually thinking this is the wrong approach

I think this approach is by far the least disruptive, and my understanding is that we have reached a consensus that this is a good way to move forward. (see PR comments from Thomas anticipating having multiple config parsers explicitly)

Re your concerns:

1. The registry is already backend-specific and linux/macOS specific, we are not aiming for wide compatibility at this point in time
2. we'd figure out mismatches with the upstream pretty quickly as users will have their packages rejected if the parsing is not correct. Moreover, having the original config and the parser implemented here allows us to eventually reimport the package (this would go via a Trustee patch, see Allowed versions and Trustees publishing #80)

I'd rather have the package managers consume their own config files and produce a purs.json file for us via some api subcommand

I'd rather not. As I tried to make clear in the whole #672 thread, I am of the opinion that passing data out-of-band is a terrible idea and a good way to dig a ditch similar to the one we're trying to climb out of with the Registry.

[f-f]

Re this PR: it looks good so far, and I think it's good to go once Thomas' comments are addressed

[thomashoneyman]

I'd rather have the package managers consume their own config files and produce a purs.json file for us via some api subcommand

I'd rather not. As I tried to make clear in the whole #672 thread, I am of the opinion that passing data out-of-band is a terrible idea and a good way to dig a ditch similar to the one we're trying to climb out of with the Registry.

I think @JordanMartinez meant that we support Spago configs by relying on spago directly in the registry, and we call spago api spago-to-purs to generate a purs.json file rather than implement codecs in the registry code.

That said, the Spago config is simple enough that I prefer to decode it directly rather than use a CLI. (Not that this would be the case for all configs we can handle; we shell out to Dhall, for example, for spago.dhall files.)

[concern:] needing to depend on a backend-specific library (e.g. yaml).

As @f-f said, I don't consider this a problem. I can see an argument for making the registry library backend-independent, since it can be depended on directly by package managers, but the registry application explicitly runs on Node on Linux. We already bind to plenty of Node-specific libraries for that reason.

[concern:] out-of-sync issues with the package manager's config file.

we'd figure out mismatches with the upstream pretty quickly as users will have their packages rejected if the parsing is not correct. Moreover, having the original config and the parser implemented here allows us to eventually reimport the package

I agree with @f-f here that I'm not super worried about mismatches in the config parsing. If we can't parse the config then the package is rejected and it's up to the package manager to get a patch merged that fixes the config; they can always fall back to committing a purs.json file in the meantime. I'm struggling to envision a way our parsing would succeed, but the resulting manifest would be different from what the author intended.

[JordanMartinez]

Ok, but I'll have to get to this another time. Likely not today.

[thomashoneyman]

Sounds good, @JordanMartinez. I'm happy to pair on this to get it through quickly, if you'd like.

[thomashoneyman]

Here's the tracking for adding files (now includes/excludes) and owners to our spago.yaml parsing — #594

[thomashoneyman]

cc: @JordanMartinez this is ready for a look if you'd like to verify it.

[JordanMartinez]

@thomashoneyman Looks good to me!