feat(pkg): single solve for all platforms

bin/pkg/lock.ml

@@ -115,9 +115,7 @@ module Platforms_by_message = struct
      are relevant, deduplicating common messages. *)
   type t = Solver_env.t list Message_map.t
 
-  let singleton message platform : t = Message_map.singleton message [ platform ]
   let to_list (t : t) : (Message.t * Solver_env.t list) list = Message_map.to_list t
-  let union_all ts : t = Message_map.union_all ts ~f:(fun _ a b -> Some (a @ b))
 
   let all_solver_errors_raising_if_any_manifest_errors t =
     let solver_errors, manifest_errors =
@@ -144,9 +142,24 @@ let solve_multiple_platforms
       ~portable_lock_dir
   =
   let open Fiber.O in
-  let solve_for_env env =
+  (* For portable lockdirs, use a portable base env (vars unset) + platform overlays.
+     For non-portable, use the full env + empty overlay. *)
+  let solver_env, platform_overlays =
+    if portable_lock_dir
+    then (
+      let portable_solver_env =
+        Solver_env.unset_multi
+          base_solver_env
+          Dune_lang.Package_variable_name.platform_specific
+      in
+      portable_solver_env, solve_for_platforms)
+    else base_solver_env, [ Solver_env.empty ]
+  in
+  (* Single solve for all platforms *)
+  let+ result =
     Dune_pkg.Opam_solver.solve_lock_dir
-      env
+      solver_env
+      ~platform_overlays
       version_preference
       repos
       ~pins
@@ -155,45 +168,20 @@ let solve_multiple_platforms
       ~selected_depopts
       ~portable_lock_dir
   in
-  let portable_solver_env =
-    Solver_env.unset_multi
-      base_solver_env
-      Dune_lang.Package_variable_name.platform_specific
-  in
-  let+ results =
-    Fiber.parallel_map solve_for_platforms ~f:(fun platform_env ->
-      let solver_env = Solver_env.extend portable_solver_env platform_env in
-      let+ solver_result = solve_for_env solver_env in
-      Result.map_error solver_result ~f:(fun message ->
-        let message : Platforms_by_message.Message.t =
-          match message with
-          | `Solve_error m -> Solve_error m
-          | `Manifest_error m -> Manifest_error m
-        in
-        Platforms_by_message.singleton message platform_env))
-  in
-  let solver_results, errors =
-    List.partition_map results ~f:(function
-      | Ok result -> Left result
-      | Error e -> Right e)
-  in
-  match solver_results, errors with
-  | [], [] -> Code_error.raise "Solver did not run for any platforms." []
-  | [], errors ->
-    `All_error
-      (Platforms_by_message.union_all errors
-       |> Platforms_by_message.all_solver_errors_raising_if_any_manifest_errors)
-  | x :: xs, errors ->
-    let merged_solver_result =
-      List.fold_left xs ~init:x ~f:Dune_pkg.Opam_solver.Solver_result.merge
+  match result with
+  | Ok solver_result -> `All_ok solver_result
+  | Error message ->
+    let error_message : Platforms_by_message.Message.t =
+      match message with
+      | `Solve_error m -> Solve_error m
+      | `Manifest_error m -> Manifest_error m
     in
-    if List.is_empty errors
-    then `All_ok merged_solver_result
-    else
-      `Partial
-        ( merged_solver_result
-        , Platforms_by_message.union_all errors
-          |> Platforms_by_message.all_solver_errors_raising_if_any_manifest_errors )
+    (* Associate error with all platforms (filtered to platform-specific vars only
+       for cleaner display). Single-solve fails for all platforms together. *)
+    let platform_envs =
+      List.map platform_overlays ~f:Solver_env.remove_all_except_platform_specific
+    in
+    `All_error (Platforms_by_message.Message_map.singleton error_message platform_envs)
 ;;
 
 let user_lock_dir_path path =
@@ -381,32 +369,6 @@ let solve_lock_dir
     match result with
     | `All_error messages -> Error messages
     | `All_ok solver_result -> Ok (solver_result, [])
-    | `Partial (solver_result, errors) ->
-      Log.info
-        "Solver found partial solution"
-        [ "error_count", Dyn.int (List.length errors) ];
-      let all_platforms =
-        List.concat_map errors ~f:snd |> List.sort_uniq ~compare:Solver_env.compare
-      in
-      Ok
-        ( solver_result
-        , [ Pp.nop
-          ; Pp.tag User_message.Style.Warning
-            @@ Pp.vbox
-            @@ Pp.concat
-                 ~sep:Pp.cut
-                 [ Pp.box
-                   @@ Pp.text "No package solution was found for some requsted platforms."
-                 ; Pp.nop
-                 ; Pp.box @@ Pp.text "Platforms with no solution:"
-                 ; Pp.box @@ Pp.enumerate all_platforms ~f:Solver_env.pp_oneline
-                 ; Pp.nop
-                 ; Pp.box
-                   @@ Pp.text
-                        "See the trace file with --trace-file for more details. \
-                         Configure platforms to solve for in the dune-workspace file."
-                 ]
-          ] )
   in
   match solver_result with
   | Error messages -> Fiber.return (Error (lock_dir_path, messages))
@@ -497,18 +459,26 @@ let solve
     if portable_lock_dir
     then
       User_error.raise
-        (List.concat_map errors ~f:(fun (path, errors) ->
+        (List.concat_map errors ~f:(fun (path, platforms_by_message) ->
+           let solver_errors =
+             Platforms_by_message.all_solver_errors_raising_if_any_manifest_errors
+               platforms_by_message
+           in
            [ Pp.box
              @@ Pp.textf
                   "Unable to solve dependencies while generating lock directory: %s"
                   (Path.to_string_maybe_quoted path)
-           ; Pp.vbox (Pp.concat ~sep:Pp.cut (pp_solve_errors_by_platforms errors))
+           ; Pp.vbox (Pp.concat ~sep:Pp.cut (pp_solve_errors_by_platforms solver_errors))
            ]))
     else
       User_error.raise
         ([ Pp.text "Unable to solve dependencies for the following lock directories:" ]
-         @ List.concat_map errors ~f:(fun (path, errors) ->
-           let messages = List.map errors ~f:fst in
+         @ List.concat_map errors ~f:(fun (path, platforms_by_message) ->
+           let solver_errors =
+             Platforms_by_message.all_solver_errors_raising_if_any_manifest_errors
+               platforms_by_message
+           in
+           let messages = List.map solver_errors ~f:fst in
            [ Pp.textf
                "Lock directory %s:"
                (Path.to_string_maybe_quoted (user_lock_dir_path path))

src/dune_pkg/lock_dir.ml

@@ -99,6 +99,17 @@ module Conditional_choice = struct
   let empty = []
   let singleton condition value = [ Conditional.make condition value ]
 
+  (* Create a conditional choice with a condition covering multiple platforms.
+     The condition is a disjunction of all provided solver_envs (filtered to
+     platform-specific vars only). This is used when a single solve produces
+     a result valid for all platforms. *)
+  let singleton_multi conditions value =
+    let condition =
+      List.map conditions ~f:Solver_env.remove_all_except_platform_specific
+    in
+    [ { Conditional.condition; value } ]
+  ;;
+
   (* A choice where a given value will be chosen unconditionally. This is only
      used to help support both portable and non-portable lockdirs with the same
      codebase and can be removed when portable lockdirs is the only option. *)
@@ -1177,7 +1188,7 @@ let create_latest_version
       ~ocaml
       ~repos
       ~expanded_solver_variable_bindings
-      ~solved_for_platform
+      ~solved_for_platforms
       ~portable_lock_dir
   =
   let packages =
@@ -1209,8 +1220,8 @@ let create_latest_version
       let complete = Int.equal (List.length repos) (List.length used) in
       complete, Some used
   in
-  let solved_for_platform_platform_specific_only =
-    Option.map solved_for_platform ~f:Solver_env.remove_all_except_platform_specific
+  let solved_for_platforms_platform_specific_only =
+    List.map solved_for_platforms ~f:Solver_env.remove_all_except_platform_specific
   in
   let expanded_solver_variable_bindings =
     match portable_lock_dir with
@@ -1227,8 +1238,7 @@ let create_latest_version
   ; ocaml
   ; repos = { complete; used }
   ; expanded_solver_variable_bindings
-  ; solved_for_platforms =
-      Loc.none, Option.to_list solved_for_platform_platform_specific_only
+  ; solved_for_platforms = Loc.none, solved_for_platforms_platform_specific_only
   }
 ;;
 

src/dune_pkg/lock_dir.mli

@@ -44,6 +44,10 @@ module Conditional_choice : sig
   val empty : 'a t
   val singleton : Solver_env.t -> 'a -> 'a t
 
+  (** Create a conditional choice with a condition covering multiple platforms.
+      Used when a single solve produces a result valid for all platforms. *)
+  val singleton_multi : Solver_env.t list -> 'a -> 'a t
+
   (** Returns the first value whose associated environment is a subset of the
       specified environment. *)
   val choose_for_platform : 'a t -> platform:Solver_env.t -> 'a option
@@ -81,6 +85,10 @@ module Pkg : sig
     -> Package_version.t option
     -> lock_dir:Path.t
     -> Path.Source.t
+
+  (** Merge platform-specific fields from two packages. Raises a code error if
+      the packages differ in non-platform-specific fields. *)
+  val merge_conditionals : t -> t -> t
 end
 
 module Repositories : sig
@@ -133,9 +141,7 @@ val create_latest_version
   -> ocaml:(Loc.t * Package_name.t) option
   -> repos:Opam_repo.t list option
   -> expanded_solver_variable_bindings:Solver_stats.Expanded_variable_bindings.t
-  -> solved_for_platform:Solver_env.t option
-       (* TODO: make the [solved_for_platform] argument non-optional when
-          portable lockdirs becomes the default *)
+  -> solved_for_platforms:Solver_env.t list
   -> portable_lock_dir:bool
   -> t
 

src/dune_pkg/lock_pkg.ml

@@ -432,7 +432,9 @@ let depexts_to_conditional_external_dependencies ~packages_in_solution package d
     { Lock_dir.Depexts.external_package_names; enabled_if })
 ;;
 
-let opam_package_to_lock_file_pkg
+(* Generate lock file entry for a single solver_env.
+   This is the core implementation that evaluates filters against one platform. *)
+let opam_package_to_lock_file_pkg_single
       solver_env
       stats_updater
       version_by_package_name
@@ -571,15 +573,10 @@ let opam_package_to_lock_file_pkg
   in
   (* Some lockfile fields contain a choice of values predicated on a set of
      platform variables to allow lockfiles to be portable across different
-     platforms. Each invocation of the solver produces a solution associated
-     with a single set of platform variables (those in [solver_env]).
-     [lockfile_field_choice value] creates a choice with a single possible
-     value predicated by the platform variables in [solver_env]. The
-     solver may be run multiple times, and the choice fields of lockfiles
-     will be merged such that different values can be chosen on different
-     platforms. *)
+     platforms. [lockfile_field_choice value] creates a choice with a single
+     possible value predicated by the platform this package is enabled on. *)
   let lockfile_field_choice value =
-    Lock_dir.Conditional_choice.singleton solver_env value
+    Lock_dir.Conditional_choice.singleton_multi [ solver_env ] value
   in
   let build_command =
     Option.map build_command ~f:lockfile_field_choice
@@ -618,7 +615,9 @@ let opam_package_to_lock_file_pkg
   in
   let depends = lockfile_field_choice depends in
   let enabled_on_platforms =
-    [ Solver_env.remove_all_except_platform_specific solver_env ]
+    if portable_lock_dir
+    then [ Solver_env.remove_all_except_platform_specific solver_env ]
+    else []
   in
   { Lock_dir.Pkg.build_command
   ; install_command
@@ -630,8 +629,12 @@ let opam_package_to_lock_file_pkg
   }
 ;;
 
+(* Public entry point: handles both single-platform and multi-platform cases.
+   For portable lockdirs with multiple solver_envs, we evaluate the opam file
+   against each platform separately and merge the results. This allows
+   platform-specific build commands, dependencies, etc. to be captured. *)
 let opam_package_to_lock_file_pkg
-      solver_env
+      solver_envs
       stats_updater
       version_by_package_name
       opam_package
@@ -641,14 +644,44 @@ let opam_package_to_lock_file_pkg
   =
   try
     Ok
-      (opam_package_to_lock_file_pkg
-         solver_env
-         stats_updater
-         version_by_package_name
-         opam_package
-         ~pinned
-         resolved_package
-         ~portable_lock_dir)
+      (match solver_envs with
+       | [] ->
+         Code_error.raise "opam_package_to_lock_file_pkg called with empty solver_envs" []
+       | [ solver_env ] ->
+         (* Single platform: use directly *)
+         opam_package_to_lock_file_pkg_single
+           solver_env
+           stats_updater
+           version_by_package_name
+           opam_package
+           ~pinned
+           resolved_package
+           ~portable_lock_dir
+       | _ when not portable_lock_dir ->
+         (* Non-portable with multiple envs: just use the first *)
+         let solver_env = List.hd solver_envs in
+         opam_package_to_lock_file_pkg_single
+           solver_env
+           stats_updater
+           version_by_package_name
+           opam_package
+           ~pinned
+           resolved_package
+           ~portable_lock_dir
+       | first_env :: rest_envs ->
+         (* Portable with multiple platforms: evaluate per-platform and merge *)
+         let to_pkg solver_env =
+           opam_package_to_lock_file_pkg_single
+             solver_env
+             stats_updater
+             version_by_package_name
+             opam_package
+             ~pinned
+             resolved_package
+             ~portable_lock_dir
+         in
+         List.fold_left rest_envs ~init:(to_pkg first_env) ~f:(fun acc env ->
+           Lock_dir.Pkg.merge_conditionals acc (to_pkg env)))
   with
   | User_error.E exn -> Error exn
 ;;

src/dune_pkg/lock_pkg.mli

@@ -8,9 +8,10 @@ val add_self_to_filter_env
   -> OpamVariable.variable_contents option
 
 (** Convert a selected opam package to a package that dune can save to the lock
-    directory *)
+    directory. For portable lockdirs, all solver_envs are used to set conditions
+    on conditional fields. The first solver_env is used for evaluating filters. *)
 val opam_package_to_lock_file_pkg
-  :  Solver_env.t
+  :  Solver_env.t list
   -> Solver_stats.Updater.t
   -> Package_version.t Package_name.Map.t
   -> OpamPackage.t