Legacy spago.dhall imports overconstrain package dependencies.

[natefaubion]

The current Spago assumes that a single spago.dhall contains both src and test dependencies, by way of the default "sources" globs and test command. The current legacy importer takes these dependencies at face value, resulting in overconstrained package dependencies. For example, many of my projects are portable libraries, but their test framework relies on node dependencies. This recently caused issues when purescript-node released breaking changes across the org, which ended up booting my projects out of the package set even though they continue to compile fine given accurately constrained dependencies. I assume that this could be quite common across the ecosystem, since my project configurations are merely the default recommended setup from stable Spago.

Stable Spago is quite pedantic in the accuracy of dependencies (noting both missing and unused dependencies), so perhaps we could amend the legacy importer for spago.dhall files to prune test dependencies by doing the "unused dependency" check on src globs only. This would presumably yield a set of dependencies that are unnecessary for installing the package.

[thomashoneyman]

This is how spago-legacy checks for unused dependencies:
https://github.com/purescript/spago-legacy/blob/11e68ece50ede286660452ef370baa3ab5c33c02/src/Spago/Build.hs#L65-L67

In the registry we produce JSON from the spago.dhall / packages.dhall alone here:

registry-dev/app/src/App/Legacy/Manifest.purs

Lines 329 to 334 in 6a803c3

	fetchSpagoDhallJson
	:: forall r
	. Address
	-> RawVersion
	-> Run (GITHUB + LOG + AFF + EFFECT + r) (Either GitHubError SpagoDhallJson)
	fetchSpagoDhallJson address ref = Run.Except.runExceptAt _spagoDhallError do

And we then write that manifest as part of the publishing process here:

registry-dev/app/src/App/API.purs

Lines 427 to 437 in 6a803c3

	Legacy.Manifest.fetchLegacyManifest payload.name address (RawVersion payload.ref) >>= case _ of
	Left manifestError -> do
	let formatError { error, reason } = reason <> " " <> Legacy.Manifest.printLegacyManifestError error
	Except.throw $ String.joinWith "\n"
	[ "Could not publish your package because there were issues converting its spago.dhall and/or bower.json files into a purs.json manifest:"
	, formatError manifestError
	]
	Right legacyManifest -> do
	Log.debug $ "Successfully produced a legacy manifest from the package source. Writing it to the package source..."
	let manifest = Legacy.Manifest.toManifest payload.name version existingMetadata.location legacyManifest
	Run.liftAff $ writeJsonFile Manifest.codec packagePursJson manifest

I don't want to fetch the full project source when producing the legacy manifest, but I think we could introduce a step after fetching the legacy manifest and before writing it as a purs.json file in which we do a quick fixup with purs graph. We have access to the project source at the point we write the legacy manifest (the code above), because of this line:

registry-dev/app/src/App/API.purs

Line 364 in 6a803c3

{ path: packageDirectory, published: publishedTime } <- Source.fetch source tmp existingMetadata.location payload.ref

An ideal solution, to my mind, would be to write a function that can take the src directory and a legacy manifest and prune that manifest such that unused dependencies are removed. We can then write independent tests for that function and then add it as a fixup step in the publish pipeline.

[f-f]

@thomashoneyman we now have a whole module for making sense of purs graph - the checkImport function will collect the unused packages, which we could prune from the list we have.

[thomashoneyman]

Ah, that's convenient! From a brief scan it looks like unused indeed skips test globs, so it looks like we could just use checkImport directly to implement this change — or we could copy/paste the Spago checkImport implementation into the registry and trim it down to only the unused dependency part so as to avoid having to produce a fake workspace package and GraphEnv.

[f-f]

@thomashoneyman we can reuse this code - it's possible to have the registry-app depend on spago-lib (since spago-lib only depends on registry-core and registry-foreign)

[thomashoneyman]

Working on this, should have something up in the next day or so.