diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index faf3e28dc36..0c149802195 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -429,18 +429,7 @@ end = struct in let action = match sandbox with - | None -> - (* CR-someday amokhov: It may be possible to support directory targets - without sandboxing. We just need to make sure we clean up all stale - directory targets before running the rule and then we can discover - all created files right in the build directory. *) - if not (Path.Build.Set.is_empty targets.dirs) then - User_error.raise ~loc - [ Pp.text "Rules with directory targets must be sandboxed." ] - ~hints: - [ Pp.text "Add (sandbox always) to the (deps ) field of the rule." - ]; - action + | None -> action | Some sandbox -> Action.sandbox action sandbox in let action = @@ -476,10 +465,7 @@ end = struct in let produced_targets = match sandbox with - | None -> - (* Directory targets are not allowed for non-sandboxed actions, so - the call below should not raise. *) - Targets.Produced.of_validated_files_exn targets + | None -> Targets.Produced.produced_after_rule_executed ~loc targets | Some sandbox -> (* The stamp file for anonymous actions is always created outside the sandbox, so we can't move it. *) diff --git a/src/dune_engine/rule_cache.ml b/src/dune_engine/rule_cache.ml index f2005f5bd2b..6e020ca8f60 100644 --- a/src/dune_engine/rule_cache.ml +++ b/src/dune_engine/rule_cache.ml @@ -122,7 +122,7 @@ module Workspace_local = struct let compute_target_digests (targets : Targets.Validated.t) : (Digest.t Targets.Produced.t, Miss_reason.t) Result.t = match Targets.Produced.of_validated targets with - | Error unix_error -> + | Error (_, unix_error) -> Miss (Error_while_collecting_directory_targets unix_error) | Ok targets -> ( match diff --git a/src/dune_engine/targets.ml b/src/dune_engine/targets.ml index 829a1b67e17..8c6b9b4103c 100644 --- a/src/dune_engine/targets.ml +++ b/src/dune_engine/targets.ml @@ -103,7 +103,7 @@ module Produced = struct let of_validated = let rec collect dir : (unit String.Map.t Path.Build.Map.t, _) result = match Path.Untracked.readdir_unsorted_with_kinds (Path.build dir) with - | Error _ as error -> error + | Error e -> Error (`Directory dir, e) | Ok dir_contents -> let open Result.O in let+ filenames, dirs = @@ -138,19 +138,22 @@ module Produced = struct in Ok { files; dirs } - let of_validated_files_exn (validated : Validated.t) = - let dirs = - match Path.Build.Set.is_empty validated.dirs with - | true -> Path.Build.Map.empty - | false -> - Code_error.raise - "Targets.Produced.of_validated_files_exn: Unexpected directory." - [ ("validated", Validated.to_dyn validated) ] - in - let files = - Path.Build.Set.to_map validated.files ~f:(fun (_ : Path.Build.t) -> ()) - in - { files; dirs } + let produced_after_rule_executed ~loc targets = + match of_validated targets with + | Ok t -> t + | Error (`Directory dir, (Unix.ENOENT, _, _)) -> + User_error.raise ~loc + [ Pp.textf "Rule failed to produce directory %S" + (Path.Build.drop_build_context_maybe_sandboxed_exn dir + |> Path.Source.to_string_maybe_quoted) + ] + | Error (`Directory dir, (unix_error, _, _)) -> + User_error.raise ~loc + [ Pp.textf "Rule produced unreadable directory %S" + (Path.Build.drop_build_context_maybe_sandboxed_exn dir + |> Path.Source.to_string_maybe_quoted) + ; Pp.verbatim (Unix.error_message unix_error) + ] let of_file_list_exn list = { files = Path.Build.Map.of_list_exn list; dirs = Path.Build.Map.empty } diff --git a/src/dune_engine/targets.mli b/src/dune_engine/targets.mli index a3f74f3427f..68c2246f09f 100644 --- a/src/dune_engine/targets.mli +++ b/src/dune_engine/targets.mli @@ -73,11 +73,13 @@ module Produced : sig (** Expand [targets : Validated.t] by recursively traversing directory targets and collecting all contained files. *) - val of_validated : Validated.t -> (unit t, Unix_error.Detailed.t) result + val of_validated : + Validated.t + -> (unit t, [ `Directory of Path.Build.t ] * Unix_error.Detailed.t) result - (** Return [targets : Validated.t] with the empty map of [dirs]. Raises a code - error if [targets.dir] is not empty. *) - val of_validated_files_exn : Validated.t -> unit t + (** Like [of_validated] but assumes the targets have been just produced by a + rule. If some directory targets aren't readable, an error is raised *) + val produced_after_rule_executed : loc:Loc.t -> Validated.t -> unit t (** Populates only the [files] field, leaving [dirs] empty. Raises a code error if the list contains duplicates. *) diff --git a/test/blackbox-tests/test-cases/directory-targets/main.t b/test/blackbox-tests/test-cases/directory-targets/main.t index 1656bf41cf3..25aac8a7503 100644 --- a/test/blackbox-tests/test-cases/directory-targets/main.t +++ b/test/blackbox-tests/test-cases/directory-targets/main.t @@ -24,17 +24,6 @@ Directory targets require an extension. > (using directory-targets 0.1) > EOF -Directory targets are not allowed for non-sandboxed rules. - - $ dune build output/x - File "dune", line 1, characters 0-56: - 1 | (rule - 2 | (targets (dir output)) - 3 | (action (bash "true"))) - Error: Rules with directory targets must be sandboxed. - Hint: Add (sandbox always) to the (deps ) field of the rule. - [1] - Ensure directory targets are produced. $ cat > dune < dune-project < (lang dune 3.4) + > (using directory-targets 0.1) + > EOF + +Build directory target from the command line without sandboxing + + $ cat > dune < (rule + > (targets (dir output)) + > (action (system "mkdir output; echo x > output/x; echo y > output/y"))) + > EOF + + $ dune build output/x + $ cat _build/default/output/x + x + $ cat _build/default/output/y + y + +We ask to build a file that doesn't exist inside the directory: + + $ dune build output/fake + File "dune", line 1, characters 0-102: + 1 | (rule + 2 | (targets (dir output)) + 3 | (action (system "mkdir output; echo x > output/x; echo y > output/y"))) + Error: This rule defines a directory target "output" that matches the + requested path "output/fake" but the rule's action didn't produce it + [1] + +When we fail to create the directory, dune complains: + + $ cat > dune < (rule + > (targets (dir output)) + > (action (system "true"))) + > EOF + + $ dune build output/ + File "dune", line 1, characters 0-56: + 1 | (rule + 2 | (targets (dir output)) + 3 | (action (system "true"))) + Error: Rule failed to produce directory "output" + [1]