dhall format --inplace should force the resulting string before writing it

[ocharles]

I just lost a Dhall file because I ran dhall-format on it and hit a bug. This meant the file just contained a huge list of ( - presumably an infinite loop somewhere - causing me to lose the contents of the file (yet to reach source control). In an ideal world it wouldn't go wrong, but we don't live in an ideal world. I'll take the extra memory usage of buffering the contents first and witnessing the formatting terminate before writing the file.

This is on Dhall 1.15. I can't report a bug as to what went wrong with formatting because I don't have the input anymore 😆

[gilligan]

freeze also does inplace changes so maybe it should apply there as well

[ocharles]

Yep, anywhere we're writing files should do this. I think using strict text might be sufficient, but that might still have the problem of truncating the file if creating that text goes into an infinite loop.

[Gabriella439]

I would like to suggest an alternative solution. Most Unix utilities that do in-place modification support backing up the original file in case anything goes wrong and perhaps the dhall utilities could do the same. The reason I suggest this is that buffering the input only fixes one possible way that the inplace modification can fail whereas keeping a backup file also handles other possible ways that the program might fail.

[sjakobi]

unliftio offers utilities for this use case:

http://hackage.haskell.org/package/unliftio-0.2.12/docs/UnliftIO-IO-File.html#v:withBinaryFileDurableAtomic

[Gabriella439]

@sjakobi: Sure, that seems fine

[sjakobi]

It seems that unliftio's binary mode operations could get us into trouble on Windows. The documentation for openBinaryFile says:

On Windows, reading a file in text mode (which is the default) will translate CRLF to LF, and writing will translate LF to CRLF. This is usually what you want with text files. With binary files this is undesirable; also, as usual under Microsoft operating systems, text mode treats control-Z as EOF. Binary mode turns off all special treatment of end-of-line and end-of-file characters. (See also hSetBinaryMode.)

We could alternatively use the atomic-write package, which doesn't seem to use binary mode, but which also doesn't currently offer durability. How important is durability for us?

Or we could just have custom wrappers around the unliftio operations that use text mode on Windows – unliftio's functions default to the ordinary withBinaryFile on Windows anyway.

[Gabriella439]

@sjakobi: I don't believe there are any issues with binary mode. The documentation you reference only raises concerns about using text mode on Unix/Windows. My understanding is that binary mode on both Unix and Windows writes out the bytes without modification, which is what we want

[sjakobi]

@Gabriel439 My understanding is, that if we'd use e.g. withBinaryFileDurableAtomic for dhall format's --inplace mode, we might on Windows read a file that uses CRLF line endings, and write it back with LFs.

[Gabriella439]

@sjakobi: Whoops, for some reason I thought your comment was in reference to #1540 (where binary mode is what we want for cache files). Yeah, I agree that for dhall format we would probably want to use text mode

[Gabriella439]

@sjakobi: Also, to answer your question about durability, it is not important. Atomicity is all that matters here

Specifically, in the absence of durability that would imply that if dhall format --inplace … succeeded and the system abruptly lost power then the file might not necessarily be updated. However, that's fine since the file would still be in the original pristine form if that were to take place (since we would still have atomicity).

Durability is usually a property that only databases needed: specifically durability guarantees that if a transaction registered as completed then the user can feel confident that the change they made took effect permanently (i.e. was "durable") and won't be accidentally lost or reverted. However, dhall format does not need to provide this sort of guarantee.

So this is a long-winded way of saying that using the atomic-write package is fine (and preferably, actually, since it's a smaller dependency).