Merge pull request #210 from Leonidas-from-XIV/only-warn-dune-univers…

…e-if-no-solution

Only warn about dune ports repo when a solution can't be found

CHANGES.md

@@ -4,6 +4,9 @@
 
 ### Changed
 
+- Only warn users about missing dune-ports repo in OPAM switch if no solution
+  can be found due to packages not building with dune (#210, @Leonidas-from-XIV)
+
 ### Deprecated
 
 ### Fixed

cli/lock.ml

@@ -87,20 +87,24 @@ let is_duniverse_repo (repo : OpamTypes.repository) =
   let url = OpamUrl.to_string repo.repo_url in
   String.equal url Config.duniverse_opam_repo
 
-let check_repo_config ~switch_state =
+let check_dune_universe_repo ~switch_state non_dune_packages =
   let repos = current_repos ~switch_state in
   let dune_universe_is_configured = List.exists ~f:is_duniverse_repo repos in
   if not dune_universe_is_configured then
     Logs.warn (fun l ->
         l
-          "The dune-universe opam-repository isn't set in the current switch. \
-           It contains dune ports for some opam packages. Note that %a will \
-           fail if not all of the project dependencies use dune as their build \
-           system. Adding this opam-repository to your current switch will \
-           help with that. If you wish to do so, run the following command:\n\
+          "Couldn't calculate a set of packages to satisfy the request. Note \
+           that %a will fail if not all of the project dependencies use dune \
+           as their build system, in your project that would be %a. To solve \
+           this issue there exists a dune-universe opam-repository which \
+           contains dune ports for some opam packages, but it is currently not \
+           set in your current switch. If you wish to set it up, run the \
+           following command:\n\
            opam repository add dune-universe %s"
           Fmt.(styled `Bold string)
-          "opam monorepo lock" Config.duniverse_opam_repo)
+          "opam monorepo lock"
+          Fmt.(list ~sep:comma Opam.Pp.package_name)
+          non_dune_packages Config.duniverse_opam_repo)
 
 let filter_local_packages ~explicit_list local_paths =
   let res =
@@ -208,15 +212,24 @@ let get_pin_depends ~global_state local_opam_files =
   let open Rresult.R.Infix in
   root_pin_depends local_opam_files >>= pull_pin_depends ~global_state
 
+let interpret_solver_error ~switch_state = function
+  | `Msg _ as err -> err
+  | `Diagnostics d ->
+      (match Opam_solve.not_buildable_with_dune d with
+      | [] -> ()
+      | offending_packages ->
+          check_dune_universe_repo ~switch_state offending_packages);
+      Opam_solve.diagnostics_message d
+
 let calculate_opam ~build_only ~allow_jbuilder ~local_opam_files ~ocaml_version
     =
   let open Rresult.R.Infix in
   OpamGlobalState.with_ `Lock_none (fun global_state ->
       OpamSwitchState.with_ `Lock_none global_state (fun switch_state ->
-          check_repo_config ~switch_state;
           get_pin_depends ~global_state local_opam_files >>= fun pin_depends ->
           Opam_solve.calculate ~build_only ~allow_jbuilder ~local_opam_files
-            ~pin_depends ?ocaml_version switch_state))
+            ~pin_depends ?ocaml_version switch_state
+          |> Result.map_error ~f:(interpret_solver_error ~switch_state)))
 
 let run (`Repo repo) (`Recurse_opam recurse) (`Build_only build_only)
     (`Allow_jbuilder allow_jbuilder) (`Ocaml_version ocaml_version)

lib/opam.ml

@@ -177,6 +177,9 @@ end
 module Pp = struct
   let package fmt p = Format.fprintf fmt "%s" (OpamPackage.to_string p)
 
+  let package_name fmt p =
+    Format.fprintf fmt "%s" (OpamPackage.Name.to_string p)
+
   let hash = Hash.pp
 
   let url fmt url = Format.fprintf fmt "%s" (OpamUrl.to_string url)

lib/opam.mli

@@ -41,6 +41,8 @@ end
 module Pp : sig
   val package : OpamPackage.t Fmt.t
 
+  val package_name : OpamPackage.Name.t Fmt.t
+
   val hash : OpamHash.t Fmt.t
 
   val url : OpamUrl.t Fmt.t

lib/opam_solve.ml

@@ -1,7 +1,11 @@
 open Import
 
 module Switch_and_local_packages_context : sig
-  include Opam_0install.S.CONTEXT
+  type r =
+    | Non_dune
+    | Switch_rejection of Opam_0install.Switch_context.rejection
+
+  include Opam_0install.S.CONTEXT with type rejection = r
 
   val create :
     ?test:OpamPackage.Name.Set.t ->
@@ -19,10 +23,12 @@ end = struct
     allow_jbuilder : bool;
   }
 
-  type rejection =
+  type r =
     | Non_dune
     | Switch_rejection of Opam_0install.Switch_context.rejection
 
+  type rejection = r
+
   let pp_rejection fmt = function
     | Non_dune -> Fmt.pf fmt "Doesn't build with dune"
     | Switch_rejection r -> Opam_0install.Switch_context.pp_rejection fmt r
@@ -129,7 +135,7 @@ let calculate_raw ~build_only ~allow_jbuilder ~ocaml_version ~local_packages
   let request = request ~allow_compiler_variants local_packages_names in
   let result = Local_solver.solve context request in
   match result with
-  | Error e -> Error (`Msg (Local_solver.diagnostics e))
+  | Error e -> Error (`Diagnostics e)
   | Ok selections ->
       let packages = Local_solver.packages_of_result selections in
       let deps =
@@ -141,6 +147,35 @@ let calculate_raw ~build_only ~allow_jbuilder ~ocaml_version ~local_packages
       in
       Ok deps
 
+type diagnostics = Local_solver.diagnostics
+
+let diagnostics_message diagnostics =
+  `Msg (Local_solver.diagnostics diagnostics)
+
+module Pkg_map = Local_solver.Solver.Output.RoleMap
+
+let no_version_builds_with_dune component =
+  let rejects, _reason = Local_solver.Diagnostics.Component.rejects component in
+  match rejects with
+  | [] -> false
+  | _ ->
+      List.for_all
+        ~f:(fun (_, reason) ->
+          match reason with
+          | `Model_rejection Switch_and_local_packages_context.Non_dune -> true
+          | _ -> false)
+        rejects
+
+let not_buildable_with_dune diagnostics =
+  let rolemap = Local_solver.diagnostics_rolemap diagnostics in
+  Pkg_map.fold
+    (fun pkg component acc ->
+      match no_version_builds_with_dune component with
+      | false -> acc
+      | true -> pkg :: acc)
+    rolemap []
+  |> List.filter_map ~f:Local_solver.package_name
+
 let get_opam_info ~pin_depends ~switch_state pkg =
   let opam_file =
     match OpamPackage.Name.Map.find_opt pkg.OpamPackage.name pin_depends with

lib/opam_solve.mli

@@ -1,13 +1,21 @@
+type diagnostics
+
 val calculate :
   build_only:bool ->
   allow_jbuilder:bool ->
   local_opam_files:(OpamTypes.version * OpamFile.OPAM.t) OpamPackage.Name.Map.t ->
   pin_depends:(OpamTypes.version * OpamFile.OPAM.t) OpamPackage.Name.Map.t ->
   ?ocaml_version:string ->
   OpamStateTypes.unlocked OpamStateTypes.switch_state ->
-  (Opam.Package_summary.t list, [> `Msg of string ]) result
+  ( Opam.Package_summary.t list,
+    [> `Diagnostics of diagnostics | `Msg of string ] )
+  result
 (** Calculates a solution for the provided local packages and their opam files
     containing their regular and test dependencies using the provided opam switch
     state. Uses [Opam_0install].
     If [build_only] then no test dependencies are taken into account. If [ocaml_version]
     is provided, the solution will contain that concrete version of ocaml. *)
+
+val diagnostics_message : diagnostics -> [> `Msg of string ]
+
+val not_buildable_with_dune : diagnostics -> OpamPackage.Name.t list