From 7e548f281843e66c9a1bb38ceb6e5415895f44e0 Mon Sep 17 00:00:00 2001 From: Alpha DIALLO Date: Wed, 4 Sep 2024 18:42:37 +0200 Subject: [PATCH 1/3] fix: enabled_if with `dune describe` Fixes #10779. Signed-off-by: Alpha DIALLO --- bin/describe/describe_workspace.ml | 9 +++++++-- .../{eif-dune-describe-crash.t => eif-dune-describe.t} | 10 ++++------ 2 files changed, 11 insertions(+), 8 deletions(-) rename test/blackbox-tests/test-cases/enabled_if/{eif-dune-describe-crash.t => eif-dune-describe.t} (61%) diff --git a/bin/describe/describe_workspace.ml b/bin/describe/describe_workspace.ml index f405e0c9861..08278c0be1a 100644 --- a/bin/describe/describe_workspace.ml +++ b/bin/describe/describe_workspace.ml @@ -562,8 +562,13 @@ module Crawl = struct (Context.build_dir context) (Dune_file.dir dune_file) in - let project = Dune_file.project dune_file in - executables sctx ~options ~project ~dir exes + let* expander = Super_context.expander sctx ~dir in + Expander.eval_blang expander exes.enabled_if + >>= (function + | false -> Memo.return None + | true -> + let project = Dune_file.project dune_file in + executables sctx ~options ~project ~dir exes) | _ -> Memo.return None) >>| List.filter_opt) >>| List.concat diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-dune-describe-crash.t b/test/blackbox-tests/test-cases/enabled_if/eif-dune-describe.t similarity index 61% rename from test/blackbox-tests/test-cases/enabled_if/eif-dune-describe-crash.t rename to test/blackbox-tests/test-cases/enabled_if/eif-dune-describe.t index 273b7a7a82a..edaa41ca97e 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-dune-describe-crash.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-dune-describe.t @@ -24,9 +24,7 @@ $ dune build $ dune runtest - $ dune describe 2>&1 | head -n 5 - Internal error, please report upstream including the contents of _build/log. - Description: - ("modules_and_obj_dir: failed lookup", - { keys = []; for_ = Exe { first_exe = "test" } }) - Raised at Stdune__Code_error.raise in file + $ dune describe 2>&1 + ((root + $TESTCASE_ROOT) + (build_context _build/default)) From 623057f6c1f2db9065c765943345c8014288dfb3 Mon Sep 17 00:00:00 2001 From: Alpha DIALLO Date: Thu, 5 Sep 2024 10:01:50 +0200 Subject: [PATCH 2/3] move to the executables function Signed-off-by: Alpha DIALLO --- bin/describe/describe_workspace.ml | 112 ++++++++++++++--------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/bin/describe/describe_workspace.ml b/bin/describe/describe_workspace.ml index 08278c0be1a..b2cf2faa72f 100644 --- a/bin/describe/describe_workspace.ml +++ b/bin/describe/describe_workspace.ml @@ -387,58 +387,63 @@ module Crawl = struct let executables sctx ~options ~project ~dir (exes : Executables.t) : (Descr.Item.t * Lib.Set.t) option Memo.t = - let first_exe = snd (Nonempty_list.hd exes.names) in - let* scope = - Scope.DB.find_by_project (Super_context.context sctx |> Context.name) project - in - let* modules_, obj_dir = - let+ modules_, obj_dir = - Dir_contents.get sctx ~dir - >>= Dir_contents.ocaml - >>= Ml_sources.modules_and_obj_dir - ~libs:(Scope.libs scope) - ~for_:(Exe { first_exe }) + let* expander = Super_context.expander sctx ~dir in + Expander.eval_blang expander exes.enabled_if + >>= function + | false -> Memo.return None + | true -> + let first_exe = snd (Nonempty_list.hd exes.names) in + let* scope = + Scope.DB.find_by_project (Super_context.context sctx |> Context.name) project in - Modules.With_vlib.modules modules_, obj_dir - in - let* pp_map = - let+ version = - let+ ocaml = Super_context.context sctx |> Context.ocaml in - ocaml.version + let* modules_, obj_dir = + let+ modules_, obj_dir = + Dir_contents.get sctx ~dir + >>= Dir_contents.ocaml + >>= Ml_sources.modules_and_obj_dir + ~libs:(Scope.libs scope) + ~for_:(Exe { first_exe }) + in + Modules.With_vlib.modules modules_, obj_dir in - Staged.unstage - @@ Pp_spec.pped_modules_map - (Preprocess.Per_module.without_instrumentation exes.buildable.preprocess) - version - in - let deps_of module_ = - let module_ = pp_map module_ in - immediate_deps_of_module ~options ~obj_dir ~modules:modules_ module_ - in - let obj_dir = Obj_dir.of_local obj_dir in - let* modules_ = modules ~obj_dir ~deps_of modules_ in - let+ requires = - let* compile_info = Exe_rules.compile_info ~scope exes in - let open Resolve.Memo.O in - let* requires = Lib.Compile.direct_requires compile_info in - if options.with_pps - then - let+ pps = Lib.Compile.pps compile_info in - pps @ requires - else Resolve.Memo.return requires - in - match Resolve.peek requires with - | Error () -> None - | Ok libs -> - let include_dirs = Obj_dir.all_cmis obj_dir in - let exe_descr = - { Descr.Exe.names = List.map ~f:snd (Nonempty_list.to_list exes.names) - ; requires = List.map ~f:uid_of_library libs - ; modules = modules_ - ; include_dirs - } + let* pp_map = + let+ version = + let+ ocaml = Super_context.context sctx |> Context.ocaml in + ocaml.version + in + Staged.unstage + @@ Pp_spec.pped_modules_map + (Preprocess.Per_module.without_instrumentation exes.buildable.preprocess) + version in - Some (Descr.Item.Executables exe_descr, Lib.Set.of_list libs) + let deps_of module_ = + let module_ = pp_map module_ in + immediate_deps_of_module ~options ~obj_dir ~modules:modules_ module_ + in + let obj_dir = Obj_dir.of_local obj_dir in + let* modules_ = modules ~obj_dir ~deps_of modules_ in + let+ requires = + let* compile_info = Exe_rules.compile_info ~scope exes in + let open Resolve.Memo.O in + let* requires = Lib.Compile.direct_requires compile_info in + if options.with_pps + then + let+ pps = Lib.Compile.pps compile_info in + pps @ requires + else Resolve.Memo.return requires + in + (match Resolve.peek requires with + | Error () -> None + | Ok libs -> + let include_dirs = Obj_dir.all_cmis obj_dir in + let exe_descr = + { Descr.Exe.names = List.map ~f:snd (Nonempty_list.to_list exes.names) + ; requires = List.map ~f:uid_of_library libs + ; modules = modules_ + ; include_dirs + } + in + Some (Descr.Item.Executables exe_descr, Lib.Set.of_list libs)) ;; (* Builds a workspace item for the provided library object *) @@ -562,13 +567,8 @@ module Crawl = struct (Context.build_dir context) (Dune_file.dir dune_file) in - let* expander = Super_context.expander sctx ~dir in - Expander.eval_blang expander exes.enabled_if - >>= (function - | false -> Memo.return None - | true -> - let project = Dune_file.project dune_file in - executables sctx ~options ~project ~dir exes) + let project = Dune_file.project dune_file in + executables sctx ~options ~project ~dir exes | _ -> Memo.return None) >>| List.filter_opt) >>| List.concat From c1c66cfe602f2d24ca203e3388c96a865446e464 Mon Sep 17 00:00:00 2001 From: Alpha DIALLO Date: Mon, 9 Sep 2024 17:17:33 +0200 Subject: [PATCH 3/3] add CHANGES entry Signed-off-by: Alpha DIALLO --- doc/changes/10881.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 doc/changes/10881.md diff --git a/doc/changes/10881.md b/doc/changes/10881.md new file mode 100644 index 00000000000..7c7bbcd7244 --- /dev/null +++ b/doc/changes/10881.md @@ -0,0 +1,2 @@ +- Fix `dune describe` when an executable is disabled with `enabled_if`. + (#10881, fixes #10779, @moyodiallo)