Add basic support for directory targets (#5025)

This PR implements a basic version of the RFC from #3316.

Basically, one can now write [(target (dir output))] in a rule to specify a
directory target. This functionality is experimental and is therefore guarded
by the [directory-targets] extension. We need directory targets internally at
Jane Street and will polish the feature as it gets some further use before
making it officially supported in Dune.

See the test file for all the currently available features. For now, I only
added a brief comment to the docs that this feature is now available for
experimentation. Documentation will be expanded when we figure out the full
story.

Signed-off-by: Andrey Mokhov <[email protected]>

CHANGES.md

@@ -241,6 +241,9 @@ Unreleased
 - Allow to cancel the initial scan via Control+C (#4460, fixes #4364
   @jeremiedimino)
 
+- Add experimental support for directory targets (#3316, #5025, Andrey Mokhov),
+  enabled via `(using directory-targets 0.1)` in `dune-project`.
+
 2.9.2 (unreleased)
 ------------------
 

bin/exec.ml

@@ -73,9 +73,12 @@ let term =
         let open Memo.Build.O in
         let+ hints =
           (* Good candidates for the "./x.exe" instead of "x.exe" error are
-             executables present in the current directory *)
+             executables present in the current directory. Note: we do not check
+             directory targets here; even if they do indeed include a matching
+             executable, they would be located in a subdirectory of [dir], so
+             it's unclear if that's what the user wanted. *)
           let+ candidates =
-            Build_system.targets_of ~dir:(Path.build dir)
+            Build_system.file_targets_of ~dir:(Path.build dir)
             >>| Path.Set.to_list
             >>| List.filter ~f:(fun p -> Path.extension p = ".exe")
             >>| List.map ~f:(fun p -> "./" ^ Path.basename p)

bin/print_rules.ml

@@ -34,11 +34,16 @@ let print_rule_makefile ppf (rule : Dune_engine.Reflection.Rule.t) =
       ; Action.for_shell rule.action
       ]
   in
+  (* Makefiles seem to allow directory targets, so we include them. *)
+  let targets =
+    Dune_engine.Targets.map rule.targets ~f:(fun ~files ~dirs ->
+        Path.Build.Set.union files dirs)
+  in
   Format.fprintf ppf
     "@[<hov 2>@{<makefile-stuff>%a:%t@}@]@,@<0>\t@{<makefile-action>%a@}@,@,"
     (Format.pp_print_list ~pp_sep:Format.pp_print_space (fun ppf p ->
          Format.pp_print_string ppf (Path.to_string p)))
-    (Targets.to_list_map rule.targets ~file:Path.build)
+    (List.map ~f:Path.build (Path.Build.Set.to_list targets))
     (fun ppf ->
       Path.Set.iter rule.expanded_deps ~f:(fun dep ->
           Format.fprintf ppf "@ %s" (Path.to_string dep)))
@@ -49,13 +54,24 @@ let print_rule_sexp ppf (rule : Dune_engine.Reflection.Rule.t) =
     Action.for_shell action |> Action.For_shell.encode
   in
   let paths ps = Dune_lang.Encoder.list Dpath.encode (Path.Set.to_list ps) in
+  let file_targets =
+    Dune_engine.Targets.map rule.targets ~f:(fun ~files ~dirs ->
+        if not (Path.Build.Set.is_empty dirs) then
+          User_error.raise
+            [ Pp.text
+                "Printing rules with directory targets is currently not \
+                 supported"
+            ];
+
+        files)
+  in
   let sexp =
     Dune_lang.Encoder.record
       (List.concat
          [ [ ("deps", Dep.Set.encode rule.deps)
            ; ( "targets"
              , paths
-                 (Targets.to_list_map rule.targets ~file:Fun.id
+                 (Path.Build.Set.to_list file_targets
                  |> Path.set_of_build_paths_list) )
            ]
          ; (match rule.context with

bin/target.ml

@@ -21,6 +21,14 @@ let target_hint (_setup : Dune_rules.Main.build_system) path =
   assert (Path.is_managed path);
   let open Memo.Build.O in
   let sub_dir = Option.value ~default:path (Path.parent path) in
+  (* CR-someday amokhov: There are two issues with the code below.
+
+     (1) We first get *all* targets but then filter out only those that are
+     defined in the [sub_dir]. It would be better to just get the targets for
+     the [sub_dir] directly (the API supports this).
+
+     (2) We currently provide the same hint for all targets. It would be nice to
+     indicate whether a hint corresponds to a file or to a directory target. *)
   let+ candidates = Build_system.all_targets () >>| Path.Build.Set.to_list in
   let candidates =
     if Path.is_in_build_dir path then

otherlibs/stdune-unstable/path.ml

@@ -686,6 +686,13 @@ module Build = struct
     | None ->
       Code_error.raise "Path.Build.drop_build_context_exn" [ ("t", to_dyn t) ]
 
+  let drop_build_context_maybe_sandboxed_exn t =
+    match extract_build_context_dir_maybe_sandboxed t with
+    | Some (_, t) -> t
+    | None ->
+      Code_error.raise "Path.Build.drop_build_context_maybe_sandboxed_exn"
+        [ ("t", to_dyn t) ]
+
   let build_dir = Fdecl.create Kind.to_dyn
 
   let build_dir_prefix = Fdecl.create Dyn.Encoder.opaque

otherlibs/stdune-unstable/path.mli

@@ -156,6 +156,8 @@ module Build : sig
 
   val drop_build_context_exn : t -> Source.t
 
+  val drop_build_context_maybe_sandboxed_exn : t -> Source.t
+
   (** [Source.t] here is a lie in some cases: consider when the context name
       happens to be ["install"] or [".alias"]. *)
   val extract_build_context : t -> (string * Source.t) option

src/dune_engine/action_exec.ml

@@ -127,7 +127,15 @@ let exec_run_dynamic_client ~ectx ~eenv prog args =
       let to_relative path =
         path |> Stdune.Path.build |> Stdune.Path.reach ~from:eenv.working_dir
       in
-      Targets.to_list_map ectx.targets ~file:to_relative |> String.Set.of_list
+      let file_targets, (_dir_targets_not_allowed : Nothing.t list) =
+        Targets.partition_map ectx.targets ~file:to_relative
+          ~dir:(fun _dir_target ->
+            User_error.raise ~loc:ectx.rule_loc
+              [ Pp.text
+                  "Directory targets are not compatible with dynamic actions"
+              ])
+      in
+      String.Set.of_list file_targets
     in
     DAP.Run_arguments.
       { prepared_dependencies = eenv.prepared_dependencies; targets }