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

Use a GHCUp monad #437

Closed
hasufell opened this issue Nov 2, 2022 · 10 comments
Closed

Use a GHCUp monad #437

hasufell opened this issue Nov 2, 2022 · 10 comments

Comments

@hasufell
Copy link
Member

hasufell commented Nov 2, 2022

In GitLab by @RebeccaT on Nov 2, 2022, 10:35

The current code is written in a mix of styles; mtl stack, plain IO, interspersed with various effect systems. This makes the codebase challenging to work with and extend. We could replace these with a GHCUp monad (similar to the PandocMonad that most of the pandoc command-line application is written in).

If this sounds like a good direction, I can work on a merge request. Otherwise, feel free to close.

@hasufell
Copy link
Member Author

hasufell commented Nov 2, 2022

In GitLab by @maerwald on Nov 2, 2022, 11:04

There are mostly 4 types of functions in GHCup codebase:

  1. no effects
  2. plain IO
  3. mtl style function (usually IO and MonadReader access)
  4. Excepts over the mtl stack (a beefed up ExceptT)

The idea is that a function communicates exactly what it needs and doesn't use any constraint/effect it doesn't. This is also the reason we use the open sum type with Excepts. We can see most exceptions on type level.

This goes out the window with a transformer newtype. It's a fuzzy interface.

Can you explain exactly what is challenging? My hunch is this is not so much about mtl style effects, but the fact that Excepts with its open sum type is complicated.

@hasufell
Copy link
Member Author

hasufell commented Nov 2, 2022

In GitLab by @RebeccaT on Nov 2, 2022, 23:39

I'm not sure exactly what we gain by declaring constraints/effects in such a granular manner. Sure, you can see what effects a function needs by looking at the types. But what does ghcup actually do with that information? Invariably, it calls logError $ T.pack $ prettyShow e and exits. The effects which add significant complexity to ghcup's internal interface are in reality only ever used as strings.

Having to learn 4 different types of interfaces (pure, IO, mtl, Excepts) as well as how to interoperate between them (nontrivial with monad transformers and Excepts adds to the complexity) in order to make different parts of ghcup talk to each other is challenging, and due to the complexity of the types involved, ghc offers little help when everything doesn't line up just right.

By abstracting those interfaces into a transformer newtype, we will lose some type-level precision, but I don't think we'll lose a meaningful amount of precision, and interfacing between different parts of the application would become more straightforward. It's a command-line application, not a library, after all. We ship an executable, not types. (Does anyone depend on ghcup as a library?)

At the end of the day, this is just, like, my opinion, man, but I wouldn't be surprised if others have been discouraged from contributing to ghcup (particularly low-effort drive-by fixes) due to its complex architecture.

@hasufell
Copy link
Member Author

hasufell commented Nov 3, 2022

In GitLab by @maerwald on Nov 3, 2022, 09:41

The effects which add significant complexity to ghcup's internal interface are in reality only ever used as strings.

This isn't particularly true. We do catch specific errors in various places, e.g.

https://gitlab.haskell.org/haskell/ghcup-hs/-/blob/787edc17af4907dbc51c85e25c490edd8d68b80b/lib/GHCup/Download.hs#L498

That allows us to remove the catched error from the type level list. It is much more powerful than Haskells exception system.

Having to learn 4 different types of interfaces (pure, IO, mtl, Excepts) as well as how to interoperate between them (nontrivial with monad transformers and Excepts adds to the complexity) in order to make different parts of ghcup talk to each other is challenging, and due to the complexity of the types involved, ghc offers little help when everything doesn't line up just right.

Well, but this is Haskell. Isn't it?

I don't see why reducing types in a statically typed language that promotes using types should be a goal.

Wrt mtl errors... that's something you get exposed to in a lot of libraries. And the nice thing here is that GHC, at least sometimes, can warn you about extraneous constraints.

I feel until this point we are in standard Haskell land.

What makes it really complicated is Excepts with its type level list of errors. It breaks type inference hard and has other ergonomic issues that I raised upstream: haskus/packages#32

By abstracting those interfaces into a transformer newtype, we will lose some type-level precision, but I don't think we'll lose a meaningful amount of precision, and interfacing between different parts of the application would become more straightforward. It's a command-line application, not a library, after all. We ship an executable, not types. (Does anyone depend on ghcup as a library?)

It is designed as a library as well. That's why it's uploaded to hackage and the modules are exposed.

I don't think anyone depends on it right now and the API might be too odd to be used properly, but that's not an argument to change the entire external API.

I'm generally not a huge fan of newtype app transformers. It feels like object oriented programming to me. If I could, I'd remove all transformers from my code, but I find all existing effects system dissatisfying.

What we could discuss is whether using ExceptT with an application wide closed error sum type might be more ergonomic than the open sum type mess with Excepts. Also wrt https://gitlab.haskell.org/haskell/ghcup-hs/-/issues/424

At the end of the day, this is just, like, my opinion, man, but I wouldn't be surprised if others have been discouraged from contributing to ghcup (particularly low-effort drive-by fixes) due to its complex architecture.

That's a valid concern to think about.

I wouldn't be too opposed to an RIO architecture though, but that would mean all exceptions stay within Haskells exception system and there's no ExceptT or similar.

@hasufell
Copy link
Member Author

hasufell commented Nov 3, 2022

In GitLab by @phadej on Nov 4, 2022, 04:34

effectful(-core) is reasonable effect system I have used recently. You can have reader, checked exceptions (many of them) and many other effects: https://hackage.haskell.org/package/effectful-core. The plain IO, reader and errors from your list are covered. You can even write pure functions in Eff if you really want to (with runPureEff).

@hasufell
Copy link
Member Author

hasufell commented Nov 4, 2022

In GitLab by @maerwald on Nov 4, 2022, 09:31

Yeah, looks interesting. Here are all the reasons transformers suck: https://github.com/haskell-effectful/effectful/blob/master/transformers.md

But I think that would mean I have to abandon the Excepts with it's open sum type errors?

@hasufell
Copy link
Member Author

hasufell commented Nov 8, 2022

In GitLab by @arjun on Nov 9, 2022, 01:33

i personally find it alright to work with the codebase, it's actually gotten quite a bit better since Julian refactored it last year or so. Like @maerwald said

The idea is that a function communicates exactly what it needs and doesn't use any constraint/effect it doesn't.

I've also grown to appreciate the Type-level list of exceptions listed on top a function, that may however be a personal preference.

That said, I've been intrigued by effectful for some time now. I'd be open to the refactor, but mostly because of the oohh shiny factor of it. Longer term advantages with maintainability and accessibility (new contributors would need to learn a new effects system, its hard with mlt style as is for beginners) remain to be elucidated.

@arjunkathuria
Copy link
Contributor

Revisiting if we'd like to explore an effectful re-write in a near-ish long term @hasufell

@hasufell
Copy link
Member Author

I'm not sure this is very interesting. GHCup is dealing with mainly one thing: the filesystem. And that all the time. We don't really have a lot of interesting abstract effects.

There's a much more interesting thing architecturally, which is:

  1. define phases more clearly and then have all installations follow these phases strictly (unpack, configure, compile, install, post-install etc.)
  2. the phases are not defined ad-hoc, but are separated from the actual tool installation (this is basically what source package managers do). Recipes (e.g. for GHC) instantiate these phases and the package manager carries them out following certain constraints (e.g. the root filesystem cannot be written to in compile phase).
  3. For this, we need a way to filter syscalls, e.g. via seccomp, also see Support seccomp notify? teh/hsseccomp#8

This would be a long way to go to implement a syscall sandbox that works.

But this is real architecture and not just mere monad shuffling.

@arjunkathuria
Copy link
Contributor

Apart from the Filesystem (which is the major things ghcup deals with, as you mentioned), We also do Network and quite a lot of error handling.
Our codebase is currently mostly mtl style, with some IO and interspersed ExceptT

A single style through out, with permissions of a functions and the errors that can arise / it handles in it's type signature would probably be a better place to be for the codebase as a whole. (i think we can also spell out the errors, not as a type level lists, like we do currently, but close). I think it would enable more people to read / look and contribute as well, but that's speculation at this point.

Effectful also tends to be a bit faster than mtl according to the benchs on its page but that's not very interesting for us, though it wouldn't hurt.

This, as you mentioned, would just be a refactor and would keep us functionally equivalent, the time spend here, could also be probably used for a more critical feature / architecture / extended platform support, as you mentioned. I wanted to explore the possibility of the scope here and if / or are we even open to it, not immediately, perhaps eventually.

@hasufell
Copy link
Member Author

The only real benefit I can see from an effects system is that we could abstract over filesystem operations and then have an in-memory filesystem for testing, because we can choose the interpreter for all the primitives.

However, I'm not a very firm believer in this type of testing. But I'm not opposed to it either.

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

No branches or pull requests

2 participants