Binary I/O versions of readProcess, readProcessWithExitCode, etc.

It would be useful to have versions of readProcess etc. that support binary IO on stdin/stdout.
Proposed solution: a new module (intended to be imported qualified alongside System.Process) with the following signature:

module System.Process.Binary where
import Data.ByteString.Lazy (ByteString)
readProcess :: FilePath -> [String] -> ByteString -> IO ByteString
readCreateProcess :: CreateProcess -> ByteString -> IO ByteString
readProcessWithExitCode :: FilePath -> [String] -> ByteString -> IO (ExitCode,ByteString,ByteString)
readCreateProcessWithExitCode :: CreateProcess -> ByteString -> IO (ExitCode,ByteString,ByteString)

Additionally, I propose adding a new constructor CreateBinaryPipe to the StdStream data type (CreateBinaryPipe would be to CreatePipe as openBinaryFile is to openFile).

If there are no objections, I'd be happy to implement this myself and submit a patch. Note that it would require adding bytestring as a package dependency.

[snoyberg]

This was discussed previously in #36.

Thanks for pointing out #36, which I hadn't spotted. I see that you got quite close to adding this feature but then a decision was made to drop it due to the presence of process-extras.

However, I'd like to try giving this another shot if it's okay. In my opinion, process-extras is not a satisfactory solution for the following reasons:

1. The process-extras implementation of readProcess (et al.) duplicates code that is already in 'process'. This makes maintenance difficult because any bug fix or other change in the 'process' version of readProcess will not automatically make its way into 'process-extras' and vice-versa.

2. process-extras and a number of its dependencies are outside of the Haskell Platform. I think that a binary version of readProcess is important enough that it really ought to go in the Haskell Platform (I argue a case for this below).

Why this matters: a motivating example

Consider a typical everyday scripting task: computing the SHA256 hash of an SSH public key. With a binary version of readProcess this can be done elegantly as follows:

example :: IO ()
example =
  -- Extract base64 public key from file
  fmap ((!! 2) . words) (readFile "key.pub")
  >>=
  -- Convert from base64 to binary
  B.readProcess "base64" ["-d"] . toUtf8
  >>=
  -- Compute sha256sum and convert from hex to binary
  fmap (fromHex . head . words . fromUtf8) .
       B.readProcess "sha256sum" ["-b"]
  >>=
  -- Convert from binary to base64
  fmap fromUtf8 . B.readProcess "base64" []
  >>=
  -- Print
  putStr

where the following are easily-written utility functions:

toUtf8 :: String -> ByteString
fromUtf8 :: ByteString -> String
fromHex :: String -> ByteString

Without a binary readProcess in the Haskell Platform, simple scripting tasks like this are no longer so straightforward and Haskell loses its edge over the conventional bash/sed/awk approach - which I think is a great pity!

Solutions to the outstanding issues from tdammers' original proposal

Taking note of the points discussed in #36, I would make these two improvements to tdammers' design:

1. Rather than copy-pasting code, use a private type class as follows to allow the text and binary versions of readProcess et. al. to share their implementation (similar to the approach used in 'process-extras'):

class NFData s => StringIO s where
  isBinaryString :: s -> Bool -- False for String, True for ByteString
  null :: s -> Bool
  hGetContents :: Handle -> IO s
  hPutStr :: Handle -> s -> IO ()

2. As was pointed out in the discussion of Added bytestring variants for readProcess and friends #36, use lazy rather than strict Bytestring. This makes sense because:

• it gives better compatibility with the 'binary' package (i.e. Data.Binary) which supports lazy ByteString only;
• hGetContents with strict ByteString gives undesirable memory allocation behaviour;
• the existing implementions of readProcess et al. are designed to work with lazy String, so it is trivial to adapt them to lazy ByteString but non-trivial to adapt them to strict ByteString (this issue arose in tdammers' implementation).

As for the issue of the NFData instance for ByteString: this was added in bytestring-0.10.0. I see two reasonable solutions (please say which you prefer):
either A) use 0.10.0 as the lower bound for the bytestring dependency of 'process'
or B) use 0.9.2 as the lower bound for the bytestring dependency of 'process', and add an NFData instance for ByteString within 'process' protected by #if !MIN_VERSION_bytestring(0,10,0).

Submitted pull request #61.

I've submitted pull request #63 for the 'CreateBinaryPipe' part of this proposal.

[jml]

@snoyberg Looking at your comments on #61 and #63, it seems as if you're broadly against this change. Since @robjhen (here) and @ndmitchell (here) have both provided (IMO compelling) reasons for including binary/bytestring APIs in process, I'm curious, what are your objections?

[snoyberg]

All discussions already occurred in #36, I haven't seen any new information added.

[ndmitchell]

@snoyberg The tickets are somewhat disjointed, but reading them all, as far as I can tell:

• Should the process library have bytestring functions, as discussed at Added bytestring variants for readProcess and friends #36. Conclusion was no.
• Should the process library support binary streams, as discussed at Binary I/O versions of readProcess, readProcessWithExitCode, etc. #59 with a pull request at Issue #59: added CreateBinaryPipe constructor to StdStream. #63. @snoyberg says no, but ticket remains open.

People seem to often be proposing ByteString as a solution for binary pipes. To me that's wrapping up two things (encoding and string type). I'd really like binary streams, since implementing it on top of the library is seriously ugly and it seems pretty fundamental. I don't care about bytestrings.

If the answer is no to this ticket, I suggest closing #59 and #63, to make it clear to all.

[snoyberg]

I'm pretty strongly opposed to the idea of pushing changes to encourage doing binary I/O using Strings: it's contrary to current best practices, and doesn't play at all nicely with Handles. I guess we just disagree on this point then. I agree with closing the issues; I've mostly been leaving them open to let people follow the breadcrumb trail in the future of why we haven't made these changes, but apparently the breadcrumbs haven't been clear enough.

[ndmitchell]

Perhaps an FAQ entry? Given how many times it has come up, and to be honest, the position that it's all about binary I/O over String's wasn't immediately clear to me after reading all the breadcrumbs.

[snoyberg]

To be fair, it wasn't clear to me at all that the requests were about String until you implied it right now. I honestly have not had any understanding until now as to why people were asking for this feature when Data.ByteString.hGet works correctly.

As far as a FAQ entry, I'm probably not going to write it. The discussions until now have proven that if I recommend what I think is the right thing to do, it's more than likely just going to generate more controversy.