-
Notifications
You must be signed in to change notification settings - Fork 14
[Files.save_as]: just write directly to special files #60
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
1d7a956
[Files.save_as]: just write directly to special files
cf78850
Update files.mli
arvidj 3d0d2c1
Update files.mli
arvidj 086bc2d
reformat mli
c6d5c8c
recurse
f0e7663
save_as tests
26dcb46
Merge branch 'master' into aj/save_as-no-clobber
rr0gi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -67,7 +67,7 @@ let mkdir_p ?(perm=0o755) path = | |||||||||
| in | ||||||||||
| aux path | ||||||||||
|
|
||||||||||
| let save_as name ?(mode=0o644) f = | ||||||||||
| let save_as_regular name ?(mode=0o644) f = | ||||||||||
| (* not using make_temp_file cause same dir is needed for atomic rename *) | ||||||||||
| let temp = Printf.sprintf "%s.save.%d.tmp" name (U.gettid ()) in | ||||||||||
| bracket (Unix.openfile temp [Unix.O_WRONLY;Unix.O_CREAT] mode) Unix.close begin fun fd -> | ||||||||||
|
|
@@ -81,3 +81,9 @@ let save_as name ?(mode=0o644) f = | |||||||||
| with | ||||||||||
| exn -> Exn.suppress Unix.unlink temp; raise exn | ||||||||||
| end | ||||||||||
|
|
||||||||||
| let rec save_as name ?mode f = | ||||||||||
| match (Unix.lstat name).st_kind with | ||||||||||
| | Unix.S_LNK -> save_as (Unix.realpath name) ?mode f | ||||||||||
| | Unix.S_REG | (exception Unix.Unix_error (Unix.ENOENT, _, _)) -> save_as_regular name ?mode f | ||||||||||
|
Comment on lines
+87
to
+88
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| | _ -> Out_channel.with_open_gen [ Open_wronly ] 0 name f | ||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -595,6 +595,80 @@ let () = | |
| assert_equal !accumulator 4; | ||
| () | ||
|
|
||
| let with_temp_path name f = | ||
| let path = Filename.concat (Filename.get_temp_dir_name ()) name in | ||
| (try Sys.remove path with _ -> ()); | ||
| Fun.protect ~finally:(fun () -> try Sys.remove path with _ -> ()) (fun () -> f path) | ||
|
|
||
| let () = test "Files.save_as writes to new regular file" @@ fun () -> | ||
| with_temp_path "test_save_as_new.txt" @@ fun path -> | ||
| Files.save_as path (fun oc -> output_string oc "hello\n"); | ||
| let content = In_channel.with_open_text path In_channel.input_all in | ||
| assert_equal ~printer:id "hello\n" content | ||
|
|
||
| let () = test "Files.save_as overwrites existing regular file" @@ fun () -> | ||
| with_temp_path "test_save_as_overwrite.txt" @@ fun path -> | ||
| Out_channel.with_open_text path (fun oc -> output_string oc "old\n"); | ||
| Files.save_as path (fun oc -> output_string oc "new\n"); | ||
| let content = In_channel.with_open_text path In_channel.input_all in | ||
| assert_equal ~printer:id "new\n" content | ||
|
|
||
| let () = test "Files.save_as no temp file left on success" @@ fun () -> | ||
| with_temp_path "test_save_as_no_temp.txt" @@ fun path -> | ||
| Files.save_as path (fun oc -> output_string oc "data\n"); | ||
| let temp = Printf.sprintf "%s.save.%d.tmp" path (U.gettid ()) in | ||
| assert_bool "temp file should not exist" (not (Sys.file_exists temp)) | ||
|
|
||
| let () = test "Files.save_as no temp file left on failure" @@ fun () -> | ||
| with_temp_path "test_save_as_fail.txt" @@ fun path -> | ||
| (try Files.save_as path (fun _oc -> failwith "boom") with Failure _ -> ()); | ||
| let temp = Printf.sprintf "%s.save.%d.tmp" path (U.gettid ()) in | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this test is kind of fragile but i guess can fix when if ever it breaks |
||
| assert_bool "temp file should not exist" (not (Sys.file_exists temp)); | ||
| assert_bool "target file should not exist" (not (Sys.file_exists path)) | ||
|
|
||
| let () = test "Files.save_as writes to /dev/null without error" @@ fun () -> | ||
| Files.save_as "/dev/null" (fun oc -> output_string oc "discarded\n"); | ||
| let temp = Printf.sprintf "/dev/null.save.%d.tmp" (U.gettid ()) in | ||
| assert_bool "temp file should not exist" (not (Sys.file_exists temp)); | ||
| assert_bool "/dev/null should be a char device" ((Unix.stat "/dev/null").st_kind = Unix.S_CHR) | ||
|
|
||
| let () = test "Files.save_as writes to named pipe (FIFO)" @@ fun () -> | ||
| with_temp_path "test_save_as_fifo" @@ fun fifo_path -> | ||
| Unix.mkfifo fifo_path 0o644; | ||
| let received = ref "" in | ||
| let reader = Thread.create (fun () -> received := In_channel.with_open_text fifo_path In_channel.input_all) () in | ||
| Files.save_as fifo_path (fun oc -> output_string oc "fifo data\n"); | ||
| Thread.join reader; | ||
| assert_equal ~printer:id "fifo data\n" !received | ||
|
|
||
| let () = test "Files.save_as no temp file created for FIFO" @@ fun () -> | ||
| with_temp_path "test_save_as_fifo2" @@ fun fifo_path -> | ||
| Unix.mkfifo fifo_path 0o644; | ||
| let reader = Thread.create (fun () -> ignore (In_channel.with_open_text fifo_path In_channel.input_all)) () in | ||
| Files.save_as fifo_path (fun oc -> output_string oc "data\n"); | ||
| Thread.join reader; | ||
| let temp = Printf.sprintf "%s.save.%d.tmp" fifo_path (U.gettid ()) in | ||
| assert_bool "temp file should not exist" (not (Sys.file_exists temp)) | ||
|
|
||
| let () = test "Files.save_as writes through symlink without clobbering it" @@ fun () -> | ||
| with_temp_path "test_save_as_symlink_target.txt" @@ fun target -> | ||
| with_temp_path "test_save_as_symlink_link.txt" @@ fun link -> | ||
| Out_channel.with_open_text target (fun oc -> output_string oc "old\n"); | ||
| Unix.symlink target link; | ||
| Files.save_as link (fun oc -> output_string oc "new\n"); | ||
| assert_bool "symlink should still be a symlink" ((Unix.lstat link).st_kind = Unix.S_LNK); | ||
| let content = In_channel.with_open_text target In_channel.input_all in | ||
| assert_equal ~printer:id "new\n" content | ||
|
|
||
| let () = test "Files.save_as fails on broken symlink" @@ fun () -> | ||
| with_temp_path "test_save_as_symlink_target.txt" @@ fun target -> | ||
| with_temp_path "test_save_as_symlink_link.txt" @@ fun link -> | ||
| Unix.symlink target link; | ||
| try | ||
| Files.save_as link (fun oc -> output_string oc "new\n"); | ||
| assert_failure "should fail on broken symlink" | ||
| with Unix.Unix_error(Unix.ENOENT, "realpath", _) -> () | ||
|
|
||
| let () = test "Logfmt" begin fun () -> | ||
| let eq name expected got = | ||
| assert_equal ~msg:name ~printer:(fun s -> sprintf "%S" s) expected got | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.