search

realworldocaml / mdx

Execute code blocks inside your documentation
ISC License
269 stars 45 forks source link

Rewrite redirect code with pipe and cloexec #403

Closed MisterDA closed 1 year ago

MisterDA commented 2 years ago

While debugging a problem with mdx, I've taken the liberty of rewriting the redirect code with pipes (instead of a temporary file, a tmpfs on Unix buf there's no tmpfs on Windows), and using non-blocking reads instead of lseek as lseek isn't available on pipes. I've also protected file descriptors with close-on-exec.

I think the code is correct, but I'd be delighted if there's a bigger example on which to assert everything's fine.

polytypic commented 1 year ago

I wish I had found this earlier.

Unfortunately this doesn't work on Windows, because Unix.set_nonblock doesn't work on Windows.

I would suggest that we revert back to the previous implementation.

MisterDA commented 1 year ago

Maybe I just went too far and we can do blocking IO instead? Sorry about the problems this caused you, I forgot about Windows not supporting non-blocking IO on pipes.

talex5 commented 1 year ago

BTW, won't this change make debugging hangs harder? Normally if my MDX code hangs I look in /tmp to see the output so far - does this no longer work on 2.2?

(though I'd be happy with a better solution)

Leonidas-from-XIV commented 1 year ago

Merged #414 to revert to the previous behaviour until we find a better solution. Will be part of 2.2.1.