Skip to content

Commit be5ea14

Browse files
authored
fix: promoting a file into a directory (#13516)
Allow promoting a over a directory. This can happen when a cram test changes from a file to directory. --------- Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
1 parent ee6d70a commit be5ea14

File tree

3 files changed

+38
-21
lines changed

3 files changed

+38
-21
lines changed

doc/changes/fixed/13516.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- Fix promotions that modify a directory into a file (#13516, fixes #4067,
2+
@rgrinberg)

src/dune_engine/diff_promotion.ml

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ module File = struct
5050
;;
5151

5252
let do_promote ~correction_file ~dst =
53-
Fpath.unlink_no_err (Path.Source.to_string dst);
53+
(match Fpath.unlink (Path.Source.to_string dst) with
54+
| Success | Does_not_exist -> ()
55+
| Is_a_directory -> Path.rm_rf (Path.source dst)
56+
| Error _ -> ());
5457
let chmod = Path.Permissions.add Path.Permissions.write in
5558
Path.mkdir_p (Path.source (Path.Source.parent_exn dst));
5659
match Io.copy_file ~chmod ~src:correction_file ~dst:(Path.source dst) () with
@@ -108,7 +111,18 @@ let register_intermediate ~source_file ~correction_file =
108111
Dune_trace.emit Promote (fun () ->
109112
Dune_trace.Event.Promote.register `Staged src source_file);
110113
let staging = File.in_staging_area source_file in
111-
Path.mkdir_p (Path.build (Option.value_exn (Path.Build.parent staging)));
114+
let staging_dir = Path.Build.parent_exn staging in
115+
let mkdir_p () = Fpath.mkdir_p_strict (Path.Build.to_string staging_dir) in
116+
(match
117+
match mkdir_p () with
118+
| `Not_a_dir ->
119+
Path.rm_rf (Path.build staging_dir);
120+
mkdir_p ()
121+
| x -> x
122+
with
123+
| `Already_exists | `Created -> ()
124+
| `Not_a_dir ->
125+
Code_error.raise "dir was deleted" [ "staging_dir", Path.Build.to_dyn staging_dir ]);
112126
Unix.rename (Path.Build.to_string correction_file) (Path.Build.to_string staging);
113127
db := { src; staging = Some staging; dst = source_file } :: !db
114128
;;
@@ -155,18 +169,18 @@ let promote_one dst srcs =
155169
| [] -> assert false
156170
| (src, staging) :: others ->
157171
(* We used to remove promoted files from the digest cache, to force Dune
158-
to redigest them on the next run. We did this because on OSX [mtime] is
159-
not precise enough and if a file is modified and promoted quickly, it
160-
looked like it hadn't changed even though it might have.
161-
162-
aalekseyev: This is probably unnecessary now, depending on when
163-
[do_promote] runs (before or after [invalidate_cached_timestamps]).
164-
165-
amokhov: I removed this logic. In the current state of the world, files
166-
in the build directory should be redigested automatically (plus we do
167-
not promote into the build directory anyway), and source digests should
168-
be correctly invalidated via [fs_memo]. If that doesn't happen, we
169-
should fix [fs_memo] instead of manually resetting the caches here. *)
172+
to redigest them on the next run. We did this because on OSX [mtime] is
173+
not precise enough and if a file is modified and promoted quickly, it
174+
looked like it hadn't changed even though it might have.
175+
176+
aalekseyev: This is probably unnecessary now, depending on when
177+
[do_promote] runs (before or after [invalidate_cached_timestamps]).
178+
179+
amokhov: I removed this logic. In the current state of the world, files
180+
in the build directory should be redigested automatically (plus we do
181+
not promote into the build directory anyway), and source digests should
182+
be correctly invalidated via [fs_memo]. If that doesn't happen, we
183+
should fix [fs_memo] instead of manually resetting the caches here. *)
170184
File.promote { src; staging; dst };
171185
List.iter others ~f:(fun (path, _staging) ->
172186
Console.print

test/blackbox-tests/test-cases/promote/promote-file-to-dir.t

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
Demonstrate issue https://github.com/ocaml/dune/issues/7383 with changing a file for
2-
promotion from a file to a directory whilst in the staging area. This causes Dune to get
3-
confused and stuck.
1+
Demonstrate issue https://github.com/ocaml/dune/issues/7383 with changing
2+
a file for promotion from a file to a directory whilst in the staging area.
3+
This caused Dune to get confused and stuck.
44

55
Create a cram test:
66

@@ -40,11 +40,12 @@ Run the cram test, should fail and file should be promoted
4040
@@ -1 +1,2 @@
4141
$ echo hello
4242
+ hello
43-
Error: rename(_build/default/my_cram.t/run.t.corrected): Not a directory
44-
-> required by alias my_cram
45-
-> required by alias runtest
4643
[1]
4744

48-
We cannot promote:
45+
We can promote:
4946

5047
$ dune promote
48+
Promoting _build/default/my_cram.t/run.t.corrected to my_cram.t/run.t.
49+
50+
$ [ -f my_cram.t/run.t ] && echo promted to a file
51+
promted to a file

0 commit comments

Comments
 (0)