fix(package-deps): follow library-induced package closure

bin/describe/describe_external_lib_deps.ml

@@ -90,8 +90,8 @@ let is_external db name =
   | None -> true
   | Some t ->
     (match Dune_rules.Lib_info.status (Dune_rules.Lib.info t) with
-     | Installed_private | Public _ | Private _ -> false
-     | Installed -> true)
+     | Installed_private _ | Public _ | Private _ -> false
+     | Installed _ -> true)
 ;;
 
 let resolve_lib db name kind =

bin/describe/describe_workspace.ml

@@ -691,8 +691,8 @@ module Crawl = struct
       let name = Lib_id.name (Lib_info.lib_id info) in
       let public_name =
         match Lib_info.status info with
-        | Public _ | Installed -> Some (Lib.name lib)
-        | Private _ | Installed_private -> None
+        | Public _ | Installed _ -> Some (Lib.name lib)
+        | Private _ | Installed_private _ -> None
       in
       let src_dir = Lib_info.src_dir info in
       let obj_dir = Lib_info.obj_dir info in

bin/install_uninstall.ml

@@ -296,7 +296,7 @@ module File_ops_real (W : sig
   let process_dune_package ~get_version ~get_location ic ~src oc =
     let lb = Lexing.from_channel ic in
     let dune_version = Dune_lang.Syntax.greatest_supported_version_exn Stanza.syntax in
-    match Dune_package.Or_meta.parse src lb |> User_error.ok_exn with
+    match Dune_package.Or_meta.parse ~package:None src lb |> User_error.ok_exn with
     | Use_meta ->
       with_ppf oc ~f:(Dune_package.Or_meta.pp_use_meta ~dune_version);
       Fiber.return Done

doc/changes/changed/14373.md

@@ -1,5 +1,2 @@
-- `(deps (package ...))` now exposes only the directly declared packages to
-  the action's environment (`OCAMLPATH`, `PATH`, etc.). Previously, other
-  packages in the workspace could be discoverable via the shared install
-  staging area. Actions that relied on undeclared packages being visible
-  must declare them explicitly. (#14373, @Alizter)
+- `(package <pkg>)` now also includes the packages owning the library closures
+  of libraries in workspace and installed packages. (#14373, #16362, @Alizter)

doc/concepts/dependency-spec.rst

@@ -43,7 +43,15 @@ Dependencies in ``dune`` files can be specified using one of the following:
   ``OCAMLPATH``, stublibs on ``CAML_LD_LIBRARY_PATH``, and so on. For packages
   built by Dune's package management, Dune tracks their build-tree installation
   directories and those of their full lock-directory dependency closure.
-  Otherwise, transitive package dependencies must be listed separately.
+  This applies both to explicitly requested packages and to managed providers
+  reached through library dependencies.
+
+  For workspace packages and packages installed outside Dune's package
+  management, Dune also follows the transitive dependencies of their libraries
+  and includes the installed files from their owning packages. Unrelated
+  libraries in these additional packages do not become new roots for expansion.
+  This library-based expansion does not follow dependencies declared only in
+  package metadata.
 - ``(env_var <var>)`` depends on the value of the environment variable ``<var>``.
   If this variable becomes set, becomes unset, or changes value, the target
   will be rebuilt.

flake.nix

@@ -45,6 +45,9 @@
                     utop = osuper.utop.overrideAttrs {
                       dontGzipMan = true;
                     };
+                    lambda-term = osuper.lambda-term.overrideAttrs {
+                      dontGzipMan = true;
+                    };
                     odoc-parser = osuper.odoc-parser.overrideAttrs (old: {
                       doCheck = false;
                     });

src/dune_rules/dep_conf_eval.ml

@@ -221,6 +221,46 @@ let package loc pkg_name (context : Build_context.t) ~dune_version =
       }
 ;;
 
+(* Expands a package set by taking all the libraries in those packages and
+   collecting the libraries in their transitive closures and then adding the
+   packages that own those libraries. *)
+let expand_package_set context package_names =
+  let open Memo.O in
+  Memo.map_reduce_seq
+    (Package.Name.Set.to_seq package_names)
+    ~empty:package_names
+    ~combine:Package.Name.Set.union
+    ~f:(fun package_name ->
+      (* Find all the libraries present in a given package. *)
+      let* libs =
+        let* db = Package_db.create context in
+        Package_db.find_package db package_name
+        >>= function
+        | None -> Memo.return []
+        | Some (Installed { entries; _ }) ->
+          let* lib_db = Scope.DB.public_libs context in
+          Lib_name.Map.keys entries |> Memo.List.filter_map ~f:(Lib.DB.find lib_db)
+        | Some (Local pkg) ->
+          let* { Scope.DB.Lib_entry.Set.libraries; deprecated_library_names } =
+            Scope.DB.lib_entries_of_package context (Package.name pkg)
+          in
+          let+ redirected =
+            let* lib_db = Scope.DB.public_libs context in
+            Memo.List.filter_map deprecated_library_names ~f:(fun redirect ->
+              Lib.DB.find lib_db (Deprecated_library_name.old_public_name redirect))
+          in
+          List.map libraries ~f:Lib.Local.to_lib @ redirected
+        | Some (Build _) ->
+          (* We cannot know the libraries of a given dune pkg package so
+             easily, however the dependency on that package already expands
+             correctly to include the closure we are after. *)
+          Memo.return []
+      in
+      Lib.descriptive_closure libs ~with_pps:false ~for_:Compilation_mode.Ocaml
+      >>| List.filter_map ~f:(fun lib -> Lib_info.package (Lib.info lib))
+      >>| Package.Name.Set.of_list)
+;;
+
 let rec dep expander : Dep_conf.t -> _ = function
   | Include s ->
     (* TODO this is wrong. we shouldn't allow bindings here if we are in an
@@ -322,12 +362,35 @@ and combined_package_deps_builder expander pkgs =
        Expander.host_context expander >>| Context.name)
   in
   let context = Build_context.create ~name:host_name in
-  let* package_db = Action_builder.of_memo (Package_db.create context.name) in
-  let* classified =
+  let* requested =
     Action_builder.List.map pkgs ~f:(fun (swv, loc) ->
-      let* pkg = expand_package_name expander swv in
-      let+ found = Action_builder.of_memo (Package_db.find_package package_db pkg) in
-      loc, pkg, found)
+      let+ pkg = expand_package_name expander swv in
+      loc, pkg)
+  in
+  let* classified =
+    Action_builder.of_memo
+    @@
+    let open Memo.O in
+    let* package_db = Package_db.create context.name in
+    Package.Name.Set.of_list_map requested ~f:snd
+    |> expand_package_set context.name
+    >>| Package.Name.Set.to_list
+    >>= Memo.List.map ~f:(fun package ->
+      let loc =
+        (* CR-someday Alizter: The location here doesn't make sense for
+           packages from the lib-transitive closure and so defaults to none.
+           Consider pushing a stack frame in the action builder around this
+           whole affair in order to get some information on which package
+           required which transitively, when there is trouble materialising the
+           dependency. See
+           test/blackbox-tests/test-cases/package-materialization/install-dirs.t
+           for an example of a poor error message as a result of this. *)
+        List.find_map requested ~f:(fun (loc, requested_package) ->
+          if Package.Name.equal package requested_package then Some loc else None)
+        |> Option.value ~default:Loc.none
+      in
+      let+ found = Package_db.find_package package_db package in
+      loc, package, found)
   in
   let local_package_names =
     List.filter_map classified ~f:(fun (_, _, found) ->

src/dune_rules/dune_package.ml

@@ -215,7 +215,7 @@ module Lib = struct
       | _ -> assert false)
   ;;
 
-  let decode ~(lang : Vfile.Lang.Instance.t) ~base =
+  let decode ~(lang : Vfile.Lang.Instance.t) ~base ~package =
     let open Dune_lang.Decoder in
     let path = Dune_lang.Path.Local.decode ~dir:base in
     let field_l s x = field ~default:[] s (repeat x) in
@@ -304,8 +304,8 @@ module Lib = struct
          let enabled = Memo.return Lib_info.Enabled_status.Normal in
          let status =
            match Lib_name.analyze name with
-           | Private (_, _) -> Lib_info.Status.Installed_private
-           | Public (_, _) -> Lib_info.Status.Installed
+           | Private (_, _) -> Lib_info.Status.Installed_private package
+           | Public (_, _) -> Lib_info.Status.Installed package
          in
          let version = None in
          let local_main_module_name = main_module_name in
@@ -491,10 +491,10 @@ module Entry = struct
     | Deprecated_library_name d -> d.loc
   ;;
 
-  let cstrs ~lang ~dir =
+  let cstrs ~lang ~dir ~package =
     let open Dune_lang.Decoder in
     [ ( "library"
-      , let+ lib = Lib.decode ~lang ~base:dir in
+      , let+ lib = Lib.decode ~lang ~base:dir ~package in
         Library lib )
     ; ( "deprecated_library_name"
       , let+ x = Deprecated_library_name.decode in
@@ -553,7 +553,7 @@ type t =
   ; files : (Section.t * path list) list
   }
 
-let decode ~lang ~dir =
+let decode ~lang ~dir ~package =
   let open Dune_lang.Decoder in
   let+ name = field "name" Package.Name.decode
   and+ version = field_o "version" Package_version.decode
@@ -566,7 +566,7 @@ let decode ~lang ~dir =
     field ~default:[] "sites" (repeat (pair (located Site.decode) Section.decode))
   and+ files =
     field ~default:[] "files" (repeat (pair Section.decode (enter (repeat decode_path))))
-  and+ entries = leftover_fields_as_sums (Entry.cstrs ~lang ~dir) in
+  and+ entries = leftover_fields_as_sums (Entry.cstrs ~lang ~dir ~package) in
   let entries =
     List.map entries ~f:(fun e ->
       let e =
@@ -697,18 +697,18 @@ module Or_meta = struct
     | Dune_package p -> encode ~encoding ~dune_version p
   ;;
 
-  let decode ~lang ~dir =
+  let decode ~lang ~dir ~package =
     let open Dune_lang.Decoder in
     fields
       (let* use_meta = field_b "use_meta" in
        if use_meta
        then return Use_meta
        else
-         let+ package = decode ~lang ~dir in
+         let+ package = decode ~lang ~dir ~package in
          Dune_package package)
   ;;
 
-  let parse file lexbuf =
+  let parse ~package file lexbuf =
     let dir = Path.parent_exn file in
     let extensions = [ Dune_lang.Oxcaml.(syntax, latest_version) ] in
     let with_extensions decoder =
@@ -719,7 +719,7 @@ module Or_meta = struct
       Vfile.parse_contents lexbuf ~f:(fun lang ->
         String_with_vars.set_decoding_env
           (Pform.Env.initial ~stanza:lang.version ~extensions)
-          (with_extensions (decode ~lang ~dir)))
+          (with_extensions (decode ~lang ~dir ~package)))
     with
     | contents -> Ok contents
     | exception User_error.E message -> Error message
@@ -738,7 +738,7 @@ module Or_meta = struct
            ])
   ;;
 
-  let load file = Fs.with_lexbuf_from_file file ~f:(parse file)
+  let load ~package file = Fs.with_lexbuf_from_file file ~f:(parse ~package file)
 
   let pp_encoded ppf t =
     Format.fprintf

src/dune_rules/dune_package.mli

@@ -102,7 +102,13 @@ module Or_meta : sig
     -> unit
 
   val pp_use_meta : dune_version:Dune_lang.Syntax.Version.t -> Format.formatter -> unit
-  val parse : Path.t -> Lexbuf.t -> (t, User_message.t) result
-  val load : Path.t -> (t, User_message.t) result Memo.t
+
+  val parse
+    :  package:Package.Name.t option
+    -> Path.t
+    -> Lexbuf.t
+    -> (t, User_message.t) result
+
+  val load : package:Package.Name.t option -> Path.t -> (t, User_message.t) result Memo.t
   val to_dyn : t Dyn.builder
 end