From a86c1fd9ef3a7c3a448183f1e904b65ff2bc6de6 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 19 Oct 2024 22:13:49 +0100 Subject: [PATCH] fix(pkg): dependency closure should be lazier (#11025) This is a pre-requesite for fixing OCaml syntax dune files. Signed-off-by: Rudi Grinberg --- src/dune_rules/pkg_rules.ml | 52 ++++++++++++------- .../test-cases/pkg/ocaml-syntax-gh10839.t | 7 +-- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/dune_rules/pkg_rules.ml b/src/dune_rules/pkg_rules.ml index 7b3bd8043d6..5464b2fc0ca 100644 --- a/src/dune_rules/pkg_rules.ml +++ b/src/dune_rules/pkg_rules.ml @@ -480,9 +480,10 @@ module Expander0 = struct type t = { name : Dune_pkg.Package_name.t ; paths : Path.t Paths.t - ; artifacts : Path.t Filename.Map.t + ; artifacts : Path.t Filename.Map.t Memo.t ; depends : (Variable.value Package_variable_name.Map.t * Path.t Paths.t) Package.Name.Map.t + Memo.t ; depexts : string list ; context : Context_name.t ; version : Package_version.t @@ -514,6 +515,9 @@ module Substitute = struct reconstruct everything that is needed to call our one and only substitution function. *) expander : Expander.t + ; depends : + (Variable.value Package_variable_name.Map.t * Path.t Paths.t) Package.Name.Map.t + ; artifacts : Path.t Filename.Map.t ; src : 'src ; dst : 'dst } @@ -523,12 +527,12 @@ module Substitute = struct let bimap t f g = { t with src = f t.src; dst = g t.dst } let is_useful_to ~memoize = memoize - let encode { expander; src; dst } input output : Sexp.t = + let encode { expander; depends; artifacts; src; dst } input output : Sexp.t = let e = let paths (p : Path.t Paths.t) = p.source_dir, p.target_dir, p.name in ( paths expander.paths - , expander.artifacts - , Package.Name.Map.to_list_map expander.depends ~f:(fun _ (m, p) -> m, paths p) + , artifacts + , Package.Name.Map.to_list_map depends ~f:(fun _ (m, p) -> m, paths p) , expander.version , expander.context , expander.env ) @@ -538,7 +542,7 @@ module Substitute = struct List [ Atom e; input src; output dst ] ;; - let action { expander; src; dst } ~ectx:_ ~eenv:_ = + let action { expander; depends = _; artifacts = _; src; dst } ~ectx:_ ~eenv:_ = let open Fiber.O in let* () = Fiber.return () in let env (var : Substs.Variable.t) = @@ -576,7 +580,11 @@ module Substitute = struct module A = Action_ext.Make (Spec) - let action expander ~src ~dst = A.action { Spec.expander; src; dst } + let action (expander : Expander.t) ~src ~dst = + let+ depends = expander.depends + and+ artifacts = expander.artifacts in + A.action { Spec.expander; depends; artifacts; src; dst } + ;; end let depexts_hint = function @@ -836,6 +844,7 @@ module Action_expander = struct ;; let expand_pkg_macro ~loc (paths : _ Paths.t) deps macro_invocation = + let* deps = deps in let { Package_variable.name = variable_name; scope } = match Package_variable.of_macro_invocation ~loc macro_invocation with | Ok package_variable -> package_variable @@ -943,7 +952,8 @@ module Action_expander = struct let dir = t.paths.source_dir in Memo.return @@ Ok (Path.relative dir program) | In_path -> - (match Filename.Map.find t.artifacts program with + let* artifacts = t.artifacts in + (match Filename.Map.find artifacts program with | Some s -> Memo.return @@ Ok s | None -> (let path = Global.env () |> Env_path.path in @@ -1038,9 +1048,9 @@ module Action_expander = struct in Dune_patch.action ~patch | Substitute (src, dst) -> - let+ src = + let* src = Expander.expand_pform_gen ~mode:Single expander src >>| Value.to_path ~dir - and+ dst = + and* dst = Expander.expand_pform_gen ~mode:Single expander dst >>| Value.to_path ~dir >>| Expander0.as_in_build_dir ~what:"substitute" ~loc:(String_with_vars.loc dst) @@ -1142,8 +1152,8 @@ module Action_expander = struct end let expander context (pkg : Pkg.t) = - let+ { Artifacts_and_deps.binaries; dep_info } = - Memo.push_stack_frame + let closure = + Memo.lazy_ ~human_readable_description:(fun () -> Pp.textf "Computing closure for package %S" @@ -1152,14 +1162,20 @@ module Action_expander = struct in let env = Pkg.exported_value_env pkg in let depends = - Package.Name.Map.add_exn - dep_info - pkg.info.name - (Pkg_info.variables pkg.info, pkg.paths) + Memo.Lazy.map closure ~f:(fun { Artifacts_and_deps.dep_info; _ } -> + Package.Name.Map.add_exn + dep_info + pkg.info.name + (Pkg_info.variables pkg.info, pkg.paths)) + |> Memo.Lazy.force + in + let artifacts = + let+ { Artifacts_and_deps.binaries; _ } = Memo.Lazy.force closure in + binaries in { Expander.paths = pkg.paths ; name = pkg.info.name - ; artifacts = binaries + ; artifacts ; context ; depends ; depexts = pkg.depexts @@ -1172,7 +1188,7 @@ module Action_expander = struct let expand context (pkg : Pkg.t) action = let+ action = - let* expander = expander context pkg in + let expander = expander context pkg in expand action ~expander >>| Action.chdir pkg.paths.source_dir in (* TODO copying is needed for build systems that aren't dune and those @@ -1337,7 +1353,7 @@ end = struct } in let+ exported_env = - let* expander = + let expander = Action_expander.expander (Package_universe.context_name package_universe) t in Memo.parallel_map exported_env ~f:(Action_expander.exported_env expander) diff --git a/test/blackbox-tests/test-cases/pkg/ocaml-syntax-gh10839.t b/test/blackbox-tests/test-cases/pkg/ocaml-syntax-gh10839.t index 255af28a799..8a6a6f83f2d 100644 --- a/test/blackbox-tests/test-cases/pkg/ocaml-syntax-gh10839.t +++ b/test/blackbox-tests/test-cases/pkg/ocaml-syntax-gh10839.t @@ -34,11 +34,8 @@ Dune file in OCaml syntax and a files directory should work Error: Dependency cycle between: - evaluating dune file "dune" in OCaml syntax -> _build/_private/default/.pkg/ocamlfind/target/cookie - -> Computing closure for package "base-bytes" - -> - package base-bytes - -> lock directory environment for context "default" - -> base environment for context "default" - -> loading findlib for context "default" + -> Loading all binaries in the lock directory for "default" + -> looking up binary "ocamlc" in context "default" -> loading the OCaml compiler for context "default" -> - evaluating dune file "dune" in OCaml syntax [1]