-
Notifications
You must be signed in to change notification settings - Fork 5
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 pipe instead of a temp file #4
Comments
Hey, thanks for the feedback. I'm not the original author of this library and I'm not actively working on it.
@jfischoff if you want to give this a shot then I'm more than happy to accept a patch. My only requirement would be that it works both on *nix and Windows, and that there are tests that demonstrate this. We may want to setup AppVeyor for that. |
cool. I didn’t know about this library and wrote a similar one that uses a
pipe. Those sound like perfectly reasonable requirements.
…On Fri, Jun 1, 2018 at 4:41 PM Simon Hengel ***@***.***> wrote:
Hey, thanks for the feedback.
I'm not the original author of this library and I'm not actively working
on it.
using a pipe
@jfischoff <https://github.com/jfischoff> if you want to give this a shot
then I'm more than happy to accept a patch. My only requirement would be
that it works on that there are tests that demonstrate this. We may want to
setup AppVeyor for that.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAKSOz2Tw4rw9jbOsg9UUxW9jYZMmXDsks5t4dEtgaJpZM4UXZfh>
.
|
Nice! If you are interested to collaborate on this, I'm happy to add anybody as a co-maintainer who made at least one quality contribution. |
I appreciate the offer. In all honesty I’m only 50% certain I’ll put a PR
up at all ... but yes assuming I do :p
…On Fri, Jun 1, 2018 at 6:24 PM Simon Hengel ***@***.***> wrote:
wrote a similar one that uses a pipe
Nice! If you are interested to collaborate on this, I'm happy to add
anybody as a co-maintainer who made at least one quality contribution.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAKSO50I_BRH0QdRxSLiKuhIfqrVBZZCks5t4elJgaJpZM4UXZfh>
.
|
Keep in mind the OS pipe buffers. If a pipe is full, writing to it will block. That doesn't happen for temporary files. If your output is larger than the pipe buffer, and you don't consume from the pipe, then your program will get stuck. |
You need to read from it on another thread. |
@jfischoff Right, but then put it where? You have to put it either onto disk or into memory. If the goal is disk, then doing it through a pipe read by a thread writing it to disk doesn't seem better than writing to a temporary file directly. If the goal is memory, From the issue description it isn't quite clear to me what the issue or goal is:
Robust against what, the disk being full? |
The goal is to avoid writing to disk and then reading from disk and just buffer in memory.
You will have to elaborate on what you mean here.
Yes. Also it just seems unnecessarily complex to write to the disk just to read from disk. |
I imagine people would do that so that they can handle more output than fits into RAM (or, not to occupy that RAM). But I see that indeed silently/src/System/IO/Silently.hs Lines 102 to 103 in 8809d5c
So you're right, that can be achieved more easily without a roundtrip through files. I think going via the disk would be beneficial if
I somehow assumed you were talking about using |
Agree |
For me it makes sense, because I'd like to capture stdout in unit tests. Therefore "not fitting into RAM" isn't the case (due to small amounts of test data). Here the function (inspired by the library): myCapture :: IO a -> IO (String, a)
myCapture action = do
bracket redirect restore runActionAndCapture
where
redirect = do
(pipeReadEnd, pipeWriteEnd) <- createPipe
old <- hDuplicate stdout
hDuplicateTo pipeWriteEnd stdout
return (pipeReadEnd, pipeWriteEnd, old)
runActionAndCapture (pipeReadEnd, _, _) = do
a <- action
hFlush stdout
c <- readAvailable pipeReadEnd
return (c, a)
restore (pipeReadEnd, pipeWriteEnd, old) = do
hDuplicateTo old stdout
hClose old
hClose pipeWriteEnd
hClose pipeReadEnd
readAvailable :: Handle -> IO String
readAvailable h = do
isReady <- hReady h
if isReady
then do
c <- hGetChar h
tail <- readAvailable h
return $ c : tail
else return [] Haven't tested it under Windows, though |
I think it makes sense to have both a file-based interface and at least one pipe-based one. One pipe-based solution could use a second thread to pull output from the pipe, making it available on request. A second one could use the pipe blocking mechanism, producing a stream of output chunks followed by a result. |
I don't see why this library writes to temp file as opposed to using a pipe with a call like this one: https://hackage.haskell.org/package/process-1.6.3.0/docs/System-Process.html#v:createPipe
It seems like a pipe is more robust and less likely to fail.
The text was updated successfully, but these errors were encountered: