Create .cmi files atomically (MPR#7472) #1307
Conversation
|
Wouldn't it make sense to generalize this to any file produced by the compiler ? This would also avoid corrupted objects if the compiler is killed during execution. |
xavierleroy
force-pushed
the
atomic_write_cmi
branch
from
6bdc713
to
59b6a13
Compare
This is done by writing the data to a temporary file, compute the checksum, finish writing the data, and only then rename the temporary file to the destination .cmi file. Writing .cmi files this way should avoid the corruption of .cmi files reported in MPR#4991. This corruption can occur when a .cmi file is produced simultaneously by a run of ocamlc and a run of ocamlopt. "Atomic" here means "as atomic as the underlying file system guarantees". The atomicity guarantees of Windows file systems aren't entirely clear.
xavierleroy
force-pushed
the
atomic_write_cmi
branch
from
59b6a13
to
58f841d
Compare
|
Now that #1306 is merged in trunk, I rebased and force-pushed like a pig, and this PR also works under Windows. Removing the WIP tag and asking for review. |
|
.cmt and .annot files would have the same issue (except that it might more easily go unnoticed since the compiler itself does not open those files). So I think it is worth abstracting the pattern (e.g. a new function in Misc) and reuse it at least for those files. |
|
If this going to be merged, it would be great if there was a simple pattern one can add to the gitignore to ignore these temporary files. My preference is a ".tmp" suffix. |
Follow-up to MPR#7472. The pattern "write to temporary file then rename" is abstracted in the new function Misc.output_to_file_via_temporary and applied to .cmi, .cmt and .annot files.
|
Good suggestions (".tmp" suffix + same handling for ".cmt" and ".annot" files). See latest commit. |
damiendoligez
requested review from
damiendoligez
and removed request for
damiendoligez
|
Would this hurt performance on build systems who never allows ocamlc and
ocamlopt run in parallel?
…On Tue, 29 Aug 2017 at 8:35 AM Daniel Bünzli ***@***.***> wrote:
Wouldn't it make sense to generalize this to any file produced by the
compiler ? This would also avoid corrupted objects if the compiler is
killed during execution.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1307 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAtmK6Eh1tVdiWlfmF0YQBHofAk8Aravks5sdC_JgaJpZM4PGHJk>
.
|
On unix I highly doubt that your build system performance is going to be bound by these |
…l#1307) * use .not-prose for the .btn on install page * change .prose a CSS style to take into account .not-prose
As suggested in MPR#7472, this is done by writing the data to a temporary file, compute the checksum, finish writing the data, and only then rename the temporary file to the destination .cmi file.
Writing .cmi files this way should avoid the corruption of .cmi files reported in MPR#4991. This corruption can occur when a .cmi file is produced simultaneously by a run of ocamlc and a run of ocamlopt.
"Atomic" here means "as atomic as the underlying file system guarantees". The atomicity guarantees of Windows file systems aren't entirely clear. Don't get me started on NFS either.
Speaking of Windows, this commit requires the new Win32 implementation of Sys.rename from GPR #1306 to work correctly in the MSVC and Mingw ports. I'll update this PR and remove the WIP tag when #1306 is merged. Until then Appveyor testing is expected to fail.