eif: fix name collision in same folder for exes and melange emits

doc/changes/10220.md

@@ -0,0 +1,2 @@
+- Allow defining executables or melange emit stanzas with the same name in the
+  same folder under different contexts. (#10220, @rgrinberg, @jchavarri)

src/dune_rules/artifacts_obj.ml

@@ -12,7 +12,7 @@ let lookup_library { libraries; modules = _ } = Lib_name.Map.find libraries
 
 let make ~dir ~lib_config ~libs ~exes =
   let+ libraries =
-    Memo.List.map libs ~f:(fun ((lib : Library.t), _, _, _) ->
+    Memo.List.map libs ~f:(fun ((lib : Library.t), _, _) ->
       let+ lib_config = lib_config in
       let name = Lib_name.of_local lib.name in
       let info = Library.to_lib_info lib ~dir ~lib_config in
@@ -25,12 +25,10 @@ let make ~dir ~lib_config ~libs ~exes =
         Module_name.Map.add_exn modules (Module.name m) (obj_dir, m))
     in
     let init =
-      List.fold_left
-        exes
-        ~init:Module_name.Map.empty
-        ~f:(fun modules (_, _, m, obj_dir) -> by_name modules obj_dir m)
+      List.fold_left exes ~init:Module_name.Map.empty ~f:(fun modules (m, obj_dir) ->
+        by_name modules obj_dir m)
     in
-    List.fold_left libs ~init ~f:(fun modules (_, _, m, obj_dir) ->
+    List.fold_left libs ~init ~f:(fun modules (_, m, obj_dir) ->
       by_name modules obj_dir m)
   in
   { libraries; modules }

src/dune_rules/artifacts_obj.mli

@@ -7,8 +7,8 @@ val empty : t
 val make
   :  dir:Path.Build.t
   -> lib_config:Lib_config.t Memo.t
-  -> libs:(Library.t * _ * Modules.t * Path.Build.t Obj_dir.t) list
-  -> exes:(_ * _ * Modules.t * Path.Build.t Obj_dir.t) list
+  -> libs:(Library.t * Modules.t * Path.Build.t Obj_dir.t) list
+  -> exes:(Modules.t * Path.Build.t Obj_dir.t) list
   -> t Memo.t
 
 val lookup_module : t -> Module_name.t -> (Path.Build.t Obj_dir.t * Module.t) option

src/dune_rules/melange/melange_rules.ml

@@ -598,28 +598,41 @@ let emit_rules sctx { stanza_dir; stanza } =
 ;;
 
 (* Detect if [dir] is under the target directory of a melange.emit stanza. *)
-let rec under_melange_emit_target ~dir =
+let rec under_melange_emit_target ~sctx ~dir =
   match Path.Build.parent dir with
   | None -> Memo.return None
   | Some parent ->
     Dune_load.stanzas_in_dir parent
     >>= (function
-     | None -> under_melange_emit_target ~dir:parent
+     | None -> under_melange_emit_target ~sctx ~dir:parent
      | Some stanzas ->
        Dune_file.find_stanzas stanzas Melange_stanzas.Emit.key
-       >>| List.find_map ~f:(fun mel ->
+       >>= Memo.List.find_map ~f:(fun (mel : Melange_stanzas.Emit.t) ->
          let target_dir = Melange_stanzas.Emit.target_dir ~dir:parent mel in
-         Option.some_if (Path.Build.equal target_dir dir) mel)
+         match Path.Build.equal target_dir dir with
+         | false -> Memo.return None
+         | true ->
+           (* In the case where we have two melange.emit stanzas in the same folder,
+              with one enabled in the current context and one disabled, we want to
+              make sure that we pick the enabled one *)
+           let+ enabled =
+             let* expander =
+               let* sctx = sctx in
+               Super_context.expander sctx ~dir
+             in
+             Expander.eval_blang expander mel.enabled_if
+           in
+           Option.some_if enabled mel)
        >>= (function
-        | None -> under_melange_emit_target ~dir:parent
+        | None -> under_melange_emit_target ~sctx ~dir:parent
         | Some stanza -> Memo.return @@ Some { stanza_dir = parent; stanza }))
 ;;
 
 let gen_emit_rules sctx ~dir ({ stanza_dir; stanza } as for_melange) =
   match Path.Build.equal dir (Melange_stanzas.Emit.target_dir ~dir:stanza_dir stanza) with
   | false -> Memo.return None
   | true ->
-    under_melange_emit_target ~dir:stanza_dir
+    under_melange_emit_target ~sctx ~dir:stanza_dir
     >>| (function
      | None -> Some (emit_rules sctx for_melange)
      | Some { stanza_dir = parent_melange_emit_dir; stanza = parent_stanza } ->
@@ -657,7 +670,7 @@ let gen_emit_rules sctx ~dir ({ stanza_dir; stanza } as for_melange) =
 module Gen_rules = Import.Build_config.Gen_rules
 
 let setup_emit_js_rules sctx ~dir =
-  under_melange_emit_target ~dir
+  under_melange_emit_target ~sctx ~dir
   >>= function
   | Some melange ->
     gen_emit_rules sctx ~dir melange

src/dune_rules/ml_sources.ml

@@ -41,10 +41,11 @@ module Modules = struct
   ;;
 
   type 'stanza group_part =
-    'stanza
-    * (Loc.t * Module.Source.t) Module_trie.t
-    * Modules_group.t
-    * Path.Build.t Obj_dir.t
+    { stanza : 'stanza
+    ; sources : (Loc.t * Module.Source.t) Module_trie.t
+    ; modules : Modules_group.t
+    ; obj_dir : Path.Build.t Obj_dir.t
+    }
 
   type groups =
     { libraries : Library.t group_part list
@@ -55,38 +56,38 @@ module Modules = struct
   let make { libraries = libs; executables = exes; melange_emits = emits } =
     let libraries =
       match
-        Lib_name.Map.of_list_map libs ~f:(fun (lib, _, m, obj_dir) ->
-          Library.best_name lib, (m, obj_dir))
+        Lib_name.Map.of_list_map libs ~f:(fun part ->
+          Library.best_name part.stanza, (part.modules, part.obj_dir))
       with
       | Ok x -> x
-      | Error (name, _, (lib2, _, _, _)) ->
+      | Error (name, _, part) ->
         User_error.raise
-          ~loc:lib2.buildable.loc
+          ~loc:part.stanza.buildable.loc
           [ Pp.textf
               "Library %S appears for the second time in this directory"
               (Lib_name.to_string name)
           ]
     in
     let executables =
       match
-        String.Map.of_list_map exes ~f:(fun ((exes : Executables.t), _, m, obj_dir) ->
-          snd (List.hd exes.names), (m, obj_dir))
+        String.Map.of_list_map exes ~f:(fun (part : Executables.t group_part) ->
+          snd (List.hd part.stanza.names), (part.modules, part.obj_dir))
       with
       | Ok x -> x
-      | Error (name, _, (exes2, _, _, _)) ->
+      | Error (name, _, part) ->
         User_error.raise
-          ~loc:exes2.buildable.loc
+          ~loc:part.stanza.buildable.loc
           [ Pp.textf "Executable %S appears for the second time in this directory" name ]
     in
     let melange_emits =
       match
-        String.Map.of_list_map emits ~f:(fun (mel, _, m, obj_dir) ->
-          mel.target, (m, obj_dir))
+        String.Map.of_list_map emits ~f:(fun part ->
+          part.stanza.target, (part.modules, part.obj_dir))
       with
       | Ok x -> x
-      | Error (name, _, (mel, _, _, _)) ->
+      | Error (name, _, part) ->
         User_error.raise
-          ~loc:mel.loc
+          ~loc:part.stanza.loc
           [ Pp.textf "Target %S appears for the second time in this directory" name ]
     in
     let rev_map =
@@ -96,9 +97,12 @@ module Modules = struct
             (Module.Source.path m, origin) :: acc)
         in
         List.concat
-          [ List.concat_map libs ~f:(fun (l, m, _, _) -> by_path (Library l) m)
-          ; List.concat_map exes ~f:(fun (e, m, _, _) -> by_path (Executables e) m)
-          ; List.concat_map emits ~f:(fun (l, m, _, _) -> by_path (Melange l) m)
+          [ List.concat_map libs ~f:(fun part ->
+              by_path (Library part.stanza) part.sources)
+          ; List.concat_map exes ~f:(fun part ->
+              by_path (Executables part.stanza) part.sources)
+          ; List.concat_map emits ~f:(fun part ->
+              by_path (Melange part.stanza) part.sources)
           ]
       in
       match Module_name.Path.Map.of_list modules with
@@ -389,70 +393,88 @@ let modules_of_stanzas =
       ; melange_emits = List.rev melange_emits
       }
   in
+  let make_executables ~dir ~expander ~modules ~project exes =
+    let obj_dir = Executables.obj_dir ~dir exes in
+    let+ sources, modules =
+      let { Buildable.loc = stanza_loc; modules = modules_settings; _ } =
+        exes.buildable
+      in
+      Modules_field_evaluator.eval
+        ~expander
+        ~modules
+        ~stanza_loc
+        ~src_dir:dir
+        ~kind:Modules_field_evaluator.Exe_or_normal_lib
+        ~private_modules:Ordered_set_lang.Unexpanded.standard
+        ~version:exes.dune_version
+        modules_settings
+    in
+    let modules =
+      let obj_dir = Obj_dir.obj_dir obj_dir in
+      if Dune_project.wrapped_executables project
+      then Modules_group.make_wrapped ~obj_dir ~modules `Exe
+      else Modules_group.exe_unwrapped modules ~obj_dir
+    in
+    `Executables { Modules.stanza = exes; sources; modules; obj_dir }
+  in
   fun stanzas ~expander ~project ~dir ~libs ~lookup_vlib ~modules ~include_subdirs ->
     Memo.parallel_map stanzas ~f:(fun stanza ->
-      match Stanza.repr stanza with
-      | Library.T lib ->
-        (* jeremiedimino: this [Resolve.get] means that if the user writes an
-           invalid [implements] field, we will get an error immediately even if
-           the library is not built. We should change this to carry the
-           [Or_exn.t] a bit longer. *)
-        let+ sources, modules =
-          let lookup_vlib = lookup_vlib ~loc:lib.buildable.loc in
-          make_lib_modules
-            ~expander
-            ~dir
-            ~libs
-            ~lookup_vlib
-            ~modules
-            ~lib
-            ~include_subdirs
-            ~version:lib.dune_version
-          >>= Resolve.read_memo
-        in
-        let obj_dir = Library.obj_dir lib ~dir in
-        `Library (lib, sources, modules, obj_dir)
-      | Executables.T exes | Tests.T { exes; _ } ->
-        let obj_dir = Executables.obj_dir ~dir exes in
-        let+ sources, modules =
-          let { Buildable.loc = stanza_loc; modules = modules_settings; _ } =
-            exes.buildable
-          in
-          Modules_field_evaluator.eval
-            ~expander
-            ~modules
-            ~stanza_loc
-            ~src_dir:dir
-            ~kind:Modules_field_evaluator.Exe_or_normal_lib
-            ~private_modules:Ordered_set_lang.Unexpanded.standard
-            ~version:exes.dune_version
-            modules_settings
-        in
-        let modules =
-          let obj_dir = Obj_dir.obj_dir obj_dir in
-          if Dune_project.wrapped_executables project
-          then Modules_group.make_wrapped ~obj_dir ~modules `Exe
-          else Modules_group.exe_unwrapped modules ~obj_dir
-        in
-        `Executables (exes, sources, modules, obj_dir)
-      | Melange_stanzas.Emit.T mel ->
-        let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in
-        let+ sources, modules =
-          Modules_field_evaluator.eval
-            ~expander
-            ~modules
-            ~stanza_loc:mel.loc
-            ~kind:Modules_field_evaluator.Exe_or_normal_lib
-            ~version:mel.dune_version
-            ~private_modules:Ordered_set_lang.Unexpanded.standard
-            ~src_dir:dir
-            mel.modules
-        in
-        let modules =
-          Modules_group.make_wrapped ~obj_dir:(Obj_dir.obj_dir obj_dir) ~modules `Melange
-        in
-        `Melange_emit (mel, sources, modules, obj_dir)
-      | _ -> Memo.return `Skip)
+      let enabled_if =
+        match Stanza.repr stanza with
+        | Library.T lib -> lib.enabled_if
+        | Tests.T exes -> exes.build_if
+        | Executables.T exes -> exes.enabled_if
+        | Melange_stanzas.Emit.T mel -> mel.enabled_if
+        | _ -> Blang.true_
+      in
+      Expander.eval_blang expander enabled_if
+      >>= function
+      | false -> Memo.return `Skip
+      | true ->
+        (match Stanza.repr stanza with
+         | Library.T lib ->
+           (* jeremiedimino: this [Resolve.get] means that if the user writes an
+              invalid [implements] field, we will get an error immediately even if
+              the library is not built. We should change this to carry the
+              [Or_exn.t] a bit longer. *)
+           let+ sources, modules =
+             let lookup_vlib = lookup_vlib ~loc:lib.buildable.loc in
+             make_lib_modules
+               ~expander
+               ~dir
+               ~libs
+               ~lookup_vlib
+               ~modules
+               ~lib
+               ~include_subdirs
+               ~version:lib.dune_version
+             >>= Resolve.read_memo
+           in
+           let obj_dir = Library.obj_dir lib ~dir in
+           `Library { Modules.stanza = lib; sources; modules; obj_dir }
+         | Executables.T exes -> make_executables ~dir ~expander ~modules ~project exes
+         | Tests.T { exes; _ } -> make_executables ~dir ~expander ~modules ~project exes
+         | Melange_stanzas.Emit.T mel ->
+           let obj_dir = Obj_dir.make_melange_emit ~dir ~name:mel.target in
+           let+ sources, modules =
+             Modules_field_evaluator.eval
+               ~expander
+               ~modules
+               ~stanza_loc:mel.loc
+               ~kind:Modules_field_evaluator.Exe_or_normal_lib
+               ~version:mel.dune_version
+               ~private_modules:Ordered_set_lang.Unexpanded.standard
+               ~src_dir:dir
+               mel.modules
+           in
+           let modules =
+             Modules_group.make_wrapped
+               ~obj_dir:(Obj_dir.obj_dir obj_dir)
+               ~modules
+               `Melange
+           in
+           `Melange_emit { Modules.stanza = mel; sources; modules; obj_dir }
+         | _ -> Memo.return `Skip))
     >>| filter_partition_map
 ;;
 
@@ -545,11 +567,15 @@ let make
   let modules = Modules.make modules_of_stanzas in
   let artifacts =
     Memo.lazy_ (fun () ->
-      Artifacts_obj.make
-        ~dir
-        ~lib_config
-        ~libs:modules_of_stanzas.libraries
-        ~exes:modules_of_stanzas.executables)
+      let libs =
+        List.map modules_of_stanzas.libraries ~f:(fun (part : _ Modules.group_part) ->
+          part.stanza, part.modules, part.obj_dir)
+      in
+      let exes =
+        List.map modules_of_stanzas.executables ~f:(fun (part : _ Modules.group_part) ->
+          part.modules, part.obj_dir)
+      in
+      Artifacts_obj.make ~dir ~lib_config ~libs ~exes)
   in
   { modules; artifacts; include_subdirs }
 ;;

test/blackbox-tests/test-cases/enabled_if/cma-pform.t

@@ -11,7 +11,7 @@ We try to build a disabled library using the %{cma:..} pfrom
   > EOF
 
   $ dune build %{cma:./foo}
-  Error: No rule found for foo.cma
-  -> required by %{cma:./foo} at command line:1
+  File "command line", line 1, characters 0-12:
+  Error: Library foo does not exist.
   [1]
   $ dune build --profile with-foo %{cma:./foo}

test/blackbox-tests/test-cases/enabled_if/eif-exe-name-collision-same-folder.t

@@ -27,9 +27,3 @@ in the same dune file
   > EOF
 
   $ dune build
-  File "dune", line 4, characters 0-72:
-  4 | (executable
-  5 |  (name foo)
-  6 |  (enabled_if (= %{context_name} "alt-context")))
-  Error: Executable "foo" appears for the second time in this directory
-  [1]

test/blackbox-tests/test-cases/enabled_if/eif-library-disabled-cmo-error.t

@@ -15,7 +15,7 @@ Test behavior when targeting artifacts of a disabled library
   > EOF
 
   $ dune build %{cmo:foo}
-  Error: No rule found for .foo.objs/byte/foo.cmo
-  -> required by %{cmo:foo} at command line:1
+  File "command line", line 1, characters 0-10:
+  Error: Module Foo does not exist.
   [1]
 

test/blackbox-tests/test-cases/enabled_if/eif-melange-emit-name-collision-same-folder.t

@@ -28,9 +28,3 @@ in the same dune file
   > EOF
 
   $ dune build
-  File "dune", line 4, characters 0-76:
-  4 | (melange.emit
-  5 |  (target foo)
-  6 |  (enabled_if (= %{context_name} "alt-context")))
-  Error: Target "foo" appears for the second time in this directory
-  [1]