Skip to content

Conversation

@f-f
Copy link
Member

@f-f f-f commented Jan 19, 2026

This PR stems from a note I left in here:

As an aside, we should patch memoisedGetPackageDependencies to respect the offline flag, since something that is not cached might hit the network. We can push the failures even lower in the stack, but I don't know off the top of my head if that would cause trouble (e.g. if we gate all the Spago.Registry functions with it). The Spago.Git module has it at least, we just haven't propagated in all the low-level places that hit the network since we usually handle it at high level.

..so this PR does just that. We add offline-ness checking a little deeper in the stack so that we don't accidentally hit the network.

@f-f f-f requested a review from fsoikin January 19, 2026 09:57
Copy link
Collaborator

@fsoikin fsoikin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool. I left some whenM/unlessM suggestions, but not really important, just reducing the boilerplace.

Also: what about testing this new behavior?

pure setVersions
else do
logDebug $ "Package sets directory does not exist at " <> Path.quote Paths.packageSetsPath
pure []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure this should return empty in this case rather than dying. What would be upstack behavior when no versions are available?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function is in a where block inside the getRegistryFns function, and the next thing that happens after it is a die in case the registry is not there. So I don't think this case ever happens, but I guess calling die is a sure way to ensure that we do the right thing 🙂

Co-authored-by: Fyodor Soikin <name.fa@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants