Skip to content

Commit 753e345

Browse files
committed
Revert "refactor(rules): remove support for patch back into source tree (#10771)"
This reverts commit ebc7035. Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
1 parent dc035e6 commit 753e345

11 files changed

Lines changed: 347 additions & 104 deletions

File tree

src/dune_engine/diff_promotion.ml

Lines changed: 68 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,16 @@ module File = struct
9090
;;
9191
end
9292

93-
type db = File.t list
93+
type op =
94+
| File of File.t
95+
| Delete of Path.Source.t
96+
97+
let dyn_of_op = function
98+
| File f -> Dyn.variant "File" [ File.to_dyn f ]
99+
| Delete d -> Dyn.variant "Delete" [ Path.Source.to_dyn d ]
100+
;;
101+
102+
type db = op list
94103

95104
let db : db ref = ref []
96105
let clear_cache () = db := []
@@ -100,7 +109,7 @@ let register_dep ~source_file ~correction_file =
100109
let src = snd (Path.Build.split_sandbox_root correction_file) in
101110
Dune_trace.emit Promote (fun () ->
102111
Dune_trace.Event.Promote.register `Direct src source_file);
103-
db := { src; staging = None; dst = source_file } :: !db
112+
db := File { src; staging = None; dst = source_file } :: !db
104113
;;
105114

106115
let register_intermediate ~source_file ~correction_file =
@@ -110,28 +119,30 @@ let register_intermediate ~source_file ~correction_file =
110119
let staging = File.in_staging_area source_file in
111120
Path.mkdir_p (Path.build (Option.value_exn (Path.Build.parent staging)));
112121
Unix.rename (Path.Build.to_string correction_file) (Path.Build.to_string staging);
113-
db := { src; staging = Some staging; dst = source_file } :: !db
122+
db := File { src; staging = Some staging; dst = source_file } :: !db
114123
;;
115124

116125
module P = Persistent.Make (struct
117-
type t = File.t list
126+
type t = db
118127

119128
let name = "TO-PROMOTE"
120-
let version = 3
121-
let to_dyn = Dyn.list File.to_dyn
129+
let version = 4
130+
let to_dyn = Dyn.list dyn_of_op
122131

123132
let test_example () =
124-
[ { File.src = Path.Build.(relative root "foo")
125-
; dst = Path.Source.of_string "bar"
126-
; staging = Some Path.Build.(relative root "baz")
127-
}
133+
[ File
134+
{ File.src = Path.Build.(relative root "foo")
135+
; dst = Path.Source.of_string "bar"
136+
; staging = Some Path.Build.(relative root "baz")
137+
}
138+
; Delete (Path.Source.of_string "foo")
128139
]
129140
;;
130141
end)
131142

132143
let db_file = Path.relative Path.build_dir ".to-promote"
133144

134-
let dump_db db =
145+
let dump_db (db : db) =
135146
if Path.build_dir_exists ()
136147
then (
137148
match db with
@@ -143,43 +154,54 @@ let dump_db db =
143154
let load_db () = Option.value ~default:[] (P.load db_file)
144155

145156
let group_by_targets db =
146-
List.map db ~f:(fun { File.src; staging; dst } -> dst, (src, staging))
157+
List.map db ~f:(fun op ->
158+
match op with
159+
| Delete f -> f, `Delete
160+
| File { File.src; staging; dst } -> dst, `Promote (src, staging))
147161
|> Path.Source.Map.of_list_multi
148162
(* Sort the list of possible sources for deterministic behavior *)
149-
|> Path.Source.Map.map
150-
~f:(List.sort ~compare:(fun (x, _) (y, _) -> Path.Build.compare x y))
163+
|> Path.Source.Map.map ~f:(List.sort ~compare:Poly.compare)
151164
;;
152165

153166
let promote_one dst srcs =
154167
match srcs with
155168
| [] -> assert false
156-
| (src, staging) :: others ->
169+
| op :: others ->
157170
(* 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. *)
170-
File.promote { src; staging; dst };
171-
List.iter others ~f:(fun (path, _staging) ->
171+
to redigest them on the next run. We did this because on OSX [mtime] is
172+
not precise enough and if a file is modified and promoted quickly, it
173+
looked like it hadn't changed even though it might have.
174+
175+
aalekseyev: This is probably unnecessary now, depending on when
176+
[do_promote] runs (before or after [invalidate_cached_timestamps]).
177+
178+
amokhov: I removed this logic. In the current state of the world, files
179+
in the build directory should be redigested automatically (plus we do
180+
not promote into the build directory anyway), and source digests should
181+
be correctly invalidated via [fs_memo]. If that doesn't happen, we
182+
should fix [fs_memo] instead of manually resetting the caches here. *)
183+
(match op with
184+
| `Promote (src, staging) -> File.promote { src; staging; dst }
185+
| `Delete -> Unix.unlink (Path.Source.to_string dst));
186+
List.iter others ~f:(fun op ->
187+
let path =
188+
match op with
189+
| `Promote (src, _) -> Path.build src
190+
| `Delete -> Path.source dst
191+
in
172192
Console.print
173-
[ Pp.textf " -> ignored %s." (Path.to_string_maybe_quoted (Path.build path))
174-
; Pp.space
175-
])
193+
[ Pp.textf " -> ignored %s." (Path.to_string_maybe_quoted path); Pp.space ])
176194
;;
177195

178196
let do_promote_all db = group_by_targets db |> Path.Source.Map.iteri ~f:promote_one
179197

180198
let do_promote_these db files =
181199
let by_targets = group_by_targets db in
182-
let by_targets, missing =
200+
let ( (by_targets :
201+
[> `Delete | `Promote of Path.Build.t * Path.Build.t option ] list
202+
Path.Source.Map.t)
203+
, missing )
204+
=
183205
let files = Path.Source.Set.of_list files in
184206
Path.Source.Set.fold files ~init:(by_targets, []) ~f:(fun fn (map, missing) ->
185207
match Path.Source.Map.find map fn with
@@ -190,8 +212,10 @@ let do_promote_these db files =
190212
in
191213
let remaining =
192214
Path.Source.Map.to_list by_targets
193-
|> List.concat_map ~f:(fun (dst, srcs) ->
194-
List.map srcs ~f:(fun (src, staging) -> { File.src; staging; dst }))
215+
|> List.concat_map ~f:(fun (dst, ops) ->
216+
List.map ops ~f:(function
217+
| `Delete -> Delete dst
218+
| `Promote (src, staging) -> File { File.src; staging; dst }))
195219
in
196220
(* [group_by_targets] will sort all files, but the [fold] above reverses
197221
the list of missing files. Here we re-reverse it so it is sorted.
@@ -236,7 +260,13 @@ type all =
236260
(** [partition_db db files_to_promote] splits [files_to_promote] into two lists
237261
- The files present in [db] as actual [File.t]s.
238262
- The files absent from [db] as [Path]s. *)
239-
let partition_db db files_to_promote =
263+
let partition_db (db : db) files_to_promote =
264+
let db =
265+
(* CR-soon rgrinberg: communicate deletions to the user *)
266+
List.filter_map db ~f:(function
267+
| File f -> Some f
268+
| Delete _ -> None)
269+
in
240270
let present, missing =
241271
match files_to_promote with
242272
| Files_to_promote.All -> db, []
@@ -248,3 +278,5 @@ let partition_db db files_to_promote =
248278
in
249279
{ present; missing }
250280
;;
281+
282+
let register_delete src = db := Delete src :: !db

src/dune_engine/diff_promotion.mli

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,5 @@ val register_intermediate
5151
refers to a path in the build dir, not in the sandbox (it can point to the
5252
sandbox, but the sandbox root will be stripped). *)
5353
val register_dep : source_file:Path.Source.t -> correction_file:Path.Build.t -> unit
54+
55+
val register_delete : Path.Source.t -> unit

src/dune_engine/sandbox.ml

Lines changed: 26 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -181,26 +181,7 @@ let create ~mode ~rule_loc ~dirs ~deps ~rule_dir ~rule_digest =
181181
in
182182
Option.iter dune_stats ~f:(fun trace -> Dune_trace.Out.finish trace event);
183183
match mode with
184-
| Patch_back_source_tree ->
185-
(* Only supported on Linux because we rely on the mtime changing to detect
186-
when a file changes. This doesn't work on OSX for instance as the file
187-
system granularity is 1s, which is too coarse. *)
188-
(match Platform.OS.value with
189-
| Linux -> ()
190-
| _ ->
191-
User_error.raise
192-
~loc:rule_loc
193-
[ Pp.textf
194-
"(mode patch-back-source-tree) is only supported on Linux at the moment."
195-
]);
196-
(* We expect this call to [snapshot t] to return the same set of files as
197-
[deps], given that's exactly what we just copied in the sandbox. So in
198-
theory, we could iterate over [deps] rather than scan the file system.
199-
However, the code is simpler if we just call [snapshot t] before and
200-
after running the action. Given that [patch_back_source_tree] is a dodgy
201-
feature that we hope to get rid of in the long run, we favor code
202-
simplicity over performance. *)
203-
{ t with snapshot = Some (snapshot t) }
184+
| Patch_back_source_tree -> { t with snapshot = Some (snapshot t) }
204185
| _ -> t
205186
;;
206187

@@ -217,35 +198,38 @@ let rename_optional_file ~src ~dst =
217198
| () -> ())
218199
;;
219200

220-
let apply_changes_to_source_tree t ~old_snapshot =
201+
let apply_changes_to_source_tree t (targets : Targets.Validated.t) ~old_snapshot =
221202
let new_snapshot = snapshot t in
222203
(* Same as promotion: make the file writable when copying to the source
223204
tree. *)
224205
let in_source_tree p =
225-
Path.extract_build_context_dir_maybe_sandboxed p
226-
|> Option.value_exn
227-
|> snd
228-
|> Path.source
206+
Path.extract_build_context_dir_maybe_sandboxed p |> Option.value_exn |> snd
229207
in
230208
let copy_file p =
231-
let in_source_tree = in_source_tree p in
232-
Fpath.unlink_no_err (Path.to_string in_source_tree);
233-
Option.iter (Path.parent in_source_tree) ~f:Path.mkdir_p;
234-
Io.copy_file ~src:p ~dst:in_source_tree ()
235-
in
236-
let delete_file p =
237-
let in_source_tree = in_source_tree p in
238-
Fpath.unlink_no_err (Path.to_string in_source_tree)
209+
let source_file = in_source_tree p in
210+
let correction_file = Path.as_in_build_dir_exn p in
211+
Diff_promotion.register_intermediate ~source_file ~correction_file
239212
in
213+
let delete_file file = in_source_tree file |> Diff_promotion.register_delete in
214+
(* CR-soon rgrinberg: handle deleting directories *)
215+
let target_root_in_sandbox = map_path t targets.root in
240216
Path.Map.iter2 old_snapshot new_snapshot ~f:(fun p before after ->
241-
match before, after with
242-
| None, None -> assert false
243-
| None, Some _ -> copy_file p
244-
| Some _, None -> delete_file p
245-
| Some before, Some after ->
246-
(match Cached_digest.Reduced_stats.compare before after with
247-
| Eq -> ()
248-
| Lt | Gt -> copy_file p))
217+
if
218+
not
219+
(let dir = Path.as_in_build_dir_exn (Path.parent_exn p) in
220+
Path.Build.equal dir target_root_in_sandbox
221+
&&
222+
let basename = Path.basename p in
223+
Filename.Set.mem targets.files basename || Filename.Set.mem targets.dirs basename)
224+
then (
225+
match before, after with
226+
| None, None -> assert false
227+
| None, Some _ -> copy_file p
228+
| Some _, None -> delete_file p
229+
| Some before, Some after ->
230+
(match Cached_digest.Reduced_stats.compare before after with
231+
| Eq -> ()
232+
| Lt | Gt -> copy_file p)))
249233
;;
250234

251235
let hint_delete_dir =
@@ -260,7 +244,7 @@ let move_targets_to_build_dir t ~should_be_skipped ~(targets : Targets.Validated
260244
=
261245
maybe_async (fun () ->
262246
Option.iter t.snapshot ~f:(fun old_snapshot ->
263-
apply_changes_to_source_tree t ~old_snapshot);
247+
apply_changes_to_source_tree t targets ~old_snapshot);
264248
Targets.Validated.iter
265249
targets
266250
~file:(fun target ->

src/dune_engine/sandbox_config.ml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ module Partial = struct
6363
| _ -> None)
6464
;;
6565

66+
let patch_back_source_tree =
67+
Sandbox_mode.Dict.of_func (function
68+
| Some Patch_back_source_tree -> Some true
69+
| _ -> Some false)
70+
;;
71+
6672
let disallow (mode : Sandbox_mode.t) =
6773
Sandbox_mode.Dict.of_func (fun mode' ->
6874
if Sandbox_mode.equal mode mode' then Some false else None)

src/dune_engine/sandbox_config.mli

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,6 @@ module Partial : sig
4747
val no_special_requirements : t
4848
val no_sandboxing : t
4949
val needs_sandboxing : t
50+
val patch_back_source_tree : t
5051
val disallow : Sandbox_mode.t -> t
5152
end

src/dune_lang/dep_conf.ml

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,30 @@ module Glob_files = struct
1717
end
1818

1919
module Sandbox_config = struct
20-
type t = Loc.t * [ `None | `Always | `Preserve_file_kind ] list
21-
22-
let equal =
23-
Tuple.T2.equal
24-
Loc.equal
25-
(List.equal (fun a b ->
26-
match a, b with
27-
| `None, `None | `Always, `Always | `Preserve_file_kind, `Preserve_file_kind ->
28-
true
29-
| _, _ -> false))
30-
;;
20+
type mode =
21+
[ `None
22+
| `Always
23+
| `Preserve_file_kind
24+
| `Patch_back_source_tree
25+
]
26+
27+
type t = Loc.t * mode list
28+
29+
let equal_mode : mode -> mode -> bool = Poly.equal
30+
let equal = Tuple.T2.equal Loc.equal (List.equal equal_mode)
3131

3232
let all =
33-
[ "none", `None; "always", `Always; "preserve_file_kind", `Preserve_file_kind ]
33+
[ "none", `None, (1, 12)
34+
; "always", `Always, (1, 12)
35+
; "preserve_file_kind", `Preserve_file_kind, (1, 12)
36+
; "patch_back_source_tree", `Patch_back_source_tree, (3, 22)
37+
]
3438
;;
3539

3640
let loc (loc, _) = loc
3741

3842
let string_of_mode mode =
39-
List.find_map all ~f:(fun (s, mode') ->
43+
List.find_map all ~f:(fun (s, mode', _) ->
4044
if Poly.equal mode mode' then Some s else None)
4145
|> Option.value_exn
4246
;;
@@ -46,7 +50,13 @@ module Sandbox_config = struct
4650
;;
4751

4852
let decode : t Decoder.t =
49-
Syntax.since Stanza.syntax (1, 12) >>> located (repeat (enum all))
53+
let all =
54+
List.map all ~f:(fun (name, mode, version) ->
55+
( name
56+
, let+ () = Syntax.since Stanza.syntax version in
57+
mode ))
58+
in
59+
located (repeat (enum' all))
5060
;;
5161

5262
let fold (_, xs) ~f ~init = List.fold_left xs ~init ~f:(fun acc a -> f a acc)

src/dune_lang/dep_conf.mli

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ module Sandbox_config : sig
2121

2222
val fold
2323
: t
24-
-> f:([ `None | `Always | `Preserve_file_kind ] -> 'acc -> 'acc)
24+
-> f:
25+
([ `None | `Always | `Preserve_file_kind | `Patch_back_source_tree ]
26+
-> 'acc
27+
-> 'acc)
2528
-> init:'acc
2629
-> 'acc
2730
end

src/dune_rules/dep_conf_eval.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ let make_sandboxing_config config =
99
| `None -> Sandbox_config.Partial.no_sandboxing
1010
| `Always -> Sandbox_config.Partial.needs_sandboxing
1111
| `Preserve_file_kind -> Sandbox_config.Partial.disallow Sandbox_mode.symlink
12+
| `Patch_back_source_tree -> Sandbox_config.Partial.patch_back_source_tree
1213
in
1314
partial :: acc)
1415
|> Dune_engine.Sandbox_config.Partial.merge ~loc

0 commit comments

Comments
 (0)