fix: account for workspace binaries when generating rules for ./.bin (#12952)

fixes #6220

- [x] changes entry

---------

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

Author: anmonteiro

doc/changes/fixed/12952.md

@@ -0,0 +1,3 @@
+- Resolve context and workspace binaries introduced by the respective `(env
+  (binaries ..))` stanzas. (#12952, fixes #6220, @anmonteiro)
+

src/dune_rules/artifacts.ml

@@ -16,7 +16,10 @@ type where =
   | Original_path
 
 type path =
-  | Resolved of Path.Build.t
+  | Resolved of
+      { binding : File_binding.Expanded.t
+      ; path : Path.Build.t
+      }
   | Origin of origin list
 
 type local_bins = path Filename.Map.t
@@ -36,6 +39,13 @@ let force { local_bins; _ } =
   ()
 ;;
 
+let local_binaries { local_bins; _ } =
+  let+ local_bins = Memo.Lazy.force local_bins in
+  List.filter_map (Filename.Map.to_list local_bins) ~f:(function
+    | _, Resolved p -> Some p.binding
+    | _, Origin _origins -> None)
+;;
+
 let analyze_binary t name =
   match Filename.is_relative name with
   | false -> Memo.return (`Resolved (Path.of_filename_relative_to_initial_cwd name))
@@ -48,7 +58,7 @@ let analyze_binary t name =
       | Some path -> `Resolved path
     in
     (match Filename.Map.find local_bins name with
-     | Some (Resolved p) -> Memo.return (`Resolved (Path.build p))
+     | Some (Resolved p) -> Memo.return (`Resolved (Path.build p.path))
      | None -> which ()
      | Some (Origin origins) ->
        Memo.parallel_map origins ~f:(fun origin ->
@@ -108,9 +118,9 @@ let add_binaries t ~dir l =
   let local_bins =
     Memo.lazy_ ~name:"Artifacts.Bin.add_binaries" (fun () ->
       let+ local_bins = Memo.Lazy.force t.local_bins in
-      List.fold_left l ~init:local_bins ~f:(fun acc fb ->
-        let path = File_binding.Expanded.dst_path fb ~dir:(local_bin dir) in
-        Filename.Map.set acc (Path.Build.basename path) (Resolved path)))
+      List.fold_left l ~init:local_bins ~f:(fun acc binding ->
+        let path = File_binding.Expanded.dst_path binding ~dir:(local_bin dir) in
+        Filename.Map.set acc (Path.Build.basename path) (Resolved { binding; path })))
   in
   { t with local_bins }
 ;;

src/dune_rules/artifacts.mli

@@ -24,6 +24,9 @@ val bin_dir_basename : Filename.t
     rules defined in [dir] *)
 val local_bin : Path.Build.t -> Path.Build.t
 
+(** Binaries that are symlinked in the associated .bin directory *)
+val local_binaries : t -> File_binding.Expanded.t list Memo.t
+
 (** A named artifact that is looked up in the PATH if not found in the tree If
     the name is an absolute path, it is used as it. *)
 val binary

src/dune_rules/env_node.ml

@@ -38,12 +38,6 @@ let make
       >>= extend)
   in
   let config_binaries = Option.value config.binaries ~default:[] in
-  let local_binaries =
-    Memo.lazy_ (fun () ->
-      Memo.parallel_map
-        config_binaries
-        ~f:(File_binding_expand.expand ~dir ~f:(expand_str_lazy expander)))
-  in
   let external_env =
     inherited ~field:external_env ~root:default_env (fun env ->
       let env =
@@ -58,7 +52,13 @@ let make
   in
   let artifacts =
     inherited ~field:artifacts ~root:default_artifacts (fun binaries ->
-      Memo.Lazy.force local_binaries >>| Artifacts.add_binaries binaries ~dir)
+      Memo.parallel_map
+        config_binaries
+        ~f:(File_binding_expand.expand ~dir ~f:(expand_str_lazy expander))
+      >>| Artifacts.add_binaries binaries ~dir)
+  in
+  let local_binaries =
+    Memo.lazy_ (fun () -> Memo.Lazy.force artifacts >>= Artifacts.local_binaries)
   in
   { external_env; artifacts; local_binaries }
 ;;

src/dune_rules/env_node.mli

@@ -16,8 +16,7 @@ val make
 
 val external_env : t -> Env.t Memo.t
 
-(** Binaries that are symlinked in the associated .bin directory of [dir]. This
-    associated directory is *)
+(** Binaries that are symlinked in the associated .bin directory of [dir]. *)
 val local_binaries : t -> File_binding.Expanded.t list Memo.t
 
 val artifacts : t -> Artifacts.t Memo.t

test/blackbox-tests/test-cases/dune-workspace-binaries-overlap.t

@@ -76,24 +76,11 @@ Workspace binary should print "Workspace."
 
   $ dune clean
   $ dune build ./message.txt
-  File "dune", lines 1-3, characters 0-78:
-  1 | (rule
-  2 |  (target message.txt)
-  3 |  (action (with-stdout-to %{target} (run foobar))))
-  Error: No rule found for .bin/foobar
-  File "dune", lines 1-3, characters 0-78:
-  1 | (rule
-  2 |  (target message.txt)
-  3 |  (action (with-stdout-to %{target} (run foobar))))
-  Error: No rule found for .bin/foobar (context other_context)
-  [1]
   $ cat _build/default/message.txt
-  cat: _build/default/message.txt: No such file or directory
-  [1]
+  Workspace.
 
 Context binaries override the workspace binaries. Expecting "Context."
 
   $ cat _build/other_context/message.txt
-  cat: _build/other_context/message.txt: No such file or directory
-  [1]
+  Context.
 

test/blackbox-tests/test-cases/dune-workspace-binaries.t

@@ -27,9 +27,3 @@ a dune-workspace file:
 
   $ chmod +x test.sh
   $ dune build ./message.txt
-  File "dune", lines 1-3, characters 0-78:
-  1 | (rule
-  2 |  (target message.txt)
-  3 |  (action (with-stdout-to %{target} (run foobar))))
-  Error: No rule found for .bin/foobar
-  [1]

test/blackbox-tests/test-cases/github5555.t

@@ -11,7 +11,7 @@ This test is about `binaries` in `env` stanzas in `dune-workspace` files.
   > EOF
 
   $ cat >x <<EOF
-  > #/bin/sh
+  > #!/bin/sh
   > echo foo
   > EOF
 
@@ -72,18 +72,11 @@ With 3.2, this fixes the error.
 In the default context:
 
   $ t 3.2 _ x
-  Error: No rule found for .bin/x
-  -> required by %{bin:x} at dune:3
-  -> required by alias foo in dune:1
-  [1]
+  foo
 
 And for explicit profiles:
 
   $ t 3.2 dev x
-  Error: No rule found for .bin/x
-  -> required by %{bin:x} at dune:3
-  -> required by alias foo in dune:1
-  [1]
 
 And for another profile: