garnix-io / cradle Goto Github PK

We briefly discussed this already, but I thought I'd bring up the related set of issues that are streaming, returning handles rather than files, and async execution of processes.

One option that we came up with is passing the consumer as an argument. But there are a couple more:

• Having a separate AsyncOutput class. Instead of instances for Exit and Stdout, for example, it might have instances for Async Exit and StdoutHandle. It would, I imagine, be called with a different function that run (e.g., start).
• Having Output instances for Async Exit, StdoutHandle etc, but expanding the class so extractOutput has a signature of Either ProcessStarted ProcessResult -> output instead of just ProcessResult -> output. If any output is async, then we get a Left. This likely isn't safe, in the sense that having e.g. (ExitCode, StdoutHandle) might just be an error.
• Fancy types to make sure sync and async outputs aren't mixed. I feel like this might get messy, but it might also help with other non-sensical combinations (like having two different types of Stdout).

It's probably sensible to just put on all these questions, but I was thinking a bit about this and wanted to just jot down ideas.

Currently the output Outputs (StdoutTrimmed, StdoutUntrimmed, Stderr) return bytestrings. The idea then is that users will call conversion functions themselves.

There are, I think, several problems with this:

• Does it even make sense to talk about "newline" and trimming on binary inputs?
• I believe generally the handles are not set to binary mode (e.g., try doing hGetEncoding)
• Users can't get access to the encoding of the relevant handles, so they don't have an easy way to decide what the encoding even should be
• In general, deciding whether output is binary or not (and what the encoding is) isn't, I think, something to be done after the fact at all, since those are properties of the handle.

What I propose instead is:

• The bytestring function calls hSetBinaryMode True on the handle
• There is only one bytestring function for each output handle, and it does no trimming
• There's a Std(out/err)Text, with trimmed and untrimmed variants. This behaves like the functions in Data.Text.IO; that is, it uses the locale encoding and converts from that (potentially just directly using the Data.Text.IO functions
• Optionally, there are functions like StdoutUtf8 and StdoutLatin1 (with trimmed variants?), which set the encoding of the handle to the respective encoding, and use the corresponding encoding/decoding functions in Data.Text.

It'd be helpful to compare this tool with similar ones:

• process
• typed-process
• turtle
• shelly
• shake

I tried using cradle to write a helper for limiting system resources with systemd, but ran into the problem that only the Cradle module is exposed, and that doesn't expose the Input class method, or the ProcessConfig type. I think those should be re-exported from somewhere. But more importantly, maybe all modules should be exposed, even if with an Internal name? Otherwise anyone who has an unforeseen use case has to fork the library.

For stdout, there's StdoutTrimmed and StdoutUntrimmed. For stderr there's just Stderr (which is untrimmed). Is there a reason for this, or should they be unified.

As I mentioned in another issue, I was hoping to write a SystemdRunLimits Input, and a couple of questions came up:

• 1. Why is Input a class? All of the inputs are just representing ProcessConfiguration -> ProcessConfiguration functions. So they could just be combined via <> rather than anything fancier. E.g.:

run (cmd "mycmd" <> args ["arg1", "arg2"] <> stdinHandle hdl <> workingDir "some-dir")

Type inference would improve, the codebase would be simpler, and the types much easier to read.

• 2. Why is the command (and, I would think, also the arguments) not a "proper" argument to run (instead of an Input)? The disadvantages are that a clearly wrong use of run typechecks but throws a runtime error, and that other Inputs that may want to modify the command (e.g.: a Sudo Input), now have to be run in a very specific order which is opaque to the user (and, to me at least, it's harder to read or understand what each command is doing).

(Combined with 2 the above example would be:

run "mycmd" ["arg1", "arg2"] (stdinHandle hdl <> workingDir "some-dir")

And run :: (MonadIO m, Output a) => String -> [String] -> (ProcessConfig -> ProcessConfig) -> m a (though having a newtype for ProcessConfig -> ProcessConfig might be nice).