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

Publish rebuilds with another version #1110

Closed
flip111 opened this issue Nov 1, 2023 · 10 comments · Fixed by #1283
Closed

Publish rebuilds with another version #1110

flip111 opened this issue Nov 1, 2023 · 10 comments · Fixed by #1283

Comments

@flip111
Copy link
Contributor

flip111 commented Nov 1, 2023

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

normal npx spago build works fine

@f-f
Copy link
Member

f-f commented Nov 1, 2023

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.

@flip111
Copy link
Contributor Author

flip111 commented Nov 1, 2023

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

@f-f
Copy link
Member

f-f commented Nov 1, 2023

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.

@flip111
Copy link
Contributor Author

flip111 commented Dec 24, 2023

Should this be called from here?

@f-f
Copy link
Member

f-f commented Dec 25, 2023

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)

@flip111
Copy link
Contributor Author

flip111 commented Dec 25, 2023

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.

@f-f
Copy link
Member

f-f commented Dec 25, 2023

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.

@flip111
Copy link
Contributor Author

flip111 commented Dec 25, 2023

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.

@f-f
Copy link
Member

f-f commented Dec 25, 2023

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:

runBuild :: _ -> Spago (BuildEnv _) _ -> Spago (PublishEnv _) _
runBuild { selected, dependencies } action = do
env <- ask
runSpago
{ purs: env.purs
, git: env.git
, dependencies: dependencies
, logOptions: env.logOptions
, workspace: env.workspace { selected = Just selected }
, psaCliFlags:
{ statVerbosity: (Nothing :: Maybe Core.StatVerbosity)
, strict: (Nothing :: Maybe Boolean)
}
, pedanticPackages: false
}
action

@flip111
Copy link
Contributor Author

flip111 commented Dec 25, 2023

that's a good idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment