spago icon indicating copy to clipboard operation
spago copied to clipboard

Publish rebuilds with another version

Open flip111 opened this issue 1 year ago • 10 comments

Screenshot from 2023-11-01 22-21-15

normal npx spago build works fine

flip111 avatar Nov 01 '23 21:11 flip111

The publish command is rebuilding your package using the Registry solver, which other users will use when interacting with your package.

As you noted in the screenshot, the version bounds in the configuration allow for the version that is being used in the rebuild.

I believe the logic we are using for inferring the bounds is correct (assuming that all the patch version in a minor release are not going to introduce breakage), and I believe the SemVer of that package is not correct since it introduced breakage.

You should edit your configuration to restrict the bounds from using the breaking version.

f-f avatar Nov 01 '23 21:11 f-f

I believe the logic we are using for inferring the bounds is correct

I also think that is correct. But the rebuild is failing. To rebuild version 0.4.2 should be downloaded. Possibly we should even consider to rebuild in a temporary directory not to mess with the main cache

I believe the SemVer of that package is not correct since it introduced breakage

Which package? arrays-extra or foldable-traversable-extra ?

You should edit your configuration to restrict the bounds from using the breaking version.

This is not the problem because in the new version that function is also there https://pursuit.purescript.org/packages/purescript-arrays-extra/0.4.2/docs/Data.Array.Extra.First#v:modifyOrSnoc the problem is is that the source file is not available for the compiler

flip111 avatar Nov 01 '23 21:11 flip111

Ah, I misread the error in the screenshot - indeed everything else is correct, except that Spago is not installing the missing packages. We should be fetching them before issuing a rebuild.

f-f avatar Nov 01 '23 22:11 f-f

Should this be called from here?

flip111 avatar Dec 24 '23 21:12 flip111

Probably? That would be the first thing I would try. Then I would add a test and see how it goes. (tip for tests: use itOnly to only run a single test instead of having to run the whole test suite)

f-f avatar Dec 25 '23 10:12 f-f

Fetch.run is only called from Main so far. And it doesn't take a package name as direct argument. Probably it's there in some reader/state monad. That's why i didn't know if it would be a good idea to call run directly or rather if it needs to be refactored in separate functions.

flip111 avatar Dec 25 '23 11:12 flip111

Yes, Spago carries a lot of local state through its Reader instance. I think it's fine to call it from other places as long as we pass the right flags in. The way we pass in a "package name" to select is via the workspace.selected field in the FetchEnv. We do things with it in the body of Fetch.run, which I'd recommend reading to get an idea of how to deal with it.

f-f avatar Dec 25 '23 11:12 f-f

To run fetch with a different env than public i was thinking to use this function https://github.com/purescript/spago/blob/master/core/src/Prelude.purs#L73 That function takes an env, main makes such an env with this function https://github.com/purescript/spago/blob/master/bin/src/Main.purs#L933 That function would have to be moved somewhere else if the publish command is to make use of it.

flip111 avatar Dec 25 '23 13:12 flip111

These "env creation" functions are to be considered "expensive" (in term of running time), and are best run only at the beginning of the program, which is why they are sitting in the Main module.

Once you have your hands on a "big enough" environment, you can restrict it to a smaller one - e.g. in the publishing flow we already get a BuildEnv out of a PublishEnv here: https://github.com/purescript/spago/blob/b450892d69aa3013861fde6c73299d2c17aba3dc/src/Spago/Command/Publish.purs#L363-L378

f-f avatar Dec 25 '23 13:12 f-f

that's a good idea

flip111 avatar Dec 25 '23 14:12 flip111