diff --git a/bin/pkg/lock.ml b/bin/pkg/lock.ml index 95e0490a21f..d91a2093973 100644 --- a/bin/pkg/lock.ml +++ b/bin/pkg/lock.ml @@ -54,56 +54,20 @@ module Progress_indicator = struct let add_overlay (t : t) = Console.Status_line.add_overlay (Live (fun () -> pp t)) end -module Platforms_by_message = struct - module Message = struct - type t = - | Solve_error of User_message.Style.t Pp.t - | Manifest_error of User_message.t - - let to_dyn = function - | Solve_error message -> - Dyn.variant "Solve_error" [ Pp.to_dyn User_message.Style.to_dyn message ] - | Manifest_error message -> - Dyn.variant "Manifest_error" [ User_message.to_dyn message ] - ;; - - let compare a b = - match a, b with - | Solve_error a, Solve_error b -> Pp.compare ~compare:User_message.Style.compare a b - | Solve_error _, _ -> Lt - | _, Solve_error _ -> Gt - | Manifest_error a, Manifest_error b -> User_message.compare a b - ;; - end - - module Message_map = Map.Make (Message) - - (* Map messages to the list of platforms for which those messages are - relevant. If a dependency problem has no solution on any platform, it's - likely that the error from the solver will be identical across all - platforms. We don't want to print the same error message once for each - platform, so this type collects messages and the platforms for which they - are relevant, deduplicating common messages. *) - type t = Solver_env.t list Message_map.t +module Solve_error = struct + type t = + | Solve_error of User_message.Style.t Pp.t + | Manifest_error of User_message.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 = - List.partition_map (to_list t) ~f:(fun (message, platforms) -> - match message with - | Solve_error message -> Left (message, platforms) - | Manifest_error message -> Right message) - in - match manifest_errors with - | [] -> solver_errors - | message :: _ -> raise (User_error.E message) + (* Manifest errors are raised directly; solver errors are returned for + printing with the requested platform set. *) + let to_solver_error = function + | Solve_error message -> message + | Manifest_error message -> raise (User_error.E message) ;; end -let solve_multiple_platforms +let solve_with_platform_overlays base_solver_env version_preference repos @@ -115,9 +79,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 @@ -126,45 +105,21 @@ 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 : Solve_error.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 the error with the requested platforms (filtered to + platform-specific vars only for cleaner display). The single solve + fails for the requested platform set as a whole. *) + let platform_envs = + List.map platform_overlays ~f:Solver_env.remove_all_except_platform_specific + in + `All_error (error_message, platform_envs) ;; let user_lock_dir_path path = @@ -179,7 +134,6 @@ let summary_message ~lock_dir_path ~(lock_dir : Lock_dir.t) ~maybe_perf_stats - ~maybe_unsolved_platforms_message = if portable_lock_dir then ( @@ -234,43 +188,53 @@ let summary_message ; pp_package_set packages ])) in - (Pp.tag - User_message.Style.Success - (Pp.textf - "Solution for %s" - (Path.to_string_maybe_quoted (user_lock_dir_path lock_dir_path))) - :: Pp.nop - :: Pp.text "Dependencies common to all supported platforms:" - :: pp_package_set common_packages - :: (maybe_uncommon_packages @ maybe_perf_stats)) - @ maybe_unsolved_platforms_message) + Pp.tag + User_message.Style.Success + (Pp.textf + "Solution for %s" + (Path.to_string_maybe_quoted (user_lock_dir_path lock_dir_path))) + :: Pp.nop + :: Pp.text "Dependencies common to all supported platforms:" + :: pp_package_set common_packages + :: (maybe_uncommon_packages @ maybe_perf_stats)) else - (Pp.tag - User_message.Style.Success - (Pp.textf - "Solution for %s:" - (Path.to_string_maybe_quoted (user_lock_dir_path lock_dir_path))) - :: (match Lock_dir.Packages.to_pkg_list lock_dir.packages with - | [] -> Pp.tag User_message.Style.Warning @@ Pp.text "(no dependencies to lock)" - | packages -> pp_packages packages) - :: maybe_perf_stats) - @ maybe_unsolved_platforms_message + Pp.tag + User_message.Style.Success + (Pp.textf + "Solution for %s:" + (Path.to_string_maybe_quoted (user_lock_dir_path lock_dir_path))) + :: (match Lock_dir.Packages.to_pkg_list lock_dir.packages with + | [] -> Pp.tag User_message.Style.Warning @@ Pp.text "(no dependencies to lock)" + | packages -> pp_packages packages) + :: maybe_perf_stats ;; -let pp_solve_errors_by_platforms platforms_by_message = - List.map platforms_by_message ~f:(fun (message, platforms) -> - Pp.concat - ~sep:Pp.cut - [ Pp.nop - ; Pp.box - @@ Pp.text - "The dependency solver failed to find a solution for the following \ - platforms:" - ; Pp.enumerate platforms ~f:Solver_env.pp_oneline - ; Pp.box @@ Pp.text "...with this error:" - ; message - ] - |> Pp.vbox) +(* A failed joint solve does not prove that every requested platform fails on + its own: the solver reports the first conflict it finds for the requested + platform set. *) +let pp_solve_error (message, platforms) = + Pp.concat + ~sep:Pp.cut + [ Pp.nop + ; Pp.box + @@ Pp.text + "The dependency solver failed to find a solution for the requested platforms:" + ; Pp.enumerate platforms ~f:Solver_env.pp_oneline + ; Pp.box @@ Pp.text "...with this error:" + ; message + ] + |> Pp.vbox +;; + +(* Suggest narrowing the platform set when support for every requested + platform is unnecessary. *) +let solve_for_platforms_hint = + [ Pp.text "If you don't need support for every requested platform, change" + ; Pp.text "(solve_for_platforms ...) in dune-workspace to only include the" + ; Pp.concat + ~sep:Pp.space + [ Pp.text "platforms you need, then rerun"; User_message.command "dune pkg lock" ] + ] ;; let solve_lock_dir @@ -333,7 +297,7 @@ let solve_lock_dir let time_solve_start = Time.now () in progress_state := Some Progress_indicator.Per_lockdir.State.Solving; let* result = - solve_multiple_platforms + solve_with_platform_overlays solver_env (Pkg_common.Version_preference.choose ~from_arg:version_preference @@ -350,38 +314,12 @@ let solve_lock_dir in let solver_result = 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." - ] - ] ) + | `All_error error -> Error error + | `All_ok solver_result -> Ok solver_result in match solver_result with - | Error messages -> Fiber.return (Error (lock_dir_path, messages)) - | Ok (solver_result, maybe_unsolved_platforms_message) -> + | Error error -> Fiber.return (Error (lock_dir_path, error)) + | Ok solver_result -> let { Dune_pkg.Opam_solver.Solver_result.lock_dir ; files ; pinned_packages @@ -407,12 +345,7 @@ let solve_lock_dir in let summary_message = User_message.make - (summary_message - ~portable_lock_dir - ~lock_dir_path - ~lock_dir - ~maybe_perf_stats - ~maybe_unsolved_platforms_message) + (summary_message ~portable_lock_dir ~lock_dir_path ~lock_dir ~maybe_perf_stats) in progress_state := None; let+ lock_dir = Lock_dir.compute_missing_checksums ~pinned_packages lock_dir in @@ -468,22 +401,22 @@ let solve if portable_lock_dir then User_error.raise - (List.concat_map errors ~f:(fun (path, errors) -> + ~hints:solve_for_platforms_hint + (List.concat_map errors ~f:(fun (path, (error, platforms)) -> [ 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_solve_error (Solve_error.to_solver_error error, platforms)) ])) 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, (error, _platforms)) -> [ Pp.textf "Lock directory %s:" (Path.to_string_maybe_quoted (user_lock_dir_path path)) - ; Pp.vbox (Pp.concat ~sep:Pp.cut messages) + ; Pp.vbox (Solve_error.to_solver_error error) ])) | Ok write_disks_with_summaries -> let write_disk_list, summary_messages = List.split write_disks_with_summaries in diff --git a/doc/changes/changed/15981.md b/doc/changes/changed/15981.md new file mode 100644 index 00000000000..1a138e852bd --- /dev/null +++ b/doc/changes/changed/15981.md @@ -0,0 +1,4 @@ +- `dune pkg lock` now fails without writing a lock directory when any platform + requested by `solve_for_platforms` cannot be solved, instead of producing a + partial lock directory containing only the successful platforms. (#15981, + @Alizter) diff --git a/src/dune_pkg/lock_dir.ml b/src/dune_pkg/lock_dir.ml index 41dc56fe328..72ea99c642e 100644 --- a/src/dune_pkg/lock_dir.ml +++ b/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. *) @@ -1048,24 +1059,6 @@ module Packages = struct List.fold_left pkg.enabled_on_platforms ~init:acc ~f:(fun acc platform -> Solver_env.Map.add_multi acc platform pkg)) ;; - - let merge a b = - Package_name.Map.merge a b ~f:(fun _ a b -> - match a, b with - | None, None -> - (* unreachable *) - None - | Some x, None | None, Some x -> Some x - | Some a, Some b -> - Some - (Package_version.Map.merge a b ~f:(fun _ a b -> - match a, b with - | None, None -> - (* unreachable *) - None - | Some x, None | None, Some x -> Some x - | Some a, Some b -> Some (Pkg.merge_conditionals a b)))) - ;; end type t = @@ -1177,7 +1170,7 @@ let create_latest_version ~ocaml ~repos ~expanded_solver_variable_bindings - ~solved_for_platform + ~solved_for_platforms ~portable_lock_dir = let packages = @@ -1209,8 +1202,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 +1220,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 } ;; @@ -1825,28 +1817,6 @@ let compute_missing_checksums t ~pinned_packages = { t with packages } ;; -let merge_conditionals a b = - let packages = Packages.merge a.packages b.packages in - let solved_for_platforms = - let a_loc, a_solved_for_platforms = a.solved_for_platforms in - let b_loc, b_solved_for_platforms = b.solved_for_platforms in - Loc.span a_loc b_loc, a_solved_for_platforms @ b_solved_for_platforms - in - let normalize t = - { t with - packages = Package_name.Map.empty - ; expanded_solver_variable_bindings = Solver_stats.Expanded_variable_bindings.empty - ; solved_for_platforms = Loc.none, [] - } - in - if not (equal (normalize a) (normalize b)) - then - Code_error.raise - "Platform-specific lockdirs differ in a non-platform-specific way" - [ "lockdir_1", to_dyn a; "lockdir_2", to_dyn b ]; - { a with packages; solved_for_platforms } -;; - let loc_in_source_tree loc = loc |> Loc.map_pos ~f:(fun ({ pos_fname; _ } as pos) -> diff --git a/src/dune_pkg/lock_dir.mli b/src/dune_pkg/lock_dir.mli index c9032073fee..5c1d1d584ee 100644 --- a/src/dune_pkg/lock_dir.mli +++ b/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 @@ -187,10 +193,6 @@ val transitive_dependency_closure archive urls but no checksum. *) val compute_missing_checksums : t -> pinned_packages:Package_name.Set.t -> t Fiber.t -(** Combine the platform-specific parts of a pair of lockdirs, throwing a code - error if the lockdirs differ in a non-platform-specific way. *) -val merge_conditionals : t -> t -> t - (** Returns the packages contained in the solution on the given platform. If the lockdir does not contain a solution compatible with the given platform then a [User_error] is raised. *) diff --git a/src/dune_pkg/lock_pkg.ml b/src/dune_pkg/lock_pkg.ml index 64c37f5f229..4365c041bc4 100644 --- a/src/dune_pkg/lock_pkg.ml +++ b/src/dune_pkg/lock_pkg.ml @@ -463,7 +463,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 @@ -602,15 +604,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 @@ -649,7 +646,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 @@ -661,8 +660,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 @@ -672,14 +675,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 ;; diff --git a/src/dune_pkg/lock_pkg.mli b/src/dune_pkg/lock_pkg.mli index b0a807969dc..d2f52ffaf91 100644 --- a/src/dune_pkg/lock_pkg.mli +++ b/src/dune_pkg/lock_pkg.mli @@ -24,9 +24,10 @@ val local_package_dependencies -> (Package_name.t list, Resolve_opam_formula.unsatisfied_formula) result (** 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 diff --git a/src/dune_pkg/opam_solver.ml b/src/dune_pkg/opam_solver.ml index 518cb8656b2..0d051fa3363 100644 --- a/src/dune_pkg/opam_solver.ml +++ b/src/dune_pkg/opam_solver.ml @@ -86,6 +86,8 @@ module Context = struct ; local_packages : local_package Package_name.Map.t Lazy.t ; local_constraints : (Package_name.t, local_package list) Table.t Lazy.t ; solver_env : Solver_env.t + (* Base solver env. Full platform-specific envs are computed by extending + this with the platform's own env. *) ; dune_version : OpamPackage.Version.t ; stats_updater : Solver_stats.Updater.t ; candidates_cache : (Package_name.t, candidates) Fiber.Cache.t @@ -148,7 +150,11 @@ module Context = struct ; version_preference ; local_packages ; pinned_packages - ; solver_env = Solver_env.add_sentinel_values_for_unset_platform_vars solver_env + ; solver_env = + Solver_env.add_sentinel_values_for_unset_platform_vars solver_env + (* The platform envs don't need sentinel values - they only contain + platform-specific vars that will override the sentinels in solver_env + when extended. *) ; dune_version = Dune_dep.version ; stats_updater ; candidates_cache @@ -159,6 +165,10 @@ module Context = struct } ;; + (* Compute the full platform-specific env by extending the base env with the + platform's own (platform-specific) env. *) + let platform_env t platform = Solver_env.extend t.solver_env platform + let pp_rejection = function | Unavailable -> Pp.paragraph "Availability condition not satisfied" | Refuted_by pkg -> @@ -173,31 +183,36 @@ module Context = struct | Invalid_argument msg -> Error (`Not_a_bool msg) ;; + let is_opam_available_in_env t ~solver_env opam = + let package = OpamFile.OPAM.package opam in + let available = OpamFile.OPAM.available opam in + match + OpamFilter.partial_eval + (Solver_env.to_env solver_env + |> Solver_stats.Updater.wrap_env t.stats_updater + |> Lock_pkg.add_self_to_filter_env package) + available + |> eval_to_bool + with + | Ok available -> available + | Error (`Not_a_bool msg) -> + (let package_string = OpamPackage.to_string package in + let available_string = OpamFilter.to_string available in + User_warning.emit + [ Pp.textf + "Ignoring package %s as its \"available\" filter can't be resolved to a \ + boolean value." + package_string + ; Pp.textf "available: %s" available_string + ; Pp.text msg + ]); + false + ;; + let is_opam_available t opam = let package = OpamFile.OPAM.package opam in Table.find_or_add t.available_cache package ~f:(fun (_ : OpamPackage.t) -> - let available = OpamFile.OPAM.available opam in - match - OpamFilter.partial_eval - (Solver_env.to_env t.solver_env - |> Solver_stats.Updater.wrap_env t.stats_updater - |> Lock_pkg.add_self_to_filter_env package) - available - |> eval_to_bool - with - | Ok available -> available - | Error (`Not_a_bool msg) -> - (let package_string = OpamFile.OPAM.package opam |> OpamPackage.to_string in - let available_string = OpamFilter.to_string available in - User_warning.emit - [ Pp.textf - "Ignoring package %s as its \"available\" filter can't be resolved to a \ - boolean value." - package_string - ; Pp.textf "available: %s" available_string - ; Pp.text msg - ]); - false) + is_opam_available_in_env t ~solver_env:t.solver_env opam) ;; let available_or_error t opam_file = @@ -209,6 +224,12 @@ module Context = struct if is_opam_available t opam_file then Ok opam_file else Error Unavailable ;; + (* Check availability for a specific platform. Used for multi-platform solving + where packages may be available on some platforms but not others. *) + let is_available_for_platform t ~platform opam = + is_opam_available_in_env t ~solver_env:(platform_env t platform) opam + ;; + let pinned_candidate t resolved_package = let version = Resolved_package.package resolved_package |> OpamPackage.version in let available = @@ -222,7 +243,8 @@ module Context = struct { available; resolved } ;; - let filter_deps t package filtered_formula = + (* Filter deps using a specific solver_env *) + let filter_deps_with_env t ~solver_env package filtered_formula = (* Add additional constraints to the formula. This works in two steps. First identify all the additional constraints applied to packages which appear in the current package's dependency formula. Then each additional @@ -245,13 +267,24 @@ module Context = struct |> Package_name.of_opam_package_name |> Package_name.Map.mem (Lazy.force t.local_packages) in - let with_test = package_is_local && with_test t.solver_env in - Solver_env.to_env t.solver_env + let with_test = package_is_local && with_test solver_env in + Solver_env.to_env solver_env |> Solver_stats.Updater.wrap_env t.stats_updater |> Lock_pkg.add_self_to_filter_env package |> Resolve_opam_formula.apply_filter ~with_test ~formula:filtered_formula ;; + (* Filter deps for local packages using primary solver_env *) + let filter_deps_local t package filtered_formula = + filter_deps_with_env t ~solver_env:t.solver_env package filtered_formula + ;; + + (* Filter deps for a specific platform *) + let filter_deps t ~platform package filtered_formula = + let solver_env = platform_env t platform in + filter_deps_with_env t ~solver_env package filtered_formula + ;; + exception Found of Package_name.t let try_refute t package = @@ -350,6 +383,37 @@ module Context = struct res.available ;; + (* Get all candidates with their opam files, without pre-filtering by availability. + Used for multi-platform solving where availability is checked per-platform. *) + let candidates_unfiltered t name = + let* () = Fiber.return () in + let key = Package_name.of_opam_package_name name in + match Package_name.Map.find (Lazy.force t.local_packages) key with + | Some local_package -> + let priority = Priority.allowed local_package.version in + Fiber.return [ priority, local_package.opam_file ] + | None -> + let+ res = + Fiber.Cache.find_or_add t.candidates_cache key ~f:(fun () -> + match Package_name.Map.find t.pinned_packages key with + | Some resolved_package -> Fiber.return (pinned_candidate t resolved_package) + | None -> repo_candidate t name) + in + (* Return all versions from resolved, with priority info *) + OpamPackage.Version.Map.bindings res.resolved + |> List.map ~f:(fun (version, resolved_pkg) -> + let opam = Resolved_package.opam_file resolved_pkg in + let avoid = + List.mem + (OpamFile.OPAM.flags opam) + OpamTypes.Pkgflag_AvoidVersion + ~equal:Poly.equal + in + { Priority.version; avoid }, opam) + |> List.sort ~compare:(fun (x, _) (y, _) -> + Priority.compare t.version_preference x y) + ;; + let user_restrictions : t -> OpamPackage.Name.t -> OpamFormula.version_constraint option = fun t pkg -> @@ -444,7 +508,7 @@ module Solver = struct end type role = - | Real of OpamPackage.Name.t + | Real of OpamPackage.Name.t * Solver_env.t | Virtual of Virtual_id.t * impl list and real_impl = @@ -466,11 +530,27 @@ module Solver = struct | Reject of OpamPackage.t | Dummy (* Used for diagnostics *) + (* Deduplicate a list of dependencies by package name for display purposes. + This avoids showing the same package multiple times for different platforms. *) + let deduplicate_deps_by_name deps = + let seen = ref OpamPackage.Name.Set.empty in + List.filter deps ~f:(fun (d : dependency) -> + match d.drole with + | Virtual _ -> true + | Real (name, _platform) -> + if OpamPackage.Name.Set.mem name !seen + then false + else ( + seen := OpamPackage.Name.Set.add name !seen; + true)) + ;; + let rec pp_version = function | RealImpl impl -> Pp.text (OpamPackage.Version.to_string (OpamPackage.version impl.pkg)) | Reject pkg -> Pp.text (OpamPackage.version_to_string pkg) | VirtualImpl (_i, deps) -> + let deps = deduplicate_deps_by_name deps in Pp.concat_map ~sep:(Pp.char '&') deps ~f:(fun d -> pp_role d.drole) | Dummy -> Pp.text "(no version)" @@ -481,7 +561,9 @@ module Solver = struct | Dummy -> Pp.text "(no solution found)" and pp_role = function - | Real name -> Pp.text (OpamPackage.Name.to_string name) + | Real (name, _platform) -> + (* Don't show platform in user-facing output for now *) + Pp.text (OpamPackage.Name.to_string name) | Virtual (_, impls) -> Pp.concat_map ~sep:(Pp.char '|') impls ~f:pp_impl ;; @@ -493,7 +575,10 @@ module Solver = struct let compare a b = match a, b with - | Real a, Real b -> Ordering.of_int (OpamPackage.Name.compare a b) + | Real (a_name, a_platform), Real (b_name, b_platform) -> + (match Ordering.of_int (OpamPackage.Name.compare a_name b_name) with + | Eq -> Solver_env.compare a_platform b_platform + | x -> x) | Virtual (a, _), Virtual (b, _) -> Virtual_id.compare a b | Real _, Virtual _ -> Lt | Virtual _, Real _ -> Gt @@ -510,8 +595,8 @@ module Solver = struct let user_restrictions t context = match t with | Virtual _ -> None - | Real role -> - Context.user_restrictions context role + | Real (name, _platform) -> + Context.user_restrictions context name |> Option.map ~f:(fun f -> { Restriction.kind = Ensure; expr = OpamFormula.Atom f }) ;; @@ -521,13 +606,13 @@ module Solver = struct let rejects role context = match role with | Virtual _ -> Fiber.return ([], []) - | Real role -> + | Real (name, _platform) -> let+ rejects = - Context.candidates context role + Context.candidates context name >>| List.filter_map ~f:(function | _, Ok _ -> None | { Priority.version; _ }, Error reason -> - let pkg = OpamPackage.create role version in + let pkg = OpamPackage.create name version in Some (Reject pkg, reason)) in let notes = [] in @@ -569,7 +654,7 @@ module Solver = struct let compare_version a b = match a, b with | RealImpl a, RealImpl b -> - (* CR-someday rgrinberg: shouldn't we take our version preference into account here? *) + (* CR rgrinberg: shouldn't we take our version preference into account here? *) Ordering.of_int (OpamPackage.compare a.pkg b.pkg) | RealImpl _, _ -> Gt | _, RealImpl _ -> Lt @@ -619,11 +704,12 @@ module Solver = struct ;; (* Turn an opam dependency formula into a 0install list of dependencies. *) - let list_deps ~importance ~rank deps = + let list_deps ~importance ~rank ~platform deps = let rec aux (formula : _ OpamTypes.generic_formula) = match formula with | Empty -> [] - | Atom (name, restrictions) -> [ { drole = Real name; restrictions; importance } ] + | Atom (name, restrictions) -> + [ { drole = Real (name, platform); restrictions; importance } ] | Block x -> aux x | And (x, y) -> aux x @ aux y | Or _ as o -> @@ -639,30 +725,35 @@ module Solver = struct aux deps ;; - (* Get all the candidates for a role. *) + (* Get all the candidates for a role. For multi-platform solving, + we check availability per-platform so packages that are only + available on certain platforms (e.g., linux-only, macos-only) work. *) let implementations role context = match role with | Virtual (_, impls) -> Fiber.return impls - | Real role -> - Context.candidates context role - >>| List.filter_map ~f:(function - | _, Error _rejection -> None - | { Priority.version; avoid }, Ok opam -> - let pkg = OpamPackage.create role version in + | Real (name, platform) -> + Context.candidates_unfiltered context name + >>| List.filter_map ~f:(fun (priority, opam) -> + (* Check availability for this specific platform *) + if not (Context.is_available_for_platform context ~platform opam) + then None + else ( + let { Priority.version; avoid } = priority in + let pkg = OpamPackage.create name version in (* Note: we ignore depopts here: see opam/doc/design/depopts-and-features *) let requires = lazy (let rank = Rank.assign () in let make_deps importance xform deps = - Context.filter_deps context pkg deps + Context.filter_deps context ~platform pkg deps |> xform - |> list_deps ~importance ~rank + |> list_deps ~importance ~rank ~platform in (OpamFile.OPAM.depends opam |> make_deps Ensure ensure) @ (OpamFile.OPAM.conflicts opam |> make_deps Prevent prevent)) in let conflict_class = OpamFile.OPAM.conflict_class opam in - Some (RealImpl { pkg; avoid; requires; conflict_class })) + Some (RealImpl { pkg; avoid; requires; conflict_class }))) ;; let meets_restriction impl { Restriction.kind; expr } = @@ -753,38 +844,154 @@ module Solver = struct end module Conflict_classes = struct - type t = { mutable groups : Sat.lit list ref OpamPackage.Name.Map.t } + (* Key is (conflict_class_name, platform) to ensure conflict classes + are enforced per-platform, not across platforms. In multi-platform + solving, it's valid to select the same package with a conflict class + on multiple platforms, but within each platform, at most one package + with the conflict class can be selected. *) + module Key = struct + type t = OpamPackage.Name.t * Solver_env.t + + let compare (name1, plat1) (name2, plat2) = + match Ordering.of_int (OpamPackage.Name.compare name1 name2) with + | Eq -> Solver_env.compare plat1 plat2 + | x -> x + ;; + + let to_dyn (name, plat) = + Dyn.Tuple + [ Dyn.string (OpamPackage.Name.to_string name); Solver_env.to_dyn plat ] + ;; + end - let create () = { groups = OpamPackage.Name.Map.empty } + module Key_map = Map.Make (Key) - let var t name = - match OpamPackage.Name.Map.find_opt name t.groups with + type t = { mutable groups : Sat.lit list ref Key_map.t } + + let create () = { groups = Key_map.empty } + + let var t key = + match Key_map.find t.groups key with | Some v -> v | None -> let v = ref [] in - t.groups <- OpamPackage.Name.Map.add name v t.groups; + t.groups <- Key_map.set t.groups key v; v ;; - (* Add [impl] to its conflict groups, if any. *) - let process t impl_var impl = - Input.Impl.conflict_class impl - |> List.iter ~f:(fun name -> - let impls = var t name in - impls := impl_var :: !impls) + (* Add [impl] to its conflict groups, if any. + [role] is used to extract the platform for the group key. + Virtual roles never carry conflict classes (see [Input.Impl.conflict_class] + which returns [] for VirtualImpl), so there's nothing to do for them. *) + let process t role impl_var impl = + match role with + | Input.Virtual _ -> () + | Input.Real (_, platform) -> + Input.Impl.conflict_class impl + |> List.iter ~f:(fun name -> + let impls = var t (name, platform) in + impls := impl_var :: !impls) ;; (* Call this at the end to add the final clause with all discovered groups. [t] must not be used after this. *) + let seal t = + Key_map.iter t.groups ~f:(fun impls -> + match !impls with + | _ :: _ :: _ -> + let (_ : Sat.at_most_one_clause) = Sat.at_most_one !impls in + () + | _ -> ()) + ;; + end + + module Cross_platform_version = struct + (* Implements @art-w's cross-platform version equality constraint from + https://github.com/ocaml/dune/issues/13647. For each (package_name, + version) appearing as an impl on any platform, introduce a "somewhere" + SAT variable that is true iff some platform selected [package_name] at + [version]. Two kinds of clauses are added: + - For each per-platform impl: [impl_var implies somewhere]. + - At-most-one across the somewhere-vars of each [name]. + Together these force every platform that selects [name] to select the + same version, while still permitting platforms to omit a package + entirely (e.g. unix-only packages on Windows). *) + module Key = struct + type t = OpamPackage.Name.t * OpamPackage.Version.t + + let compare (n1, v1) (n2, v2) = + match Ordering.of_int (OpamPackage.Name.compare n1 n2) with + | Eq -> Ordering.of_int (OpamPackage.Version.compare v1 v2) + | x -> x + ;; + + let to_dyn (n, v) = + Dyn.Tuple + [ Dyn.string (OpamPackage.Name.to_string n) + ; Dyn.string (OpamPackage.Version.to_string v) + ] + ;; + end + + module Key_map = Map.Make (Key) + + type t = + { sat : Sat.t + ; mutable somewhere_vars : Sat.lit Key_map.t + ; (* For each package name, the list of somewhere-vars across its + versions. Used at seal time to add the at-most-one clauses. *) + mutable per_name : Sat.lit list ref OpamPackage.Name.Map.t + } + + let create sat = + { sat; somewhere_vars = Key_map.empty; per_name = OpamPackage.Name.Map.empty } + ;; + + let somewhere_var t name version = + let key = name, version in + match Key_map.find t.somewhere_vars key with + | Some v -> v + | None -> + (* The user data attached to this SAT variable is opaque to the SAT + engine — we use Dummy since the variable doesn't represent a real + package selection. *) + let v = Sat.add_variable t.sat Input.Dummy in + t.somewhere_vars <- Key_map.set t.somewhere_vars key v; + let bucket = + match OpamPackage.Name.Map.find_opt name t.per_name with + | Some b -> b + | None -> + let b = ref [] in + t.per_name <- OpamPackage.Name.Map.add name b t.per_name; + b + in + bucket := v :: !bucket; + v + ;; + + (* If this impl represents a real (name, version, platform) selection, + add the implication [impl_var implies somewhere(name, version)]. Other + impl kinds (Virtual, Reject, Dummy) don't participate. *) + let process t impl_var (impl : Input.impl) = + match impl with + | RealImpl real_impl -> + let pkg = real_impl.pkg in + let name = OpamPackage.name pkg in + let version = OpamPackage.version pkg in + let som = somewhere_var t name version in + Sat.implies t.sat impl_var [ som ] ~reason:"cross-platform version equality" + | VirtualImpl _ | Reject _ | Dummy -> () + ;; + + (* Add at-most-one over the somewhere-vars per package name. Call after + all impls have been processed. *) let seal t = OpamPackage.Name.Map.iter - (fun _ impls -> - match !impls with - | _ :: _ :: _ -> - let (_ : Sat.at_most_one_clause) = Sat.at_most_one !impls in - () - | _ -> ()) - t.groups + (fun _name bucket -> + match !bucket with + | [] | [ _ ] -> () + | vars -> ignore (Sat.at_most_one vars : Sat.at_most_one_clause)) + t.per_name ;; end @@ -794,6 +1001,7 @@ module Solver = struct (* For each (iface, source) we have a list of implementations. *) let impl_cache = Fiber.Cache.create (module Input.Role) in let conflict_classes = Conflict_classes.create () in + let cross_platform_versions = Cross_platform_version.create sat in let avoids = ref [] in let+ () = let rec lookup_impl expand_deps role = @@ -808,7 +1016,8 @@ module Solver = struct in let+ () = Fiber.parallel_iter !impls ~f:(fun { var = impl_var; impl } -> - Conflict_classes.process conflict_classes impl_var impl; + Conflict_classes.process conflict_classes role impl_var impl; + Cross_platform_version.process cross_platform_versions impl_var impl; if Input.Impl.avoid impl then avoids := impl_var :: !avoids; match expand_deps with | `No_expand -> Fiber.return () @@ -881,6 +1090,7 @@ module Solver = struct (* All impl_candidates have now been added, so snapshot the cache. *) in Conflict_classes.seal conflict_classes; + Cross_platform_version.seal cross_platform_versions; (match max_avoids, !avoids with | None, _ | _, [] -> () | Some max_avoids, avoids -> @@ -1095,7 +1305,7 @@ module Solver = struct If [t] selected a better version anyway then we don't need to report this rejection. *) let affected_selection t impl = match t.selected_impl with - (* CR-someday rgrinberg: take account version preference here? *) + (* CR rgrinberg: take account version preference here? *) | Some selected when Input.Impl.compare_version selected impl = Gt -> false | _ -> true ;; @@ -1327,6 +1537,16 @@ module Solver = struct |> Option.iter ~f:(Component.apply_user_restriction component)) ;; + (* Check if two roles refer to the same package (ignoring the platform). + Used for conflict class checking - same package on different platforms + should not be considered in conflict with itself. *) + let same_package_name (role1 : Input.Role.t) (role2 : Input.Role.t) = + match role1, role2 with + | Real (name1, _), Real (name2, _) -> OpamPackage.Name.equal name1 name2 + | Virtual (id1, _), Virtual (id2, _) -> Input.Virtual_id.equal id1 id2 + | Real _, Virtual _ | Virtual _, Real _ -> false + ;; + (** For each selected implementation with a conflict class, reject all candidates with the same class. *) let check_conflict_classes report = @@ -1347,8 +1567,7 @@ module Solver = struct Input.Impl.conflict_class impl |> List.find_map ~f:(fun cl -> match OpamPackage.Name.Map.find_opt cl classes with - | Some other_role - when not (Ordering.is_eq (Input.Role.compare role other_role)) -> + | Some other_role when not (same_package_name role other_role) -> Some (`ClassConflict (other_role, cl)) | _ -> None))) ;; @@ -1363,7 +1582,7 @@ module Solver = struct let get_selected role (sel : Solver.selection) = let diagnostics = lazy (explain role) in let impl = if sel.impl = Input.Dummy then None else Some sel.impl in - (* CR-someday rgrinberg: Are we recomputing things here? *) + (* CR rgrinberg: Are we recomputing things here? *) let* impl_candidates = Input.implementations role context in let+ rejects, feed_problems = Input.Role.rejects role context in Component.create @@ -1386,18 +1605,23 @@ module Solver = struct ;; end - let solve context pkgs = + let solve context pkgs ~platforms = let req = - match pkgs with - | [ pkg ] -> Input.Real pkg - | pkgs -> - let impl : Input.Impl.t = - let depends = + match pkgs, platforms with + | [ pkg ], [ platform ] -> + (* Single package, single platform - use Real directly *) + Input.Real (pkg, platform) + | _ -> + (* Multiple packages or platforms - create virtual root *) + let depends = + List.concat_map platforms ~f:(fun platform -> List.map pkgs ~f:(fun name -> - { Input.drole = Real name; importance = Ensure; restrictions = [] }) - in - VirtualImpl (Input.Rank.bottom, depends) + { Input.drole = Real (name, platform) + ; importance = Ensure + ; restrictions = [] + })) in + let impl : Input.Impl.t = VirtualImpl (Input.Rank.bottom, depends) in Input.virtual_role [ impl ] in Solver.do_solve context ~closest_match:false req @@ -1406,6 +1630,68 @@ module Solver = struct | None -> Error req ;; + (* Filter a list, keeping the first occurrence of each [Some name] key. Items + whose [key] is [None] are kept unconditionally (used to pass Virtual roles + through, since they don't carry a package name to deduplicate on). *) + let filter_dedup_by_name ~key items = + let seen = ref OpamPackage.Name.Set.empty in + List.filter items ~f:(fun item -> + match key item with + | None -> true + | Some name -> + if OpamPackage.Name.Set.mem name !seen + then false + else ( + seen := OpamPackage.Name.Set.add name !seen; + true)) + ;; + + let role_name = function + | Input.Virtual _ -> None + | Input.Real (name, _) -> Some name + ;; + + let deduplicate_roles_by_name = filter_dedup_by_name ~key:role_name + + let deduplicate_components_by_name = + filter_dedup_by_name ~key:(fun (c : Diagnostics.Component.t) -> role_name c.role) + ;; + + (* Deduplicate impls by package name, keeping one representative per package. + For VirtualImpls, skip them if all their deps point to packages we've already seen. *) + let deduplicate_impls_by_name impls = + let seen = ref OpamPackage.Name.Set.empty in + List.filter impls ~f:(fun impl -> + match impl with + | Input.VirtualImpl (_, deps) -> + (* Check if this VirtualImpl has any deps we haven't seen yet *) + let has_new_deps = + List.exists deps ~f:(fun (d : Input.dependency) -> + match d.drole with + | Input.Virtual _ -> true + | Input.Real (name, _) -> not (OpamPackage.Name.Set.mem name !seen)) + in + if has_new_deps + then ( + (* Add all dep names to seen *) + List.iter deps ~f:(fun (d : Input.dependency) -> + match d.drole with + | Input.Virtual _ -> () + | Input.Real (name, _) -> seen := OpamPackage.Name.Set.add name !seen); + true) + else false + | _ -> + (match Input.Impl.version impl with + | None -> true + | Some pkg -> + let name = OpamPackage.name pkg in + if OpamPackage.Name.Set.mem name !seen + then false + else ( + seen := OpamPackage.Name.Set.add name !seen; + true))) + ;; + let pp_rolemap ~verbose reasons = let good, bad, unknown = Input.Role.Map.to_list reasons @@ -1417,6 +1703,25 @@ module Solver = struct | _, `No_candidates -> `Right role | _, _ -> `Middle component)) in + (* Deduplicate to avoid showing the same package multiple times for different platforms. + Also exclude packages from 'bad' if they appear in 'good' (a package that's + selected on one platform shouldn't be shown as a problem due to another platform). *) + let good = deduplicate_impls_by_name good in + let good_names = + List.filter_map good ~f:(fun impl -> + match Input.Impl.version impl with + | Some pkg -> Some (OpamPackage.name pkg) + | None -> None) + |> OpamPackage.Name.Set.of_list + in + let bad = + List.filter bad ~f:(fun (component : Diagnostics.Component.t) -> + match component.role with + | Input.Virtual _ -> true + | Input.Real (name, _) -> not (OpamPackage.Name.Set.mem name good_names)) + in + let bad = deduplicate_components_by_name bad in + let unknown = deduplicate_roles_by_name unknown in let pp_bad = Diagnostics.Component.pp ~verbose in let pp_unknown role = Pp.box (Input.Role.pp role) in match unknown with @@ -1450,9 +1755,33 @@ module Solver = struct Input.Role.Map.values sels |> List.filter_map ~f:(fun (sel : Solver.selection) -> Input.Impl.version sel.impl) ;; + + (* Extract packages grouped by platform from the solver result. + Returns a map: package_name -> (platform -> version) + This lets us detect if a package has different versions on different + platforms. *) + let packages_by_platform sels = + Input.Role.Map.foldi + sels + ~init:OpamPackage.Name.Map.empty + ~f:(fun role (sel : Solver.selection) acc -> + match role with + | Input.Virtual _ -> acc + | Input.Real (name, platform) -> + (match Input.Impl.version sel.impl with + | None -> acc + | Some pkg -> + let version = OpamPackage.version pkg in + let platforms = + match OpamPackage.Name.Map.find_opt name acc with + | None -> Solver_env.Map.singleton platform version + | Some platforms -> Solver_env.Map.set platforms platform version + in + OpamPackage.Name.Map.add name platforms acc)) + ;; end -let solve_package_list packages ~context = +let solve_package_list packages ~context ~platforms = Fiber.collect_errors (fun () -> (* [Solver.solve] returns [Error] when it's unable to find a solution to the dependencies, but can also raise exceptions, for example if opam @@ -1460,7 +1789,7 @@ let solve_package_list packages ~context = an unexpected opam exception from crashing dune, we catch all exceptions raised by the solver and report them as [User_error]s instead. *) - Solver.solve context packages) + Solver.solve context packages ~platforms) >>| (function | Ok (Ok res) -> Ok res | Ok (Error e) -> Error (`Diagnostics e) @@ -1469,7 +1798,10 @@ let solve_package_list packages ~context = (* CR-rgrinberg: this needs to be handled right *) Error (`Exn exn)) >>= function - | Ok packages -> Fiber.return @@ Ok (Solver.packages_of_result packages) + | Ok sels -> + let packages = Solver.packages_of_result sels in + let packages_by_platform = Solver.packages_by_platform sels in + Fiber.return @@ Ok (packages, packages_by_platform) | Error (`Diagnostics e) -> let+ diagnostics = Solver.diagnostics context e in Error (`Solve_error diagnostics) @@ -1495,28 +1827,6 @@ module Solver_result = struct ; pinned_packages : Package_name.Set.t ; num_expanded_packages : int } - - let merge a b = - let lock_dir = Lock_dir.merge_conditionals a.lock_dir b.lock_dir in - let files = - Package_name.Map.union a.files b.files ~f:(fun _ a b -> - Some - (Package_version.Map.union a b ~f:(fun _ a b -> - (* The package is present in both solutions at the same version. Make - sure its associated files are the same in both instances. *) - if not (List.equal File_entry.equal a b) - then - Code_error.raise - "Package files differ between merged solver results" - [ "files_1", Dyn.list File_entry.to_dyn a - ; "files_2", Dyn.list File_entry.to_dyn b - ]; - Some a))) - in - let pinned_packages = Package_name.Set.union a.pinned_packages b.pinned_packages in - let num_expanded_packages = a.num_expanded_packages + b.num_expanded_packages in - { lock_dir; files; pinned_packages; num_expanded_packages } - ;; end let reject_unreachable_packages = @@ -1718,6 +2028,7 @@ let resolve_opam_packages opam_packages_to_lock candidates_cache = let solve_lock_dir solver_env + ~platform_overlays version_preference repos ~local_packages @@ -1726,211 +2037,300 @@ let solve_lock_dir ~selected_depopts ~portable_lock_dir = - match Package_name.Map.add pinned_packages Dune_dep.name Resolved_package.dune with - | Error p -> - let loc = Resolved_package.loc p in - let message = - User_error.make - ~loc - [ Pp.text - "Dune cannot be pinned. The currently running version is the only one that \ - may be used" - ] - in - Fiber.return (Error (`Manifest_error message)) - | Ok pinned_packages -> - let pinned_package_names = Package_name.Set.of_keys pinned_packages in - let stats_updater = Solver_stats.Updater.init () in - let context = - let rec context = - lazy - (Context.create - ~pinned_packages - ~solver_env - ~repos - ~version_preference - ~local_packages:local_packages' - ~stats_updater - ~constraints) - and local_packages' = - lazy - (Package_name.Map.map local_packages ~f:(fun local -> - let opam_file = Local_package.For_solver.to_opam_file local in - let version = - Option.value - opam_file.version - ~default:Context.local_package_default_version - in - let deps = - lazy - (let opam_package = - OpamPackage.create (OpamFile.OPAM.name opam_file) version - in - Context.filter_deps (Lazy.force context) opam_package) - in - let depends = lazy (Lazy.force deps (OpamFile.OPAM.depends opam_file)) in - let conflicts = lazy (Lazy.force deps (OpamFile.OPAM.conflicts opam_file)) in - { Context.opam_file; version; depends; conflicts; name = local.name })) - in - Lazy.force context - in - Package_name.Map.keys local_packages @ selected_depopts - |> List.map ~f:Package_name.to_opam_package_name - |> solve_package_list ~context - >>= (function - | Error _ as e -> Fiber.return e - | Ok solution -> - let is_dune name = Package_name.equal Dune_dep.name name in - (* don't include local packages or dune in the lock dir *) - let opam_packages_to_lock = - let is_local_package = Package_name.Map.mem local_packages in - List.filter solution ~f:(fun package -> - let name = OpamPackage.name package |> Package_name.of_opam_package_name in - (not (is_local_package name)) && not (is_dune name)) + match platform_overlays with + | [] -> Code_error.raise "solve_lock_dir called with empty platform_overlays" [] + | _ -> + (match Package_name.Map.add pinned_packages Dune_dep.name Resolved_package.dune with + | Error p -> + let loc = Resolved_package.loc p in + let message = + User_error.make + ~loc + [ Pp.text + "Dune cannot be pinned. The currently running version is the only one \ + that may be used" + ] in - let* candidates_cache = Fiber.Cache.to_table context.candidates_cache in - let resolve_package name version = - (Table.find_exn candidates_cache name).resolved - |> OpamPackage.Version.Map.find version + Fiber.return (Error (`Manifest_error message)) + | Ok pinned_packages -> + let pinned_package_names = Package_name.Set.of_keys pinned_packages in + let stats_updater = Solver_stats.Updater.init () in + (* The platform envs themselves identify the platforms: every role and + every per-platform selection is keyed by the platform's own + (platform-specific) env. *) + let platforms = platform_overlays in + let full_solver_envs = + List.map platform_overlays ~f:(fun overlay -> + Solver_env.extend solver_env overlay) in - let* pkgs_by_name = - let+ pkgs = - let version_by_package_name = - Package_name.Map.of_list_map_exn - solution - ~f:(fun (package : OpamPackage.t) -> - ( Package_name.of_opam_package_name (OpamPackage.name package) - , Package_version.of_opam_package_version (OpamPackage.version package) )) - in - let+ resolved_pkgs = - resolve_opam_packages opam_packages_to_lock candidates_cache - in - List.map resolved_pkgs ~f:(fun (name, opam_package, resolved_package) -> - Lock_pkg.opam_package_to_lock_file_pkg - solver_env - stats_updater - version_by_package_name - opam_package - ~pinned:(Package_name.Set.mem pinned_package_names name) - resolved_package - ~portable_lock_dir) - |> Result.List.all + let context = + let rec context = + lazy + (Context.create + ~pinned_packages + ~solver_env + ~repos + ~version_preference + ~local_packages:local_packages' + ~stats_updater + ~constraints) + and local_packages' = + lazy + (Package_name.Map.map local_packages ~f:(fun local -> + let opam_file = Local_package.For_solver.to_opam_file local in + let version = + Option.value + opam_file.version + ~default:Context.local_package_default_version + in + let deps = + lazy + (let opam_package = + OpamPackage.create (OpamFile.OPAM.name opam_file) version + in + Context.filter_deps_local (Lazy.force context) opam_package) + in + let depends = lazy (Lazy.force deps (OpamFile.OPAM.depends opam_file)) in + let conflicts = + lazy (Lazy.force deps (OpamFile.OPAM.conflicts opam_file)) + in + { Context.opam_file; version; depends; conflicts; name = local.name })) in - Result.map pkgs ~f:(fun pkgs -> - match Package_name.Map.of_list_map pkgs ~f:(fun pkg -> pkg.info.name, pkg) with - | Error (name, _pkg1, _pkg2) -> - Code_error.raise - "Solver selected multiple versions for the same package" - [ "name", Package_name.to_dyn name ] - | Ok pkgs_by_name -> - let reachable = - reject_unreachable_packages - solver_env - ~dune_version: - (Package_version.of_opam_package_version context.dune_version) - ~local_packages - ~pkgs_by_name - in - Package_name.Map.filteri pkgs_by_name ~f:(fun name _ -> - Package_name.Set.mem reachable name)) + Lazy.force context in - let ocaml = - let open Result.O in - let* pkgs_by_name = pkgs_by_name in - (* This doesn't allow the compiler to live in the source tree. Oh + Package_name.Map.keys local_packages @ selected_depopts + |> List.map ~f:Package_name.to_opam_package_name + |> solve_package_list ~context ~platforms + >>= (function + | Error _ as e -> Fiber.return e + | Ok (solution, packages_by_platform) -> + (* The full solver_envs for the platforms a package is selected on. + Every package returned by the solver is selected on at least one + platform, so [packages_by_platform] is guaranteed to contain it; + the per-platform selection map is guaranteed non-empty and its + keys are always drawn from [platform_overlays]. *) + let solver_envs_for_package opam_name = + match OpamPackage.Name.Map.find_opt opam_name packages_by_platform with + | None -> + Code_error.raise + "solver result is missing a package it selected" + [ "name", Dyn.string (OpamPackage.Name.to_string opam_name) ] + | Some platform_map -> + (* Iterate over the requested platforms in their original order + so that lock file entries keep the user's platform order. *) + List.filter_map platform_overlays ~f:(fun platform -> + Option.some_if (Solver_env.Map.mem platform_map platform) platform) + |> List.map ~f:(fun platform -> Solver_env.extend solver_env platform) + in + let is_dune name = Package_name.equal Dune_dep.name name in + (* Don't include local packages or dune in the lock dir. The + single joint solve may return the same package once per platform, + so deduplicate here. *) + let opam_packages_to_lock = + let is_local_package = Package_name.Map.mem local_packages in + List.filter solution ~f:(fun package -> + let name = OpamPackage.name package |> Package_name.of_opam_package_name in + (not (is_local_package name)) && not (is_dune name)) + |> List.sort_uniq ~compare:(fun a b -> + Ordering.of_int (OpamPackage.compare a b)) + in + let* candidates_cache = Fiber.Cache.to_table context.candidates_cache in + let resolve_package name version = + (Table.find_exn candidates_cache name).resolved + |> OpamPackage.Version.Map.find version + in + let* pkgs_by_name = + let+ pkgs = + (* Deduplicate packages - same package may appear for multiple + platforms. The SAT-level cross-platform version equality + constraint guarantees the version is identical across + platforms, so the fold below either inserts a fresh entry or + sees a matching version. A mismatch indicates a bug in the + SAT setup, not user error. *) + let version_by_package_name = + List.fold_left + solution + ~init:Package_name.Map.empty + ~f:(fun acc package -> + let name = + Package_name.of_opam_package_name (OpamPackage.name package) + in + let version = + Package_version.of_opam_package_version + (OpamPackage.version package) + in + match Package_name.Map.find acc name with + | None -> Package_name.Map.set acc name version + | Some existing_version -> + if not (Package_version.equal existing_version version) + then + Code_error.raise + "Cross-platform version equality SAT constraint failed: solver \ + selected multiple versions of the same package" + [ "name", Package_name.to_dyn name + ; "version_a", Package_version.to_dyn existing_version + ; "version_b", Package_version.to_dyn version + ]; + acc) + in + let+ resolved_pkgs = + resolve_opam_packages opam_packages_to_lock candidates_cache + in + (* Generate lock file entries for each package. + Lock_pkg handles per-platform evaluation and merging when needed. *) + List.map resolved_pkgs ~f:(fun (name, opam_package, resolved_package) -> + let opam_name = Package_name.to_opam_package_name name in + let package_solver_envs = solver_envs_for_package opam_name in + Lock_pkg.opam_package_to_lock_file_pkg + package_solver_envs + stats_updater + version_by_package_name + opam_package + ~pinned:(Package_name.Set.mem pinned_package_names name) + resolved_package + ~portable_lock_dir) + |> Result.List.all + in + Result.map pkgs ~f:(fun pkgs -> + match + Package_name.Map.of_list_map pkgs ~f:(fun pkg -> pkg.info.name, pkg) + with + | Error (name, _pkg1, _pkg2) -> + Code_error.raise + "Solver selected multiple versions for the same package" + [ "name", Package_name.to_dyn name ] + | Ok pkgs_by_name -> + (* Compute reachability for ALL platform envs and union the results. + A package is reachable if it's reachable on any platform. *) + let reachable = + List.fold_left + full_solver_envs + ~init:Package_name.Set.empty + ~f:(fun acc solver_env -> + let reachable_on_platform = + reject_unreachable_packages + solver_env + ~dune_version: + (Package_version.of_opam_package_version context.dune_version) + ~local_packages + ~pkgs_by_name + in + Package_name.Set.union acc reachable_on_platform) + in + Package_name.Map.filteri pkgs_by_name ~f:(fun name _ -> + Package_name.Set.mem reachable name)) + in + let ocaml = + let open Result.O in + let* pkgs_by_name = pkgs_by_name in + (* This doesn't allow the compiler to live in the source tree. Oh well, it's not possible now anyway. *) - match - Package_name.Map.filter_map pkgs_by_name ~f:(fun (pkg : Lock_dir.Pkg.t) -> - match - let version = Package_version.to_opam_package_version pkg.info.version in - resolve_package pkg.info.name version |> package_kind - with - | `Compiler -> Some pkg.info.name - | `Non_compiler -> None) - |> Package_name.Map.values - with - | [] -> Ok None - | [ x ] -> Ok (Some (Loc.none, x)) - | _ -> - Error - (User_error.make - (* CR-someday rgrinberg: needs to include locations *) - [ Pp.text "multiple compilers selected" ] - ~hints:[ Pp.text "add a conflict" ]) - in - let lock_dir = - let open Result.O in - let* pkgs_by_name = pkgs_by_name - and* ocaml = ocaml in - let+ () = - Package_name.Map.values pkgs_by_name - |> Result.List.map ~f:(fun { Lock_dir.Pkg.depends; info = { name; _ }; _ } -> - match - Lock_dir.Conditional_choice.choose_for_platform - depends - ~platform:solver_env - with - | None -> Ok () - | Some depends -> - Result.List.map - depends - ~f:(fun { Lock_dir.Dependency.name = dep_name; loc } -> - match - (not (is_dune dep_name)) - && Package_name.Map.mem local_packages dep_name - with - | false -> Ok () - | true -> - Error - (User_error.make - ~loc - [ Pp.textf - "Dune does not support packages outside the workspace \ - depending on packages in the workspace. The package %S is \ - not in the workspace but it depends on the package %S \ - which is in the workspace." - (Package_name.to_string name) - (Package_name.to_string dep_name) - ])) - |> Result.map ~f:(fun (_ : unit list) -> ())) - |> Result.map ~f:(fun (_ : unit list) -> ()) - in - let expanded_solver_variable_bindings = - let stats = Solver_stats.Updater.snapshot stats_updater in - Solver_stats.Expanded_variable_bindings.of_variable_set - stats.expanded_variables - solver_env - in - Lock_dir.create_latest_version - pkgs_by_name - ~local_packages:(Package_name.Map.values local_packages) - ~ocaml - ~repos:(Some repos) - ~expanded_solver_variable_bindings - ~solved_for_platform:(Some solver_env) - ~portable_lock_dir - in - let+ files = - match pkgs_by_name with - | Error e -> Fiber.return (Error e) - | Ok pkgs_by_name -> - let+ files = - Package_name.Map.to_list_map - pkgs_by_name - ~f:(fun name (package : Lock_dir.Pkg.t) -> - Package_version.to_opam_package_version package.info.version - |> resolve_package name) - |> files - in - files - in - (match Result.both lock_dir files with - | Error e -> Error (`Manifest_error e) - | Ok (lock_dir, files) -> - Ok - { Solver_result.lock_dir - ; files - ; pinned_packages = pinned_package_names - ; num_expanded_packages = Context.count_expanded_packages context - })) + match + Package_name.Map.filter_map pkgs_by_name ~f:(fun (pkg : Lock_dir.Pkg.t) -> + match + let version = + Package_version.to_opam_package_version pkg.info.version + in + resolve_package pkg.info.name version |> package_kind + with + | `Compiler -> Some pkg.info.name + | `Non_compiler -> None) + |> Package_name.Map.values + with + | [] -> Ok None + | [ x ] -> Ok (Some (Loc.none, x)) + | _ -> + Error + (User_error.make + (* CR-someday rgrinberg: needs to include locations *) + [ Pp.text "multiple compilers selected" ] + ~hints:[ Pp.text "add a conflict" ]) + in + let lock_dir = + let open Result.O in + let* pkgs_by_name = pkgs_by_name + and* ocaml = ocaml in + let+ () = + Package_name.Map.values pkgs_by_name + |> Result.List.map + ~f:(fun { Lock_dir.Pkg.depends; info = { name; _ }; _ } -> + (* A repository package must not evade validation by + depending on a workspace package only on a + non-primary platform, so validate the dependency + choices for every platform where the package is + selected. *) + let platform_envs = + solver_envs_for_package (Package_name.to_opam_package_name name) + in + Result.List.map platform_envs ~f:(fun platform_env -> + match + Lock_dir.Conditional_choice.choose_for_platform + depends + ~platform:platform_env + with + | None -> Ok () + | Some depends -> + Result.List.map + depends + ~f:(fun { Lock_dir.Dependency.name = dep_name; loc } -> + match + (not (is_dune dep_name)) + && Package_name.Map.mem local_packages dep_name + with + | false -> Ok () + | true -> + Error + (User_error.make + ~loc + [ Pp.textf + "Dune does not support packages outside the \ + workspace depending on packages in the \ + workspace. The package %S is not in the \ + workspace but it depends on the package %S \ + which is in the workspace." + (Package_name.to_string name) + (Package_name.to_string dep_name) + ])) + |> Result.map ~f:(fun (_ : unit list) -> ())) + |> Result.map ~f:(fun (_ : unit list) -> ())) + |> Result.map ~f:(fun (_ : unit list) -> ()) + in + let expanded_solver_variable_bindings = + let stats = Solver_stats.Updater.snapshot stats_updater in + Solver_stats.Expanded_variable_bindings.of_variable_set + stats.expanded_variables + solver_env + in + Lock_dir.create_latest_version + pkgs_by_name + ~local_packages:(Package_name.Map.values local_packages) + ~ocaml + ~repos:(Some repos) + ~expanded_solver_variable_bindings + ~solved_for_platforms:full_solver_envs + ~portable_lock_dir + in + let+ files = + match pkgs_by_name with + | Error e -> Fiber.return (Error e) + | Ok pkgs_by_name -> + let+ files = + Package_name.Map.to_list_map + pkgs_by_name + ~f:(fun name (package : Lock_dir.Pkg.t) -> + Package_version.to_opam_package_version package.info.version + |> resolve_package name) + |> files + in + files + in + (match Result.both lock_dir files with + | Error e -> Error (`Manifest_error e) + | Ok (lock_dir, files) -> + Ok + { Solver_result.lock_dir + ; files + ; pinned_packages = pinned_package_names + ; num_expanded_packages = Context.count_expanded_packages context + }))) ;; diff --git a/src/dune_pkg/opam_solver.mli b/src/dune_pkg/opam_solver.mli index b3550b5fcd1..71b0c0a4c16 100644 --- a/src/dune_pkg/opam_solver.mli +++ b/src/dune_pkg/opam_solver.mli @@ -7,12 +7,11 @@ module Solver_result : sig ; pinned_packages : Package_name.Set.t ; num_expanded_packages : int } - - val merge : t -> t -> t end val solve_lock_dir : Solver_env.t + -> platform_overlays:Solver_env.t list -> Version_preference.t -> Opam_repo.t list -> local_packages:Local_package.For_solver.t Package_name.Map.t diff --git a/src/dune_rules/lock_rules.ml b/src/dune_rules/lock_rules.ml index 6a680e156b3..5733a8c2a2c 100644 --- a/src/dune_rules/lock_rules.ml +++ b/src/dune_rules/lock_rules.ml @@ -152,55 +152,25 @@ module Spec = struct ~unset:(Some unset_solver_vars) in let* solver_result = - if portable_lock_dir - then ( - (* CR-someday Alizter: This multi-platform solving logic is duplicated - from bin/pkg/lock.ml:solve_multiple_platforms. The logic for - removing platform-specific variables, solving for multiple platforms - in parallel, merging results, and error handling should be shared - between autolocking and manual locking. Consider extracting this - into a shared function in Dune_pkg.Opam_solver. *) - let portable_solver_env = - Solver_env.unset_multi - solver_env - Dune_lang.Package_variable_name.platform_specific - in - let solve_for_platforms = Solver_env.popular_platform_envs in - let+ results = - Fiber.parallel_map solve_for_platforms ~f:(fun platform_env -> - let solver_env_for_platform = - Solver_env.extend portable_solver_env platform_env - in - Opam_solver.solve_lock_dir - solver_env_for_platform - version_preference - repos - ~pins - ~local_packages - ~constraints - ~selected_depopts - ~portable_lock_dir) - 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." [] - | [], `Manifest_error diagnostic :: _ -> Error (`Manifest_error diagnostic) - | [], `Solve_error diagnostic :: _ -> Error (`Solve_error diagnostic) - | x :: xs, _ -> Ok (List.fold_left xs ~init:x ~f:Opam_solver.Solver_result.merge)) - else - Opam_solver.solve_lock_dir - solver_env - version_preference - repos - ~pins - ~local_packages - ~constraints - ~selected_depopts - ~portable_lock_dir + let base_solver_env, platform_overlays = + if portable_lock_dir + then + ( Solver_env.unset_multi + solver_env + Dune_lang.Package_variable_name.platform_specific + , Solver_env.popular_platform_envs ) + else solver_env, [ Solver_env.empty ] + in + Opam_solver.solve_lock_dir + base_solver_env + ~platform_overlays + version_preference + repos + ~pins + ~local_packages + ~constraints + ~selected_depopts + ~portable_lock_dir in match solver_result with | Error (`Manifest_error diagnostic) -> raise (User_error.E diagnostic) diff --git a/test/blackbox-tests/test-cases/pkg/autolock-with-watch-server.t b/test/blackbox-tests/test-cases/pkg/autolock-with-watch-server.t index 07f74f1e5f2..d7d08b8534c 100644 --- a/test/blackbox-tests/test-cases/pkg/autolock-with-watch-server.t +++ b/test/blackbox-tests/test-cases/pkg/autolock-with-watch-server.t @@ -39,7 +39,7 @@ configuration. Fix for https://github.com/ocaml/dune/issues/15587 $ dune pkg enabled - $ build a.exe + $ $timeout 10 dune rpc build --wait a.exe Success $ wait_for_line_with_timeout .#dune-output "Success, waiting for filesystem changes..." 200 $ cat .#dune-output @@ -57,7 +57,7 @@ Add new dependency c: Run build: - $ build a.exe + $ $timeout 10 dune rpc build --wait a.exe Success Stop the watch server diff --git a/test/blackbox-tests/test-cases/pkg/conflict-class.t b/test/blackbox-tests/test-cases/pkg/conflict-class.t index cb8ba91870b..fc42b97b199 100644 --- a/test/blackbox-tests/test-cases/pkg/conflict-class.t +++ b/test/blackbox-tests/test-cases/pkg/conflict-class.t @@ -30,7 +30,7 @@ Local conflict class defined in a local package: Unable to solve dependencies while generating lock directory: dune.lock Couldn't solve the package dependency formula. - Selected candidates: foo.dev x.dev foo&x + Selected candidates: foo.dev x.dev - bar -> (problem) Rejected candidates: bar.0.0.1: In same conflict class (ccc) as foo diff --git a/test/blackbox-tests/test-cases/pkg/helpers.sh b/test/blackbox-tests/test-cases/pkg/helpers.sh index 89da6a85268..848f88077f1 100644 --- a/test/blackbox-tests/test-cases/pkg/helpers.sh +++ b/test/blackbox-tests/test-cases/pkg/helpers.sh @@ -534,10 +534,13 @@ dune_pkg_lock_normalized() { cat "${processed}" else processed="$(mktemp)" - dune_cmd delete-between \ - 'The dependency solver failed to find a solution for the following platforms:' \ - '\.\.\.with this error:' \ + dune_cmd delete-between \ + 'The dependency solver failed to find a solution for the requested platforms:' \ + '\.\.\.with this error:' \ < "${out}" \ + | dune_cmd delete-between \ + 'Hint: If you don.t need support for every requested platform' \ + 'Hint: platforms you need, then rerun' \ > "${processed}" cat "${processed}" return 1 diff --git a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-all-or-nothing.t b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-all-or-nothing.t new file mode 100644 index 00000000000..8cee020853b --- /dev/null +++ b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-all-or-nothing.t @@ -0,0 +1,62 @@ +Demonstrate that locking fails entirely when the requested platform set has no +joint solution: no partial lock directory is written. + + $ mkrepo + $ add_mock_repo_if_needed + +Make a package that is only available on macos. + $ mkpkg foo < available: os = "macos" + > build: [ + > ["mkdir" "-p" "%{lib}%/%{name}%"] + > ["touch" "%{lib}%/%{name}%/META"] # needed for dune to recognize this as a library + > ] + > EOF + + $ make_portable_lockdirs_project + +The default platform set includes linux, where "foo" cannot be installed. The +failure is reported once for the requested platform set: + + $ dune pkg lock + Error: + Unable to solve dependencies while generating lock directory: dune.lock + + The dependency solver failed to find a solution for the requested platforms: + - arch = x86_64; os = linux + - arch = arm64; os = linux + - arch = x86_64; os = macos + - arch = arm64; os = macos + ...with this error: + Couldn't solve the package dependency formula. + Selected candidates: foo.0.0.1 x.dev + + Hint: If you don't need support for every requested platform, change + Hint: (solve_for_platforms ...) in dune-workspace to only include the + Hint: platforms you need, then rerun 'dune pkg lock' + [1] + +No partial lock directory is written: + + $ test ! -e dune.lock + +When the platform set only contains platforms where the package is available, +locking succeeds: + + $ cat > dune-workspace < (lang dune 3.11) + > (repository + > (name mock) + > (url "file://$(pwd)/mock-opam-repository")) + > (lock_dir + > (repositories mock) + > (solve_for_platforms + > ((arch arm64) + > (os macos)))) + > EOF + + $ dune pkg lock + Solution for dune.lock + + Dependencies common to all supported platforms: + - foo.0.0.1 diff --git a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-basic.t b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-basic.t index a5ab1c9d97e..f7a775ff33b 100644 --- a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-basic.t +++ b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-basic.t @@ -23,12 +23,11 @@ Create a package that writes a different value to some files depending on the os Dependencies common to all supported platforms: - foo.0.0.1 -The portable lock directory is solved independently for each of the four -platforms. +The SAT engine runs once across all requested platforms. $ dune trace cat \ > | jq -s 'include "dune"; [ .[] | satSolveEvents ] | length' - 4 + 1 $ cat ${default_lock_dir}/lock.dune (lang package 0.1) diff --git a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-no-solution.t b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-no-solution.t index 7438e22ac5a..a6716b9143c 100644 --- a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-no-solution.t +++ b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-no-solution.t @@ -31,7 +31,7 @@ Solver error when solving fails with the same error on all platforms: Error: Unable to solve dependencies while generating lock directory: dune.lock - The dependency solver failed to find a solution for the following platforms: + The dependency solver failed to find a solution for the requested platforms: - arch = x86_64; os = linux - arch = arm64; os = linux - arch = x86_64; os = macos @@ -43,13 +43,16 @@ Solver error when solving fails with the same error on all platforms: a 0.0.1 requires = 0.1 Rejected candidates: c.0.2: Incompatible with restriction: = 0.1 + Hint: If you don't need support for every requested platform, change + Hint: (solve_for_platforms ...) in dune-workspace to only include the + Hint: platforms you need, then rerun 'dune pkg lock' [1] -Each of the four platform solves retries twice before reporting the failure. +The single platform-set solve retries twice before reporting the failure. $ dune trace cat \ > | jq -s 'include "dune"; [ .[] | satSolveEvents ] | length' - 12 + 3 No partial lock directory is written: $ test ! -e dune.lock @@ -68,25 +71,19 @@ with the platforms where they are relevant: Error: Unable to solve dependencies while generating lock directory: dune.lock - The dependency solver failed to find a solution for the following platforms: + The dependency solver failed to find a solution for the requested platforms: - arch = x86_64; os = linux - arch = arm64; os = linux - ...with this error: - Couldn't solve the package dependency formula. - Selected candidates: a.0.0.1 b.0.0.1 foo.dev - - c -> (problem) - a 0.0.1 requires = 0.1 - Rejected candidates: - c.0.2: Incompatible with restriction: = 0.1 - - The dependency solver failed to find a solution for the following platforms: - arch = x86_64; os = macos - arch = arm64; os = macos ...with this error: Couldn't solve the package dependency formula. Selected candidates: a.0.0.1 b.0.0.1 foo.dev - c -> (problem) - a 0.0.1 requires = 0.3 + a 0.0.1 requires = 0.1 Rejected candidates: - c.0.2: Incompatible with restriction: = 0.3 + c.0.2: Incompatible with restriction: = 0.1 + Hint: If you don't need support for every requested platform, change + Hint: (solve_for_platforms ...) in dune-workspace to only include the + Hint: platforms you need, then rerun 'dune pkg lock' [1] diff --git a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-older-common-version.t b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-older-common-version.t index c0642e14fb1..4b474ae53ed 100644 --- a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-older-common-version.t +++ b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-older-common-version.t @@ -1,6 +1,6 @@ When one platform can only use an older version of a package while another -platform prefers a newer version, the current per-platform solver selects a -different version for each platform. +platform prefers a newer, platform-specific version, the common older version +is selected for all platforms. $ mkrepo $ add_mock_repo_if_needed @@ -35,33 +35,20 @@ Define a package bar which depends on foo without a version constraint: $ make_x_depends_bar_project -Linux prefers foo.2 while macos can only install foo.1. The current -per-platform solver selects each platform's preferred version: +Linux would prefer foo.2 but macos cannot install it. The single solve must +select foo.1 for every platform: $ dune pkg lock Solution for dune.lock Dependencies common to all supported platforms: - bar.0.0.1 - - Additionally, some packages will only be built on specific platforms. - - arch = arm64; os = linux: - - foo.2 - - arch = arm64; os = macos: - - foo.1 - - arch = x86_64; os = linux: - - foo.2 - - arch = x86_64; os = macos: - foo.1 -Build the project as if we were on linux and confirm that version 2 of foo was built: +Build the project as if we were on linux and confirm that version 1 of foo was built: $ export DUNE_CONFIG__OS=linux DUNE_CONFIG__ARCH=arm64 DUNE_CONFIG__OS_FAMILY=debian DUNE_CONFIG__OS_DISTRIBUTION=ubuntu DUNE_CONFIG__OS_VERSION=24.11 $ dune build $ cat $pkg_root/$(dune pkg print-digest foo)/target/share/version - 2 + 1 $ dune clean diff --git a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-partial-solve.t b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-partial-solve.t deleted file mode 100644 index d74006c775d..00000000000 --- a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-partial-solve.t +++ /dev/null @@ -1,95 +0,0 @@ -Demonstrate the case where a project can only be solved for a subset of platforms. - - $ mkrepo - $ add_mock_repo_if_needed - -Make a package that is only available on macos. - $ mkpkg foo < available: os = "macos" - > build: [ - > ["mkdir" "-p" "%{lib}%/%{name}%"] - > ["touch" "%{lib}%/%{name}%/META"] # needed for dune to recognize this as a library - > ] - > EOF - - $ make_portable_lockdirs_project - -Solving will still succeed, but there'll be a warning because dune will attempt -to solve for macos, linux, and windows by default. - $ dune pkg lock --trace-file trace.csexp - Solution for dune.lock - - Dependencies common to all supported platforms: - - foo.0.0.1 - - No package solution was found for some requsted platforms. - - Platforms with no solution: - - arch = arm64; os = linux - - arch = x86_64; os = linux - - See the trace file with --trace-file for more details. Configure platforms to - solve for in the dune-workspace file. - -The log file will contain errors about the package being unavailable. - $ jqScript=$(mktemp) - $ cat >$jqScript < select(.cat == "log" and .args.message != "ocamlparam" and (.args.message | contains("Shared cache") | not)) | - > .args - > EOF - $ dune trace cat --trace-file trace.csexp | jq -f $jqScript - { - "message": "Workspace root", - "root": "$TESTCASE_ROOT" - } - { - "message": "Solver found partial solution", - "error_count": 1 - } - { - "message": "Dependency solution", - "lock_dir": "dune.lock", - "packages": [ - "foo.0.0.1" - ] - } - -The lockdir will contain a list of the platforms where solving succeeded. - $ cat ${default_lock_dir}/lock.dune - (lang package 0.1) - - (dependency_hash 36e640fbcda71963e7e2f689f6c96c3e) - - (repositories - (complete false) - (used)) - - (solved_for_platforms - ((arch x86_64) - (os macos)) - ((arch arm64) - (os macos))) - -No errors when you try to build the platform on macos. - $ DUNE_CONFIG__OS=macos DUNE_CONFIG__ARCH=x86_64 DUNE_CONFIG__OS_FAMILY=homebrew DUNE_CONFIG__OS_DISTRIBUTION=homebrew DUNE_CONFIG__OS_VERSION=15.3.1 dune build - -Building on linux fails because the lockdir doesn't contain a compatible solution. - $ DUNE_CONFIG__OS=linux DUNE_CONFIG__ARCH=arm64 DUNE_CONFIG__OS_FAMILY=debian DUNE_CONFIG__OS_DISTRIBUTION=ubuntu DUNE_CONFIG__OS_VERSION=24.11 dune build - File "dune.lock/lock.dune", lines 10-13, characters 1-58: - 10 | ((arch x86_64) - 11 | (os macos)) - 12 | ((arch arm64) - 13 | (os macos))) - Error: The lockdir does not contain a solution compatible with the current - platform. - The current platform is: - - arch = arm64 - - os = linux - - os-distribution = ubuntu - - os-family = debian - - os-version = 24.11 - - sys-ocaml-version = 5.4.0+fake - Hint: Try adding the following to dune-workspace: - Hint: (lock_dir (solve_for_platforms ((arch arm64) (os linux)))) - Hint: ...and then rerun 'dune pkg lock' - [1] diff --git a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version-extra-files.t b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version-extra-files.t index 959e088ffe4..84882e784a8 100644 --- a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version-extra-files.t +++ b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version-extra-files.t @@ -42,41 +42,27 @@ Define a package bar which conditionally depends on different versions of foo: Define a project with a package depending on bar: $ make_x_depends_bar_project -Solve the project. The solution will contain extra files for both versions of foo: +The cross-platform version constraint rejects the disagreement on foo. No lock +directory, including package extra files, may be written: + $ dune pkg lock - Solution for dune.lock - - Dependencies common to all supported platforms: - - bar.0.0.1 - - Additionally, some packages will only be built on specific platforms. - - arch = arm64; os = linux: - - foo.1 - - arch = arm64; os = macos: - - foo.2 + Error: + Unable to solve dependencies while generating lock directory: dune.lock - arch = x86_64; os = linux: - - foo.1 - - arch = x86_64; os = macos: - - foo.2 - -Verify the contents of the extra files for each version of foo: - $ cat ${default_lock_dir}/foo.1.files/version.txt - version_1 - $ cat ${default_lock_dir}/foo.2.files/version.txt - version_2 - -Build as if we're on linux and verify that the appropriate extra file was copied into _build: - $ DUNE_CONFIG__OS=linux DUNE_CONFIG__ARCH=arm64 DUNE_CONFIG__OS_FAMILY=debian DUNE_CONFIG__OS_DISTRIBUTION=ubuntu DUNE_CONFIG__OS_VERSION=24.11 dune build - $ cat ${default_lock_dir}/foo.1.files/version.txt - version_1 - - $ dune clean + The dependency solver failed to find a solution for the requested platforms: + - arch = x86_64; os = linux + - arch = arm64; os = linux + - arch = x86_64; os = macos + - arch = arm64; os = macos + ...with this error: + Couldn't solve the package dependency formula. + Selected candidates: bar.0.0.1 x.dev + - foo -> foo.1 + bar 0.0.1 requires = 1 + Hint: If you don't need support for every requested platform, change + Hint: (solve_for_platforms ...) in dune-workspace to only include the + Hint: platforms you need, then rerun 'dune pkg lock' + [1] -Build as if we're on macos and verify that the appropriate extra file was copied into _build: - $ DUNE_CONFIG__OS=macos DUNE_CONFIG__ARCH=x86_64 DUNE_CONFIG__OS_FAMILY=homebrew DUNE_CONFIG__OS_DISTRIBUTION=homebrew DUNE_CONFIG__OS_VERSION=15.3.1 dune build - $ cat ${default_lock_dir}/foo.2.files/version.txt - version_2 +No partial lock directory is written: + $ test ! -e dune.lock diff --git a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version.t b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version.t index eab7cf2e6f4..b2ac1811a4b 100644 --- a/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version.t +++ b/test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-version.t @@ -27,44 +27,35 @@ Define a package bar which conditionally depends on different versions of foo: $ make_x_depends_bar_project +Linux requires foo.1 while macos requires foo.2. The cross-platform version +constraint makes the requested platform set unsatisfiable: + $ DUNE_TRACE=+sat dune pkg lock - Solution for dune.lock - - Dependencies common to all supported platforms: - - bar.0.0.1 - - Additionally, some packages will only be built on specific platforms. - - arch = arm64; os = linux: - - foo.1 - - arch = arm64; os = macos: - - foo.2 + Error: + Unable to solve dependencies while generating lock directory: dune.lock - arch = x86_64; os = linux: - - foo.1 - - arch = x86_64; os = macos: - - foo.2 - -The portable lock directory is solved independently for each of the four -platforms. + The dependency solver failed to find a solution for the requested platforms: + - arch = x86_64; os = linux + - arch = arm64; os = linux + - arch = x86_64; os = macos + - arch = arm64; os = macos + ...with this error: + Couldn't solve the package dependency formula. + Selected candidates: bar.0.0.1 x.dev + - foo -> foo.1 + bar 0.0.1 requires = 1 + Hint: If you don't need support for every requested platform, change + Hint: (solve_for_platforms ...) in dune-workspace to only include the + Hint: platforms you need, then rerun 'dune pkg lock' + [1] + +The SAT engine itself rejects the conflict; the post-solve version-conflict +check is no longer reached. The do_solve retry path runs SAT 3 times before +reporting the failure, and each run records the same cross-platform conflict. $ dune trace cat \ > | jq -s 'include "dune"; [ .[] | satSolveEvents ] | length' - 4 - -Build the project as if we were on linux and confirm that version 1 of foo was built: - $ export DUNE_CONFIG__OS=linux DUNE_CONFIG__ARCH=arm64 DUNE_CONFIG__OS_FAMILY=debian DUNE_CONFIG__OS_DISTRIBUTION=ubuntu DUNE_CONFIG__OS_VERSION=24.11 - $ dune build - $ cat $pkg_root/$(dune pkg print-digest foo)/target/share/version - 1 - - $ dune clean - -Build the project as if we were on macos and confirm that version 2 of foo was built: - $ export DUNE_CONFIG__OS=macos DUNE_CONFIG__ARCH=x86_64 DUNE_CONFIG__OS_FAMILY=homebrew DUNE_CONFIG__OS_DISTRIBUTION=homebrew DUNE_CONFIG__OS_VERSION=15.3.1 - $ dune build - $ cat $pkg_root/$(dune pkg print-digest foo)/target/share/version - 2 + 3 +No partial lock directory is written: + $ test ! -e dune.lock diff --git a/test/blackbox-tests/test-cases/pkg/workspace-deps/solver-missing-package.t b/test/blackbox-tests/test-cases/pkg/workspace-deps/solver-missing-package.t index 4ada97f2098..dd96ce6097c 100644 --- a/test/blackbox-tests/test-cases/pkg/workspace-deps/solver-missing-package.t +++ b/test/blackbox-tests/test-cases/pkg/workspace-deps/solver-missing-package.t @@ -17,7 +17,7 @@ the repository nor in the workspace. The solver should reject this. Error: Unable to solve dependencies while generating lock directory: dune.lock - The dependency solver failed to find a solution for the following platforms: + The dependency solver failed to find a solution for the requested platforms: - arch = x86_64; os = linux - arch = arm64; os = linux - arch = x86_64; os = macos @@ -25,5 +25,8 @@ the repository nor in the workspace. The solver should reject this. ...with this error: Couldn't solve the package dependency formula. The following packages couldn't be found: nonexistent-pkg + Hint: If you don't need support for every requested platform, change + Hint: (solve_for_platforms ...) in dune-workspace to only include the + Hint: platforms you need, then rerun 'dune pkg lock' [1] diff --git a/test/expect-tests/dune_pkg/encode_decode_tests.ml b/test/expect-tests/dune_pkg/encode_decode_tests.ml index 977ff6aba67..57f09c00570 100644 --- a/test/expect-tests/dune_pkg/encode_decode_tests.ml +++ b/test/expect-tests/dune_pkg/encode_decode_tests.ml @@ -110,7 +110,7 @@ let%expect_test "encode/decode round trip test for lockdir with no deps" = ~ocaml:None ~repos:None ~expanded_solver_variable_bindings:Expanded_variable_bindings.empty - ~solved_for_platform:None + ~solved_for_platforms:[] ~portable_lock_dir:false) (); [%expect @@ -163,7 +163,7 @@ let%expect_test "encode/decode round trip test for lockdir with simple deps" = [ Package_variable_name.os, Variable_value.string "linux" ] ; unset_variables = [ Package_variable_name.os_family ] } - ~solved_for_platform:None + ~solved_for_platforms:[] ~portable_lock_dir:false (Package_name.Map.of_list_exn [ mk_pkg_basic ~name:"foo" ~version:(Package_version.of_string "0.1.0") @@ -325,7 +325,7 @@ let%expect_test "encode/decode round trip test for lockdir with complex deps" = ~ocaml:(Some (Loc.none, Package_name.of_string "ocaml")) ~repos:(Some [ opam_repo ]) ~expanded_solver_variable_bindings:Expanded_variable_bindings.empty - ~solved_for_platform:None + ~solved_for_platforms:[] ~portable_lock_dir:false (Package_name.Map.of_list_exn [ pkg_a; pkg_b; pkg_c ]) in @@ -473,7 +473,7 @@ let%expect_test "encode/decode round trip test with locked repo revision" = ~ocaml:(Some (Loc.none, Package_name.of_string "ocaml")) ~repos:(Some [ opam_repo ]) ~expanded_solver_variable_bindings:Expanded_variable_bindings.empty - ~solved_for_platform:None + ~solved_for_platforms:[] ~portable_lock_dir:false (Package_name.Map.of_list_exn [ pkg_a; pkg_b; pkg_c ]) in