From 46b287f4fcf35b4cd31ad85378b7d37278390351 Mon Sep 17 00:00:00 2001 From: Stephen Sherratt Date: Thu, 27 Apr 2023 16:33:40 +1000 Subject: [PATCH] Simplify error message when deps don't build with dune When trying to generate a lockfile on a package which has dependencies which don't use dune as their build system, the diagnostic message generated by opam-0install is of little use. This change removes the opam-0install diagnostic message from the output of `opam monorepo lock` in the case where it fails due to dependencies not building with dune. Additionally this rewords the error message to highlight the salient information. In particular the list of non-dune packages is printed as a bulleted list rather than a comma-separated list inside the error message (it often takes me a minute to find the list of packages in the old error message). --- CHANGES.md | 5 ++ cli/lock.ml | 76 +++++++++++++++++++++---------- test/bin/no-build-command.t/run.t | 4 +- 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 01268b60f..23d972ee2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,11 @@ semantically equivalent URLs, this should reduce the risk of build failures due to duplicate code pulled (#118, #365 @TheLortex, @Leonidas-from-XIV) +- Simple the error message printed when dependencies don't use dune as their + build system. The opam-0install diagnostic message is no longer printed in + this case and the message has been reformatted and reworded to make the + salient information easier to see. (#384, @gridbugs) + ### Deprecated ### Fixed diff --git a/cli/lock.ml b/cli/lock.ml index b4e1f68e1..cd001da1a 100644 --- a/cli/lock.ml +++ b/cli/lock.ml @@ -84,25 +84,45 @@ let is_duniverse_repo repo_url = let url = OpamUrl.to_string repo_url in String.equal url D.Config.duniverse_opam_repo -let check_dune_universe_repo ~repositories non_dune_packages = +let error_message_when_dependencies_don't_build_with_dune ~repositories + non_dune_packages = let dune_universe_is_configured = List.exists ~f:is_duniverse_repo repositories in - if not dune_universe_is_configured then - Logs.warn (fun l -> - l - "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" - Fmt.(list ~sep:comma D.Opam.Pp.package_name) - non_dune_packages D.Config.duniverse_opam_repo) + let pp_package_name_bulleted_list = + Fmt.( + list ~sep:(any "\n") (fun ppf p -> + Fmt.pf ppf "- %a" D.Opam.Pp.package_name p)) + in + let dune_universe_state_message = + if dune_universe_is_configured then + Fmt.str + "The dune-universe opam repository (%s) contains dune ports of some \ + popular packages to help build more packages with dune however it \ + appears to already be set up on this switch. Thus it is possible that \ + no dune port exists for any of these packages.\n\n\ + For information on how to contribute a new dune port, see: \ + https://github.com/dune-universe/opam-overlays" + D.Config.duniverse_opam_repo + else + Fmt.str + "The dune-universe opam repository (%s) contains dune ports of some \ + popular packages to help build more packages with dune. It doesn't \ + appear to be set up on this switch. Adding it to this switch may fix \ + this issue. Add the dune-universe opam repository to this switch by \ + running the command:\n\n\ + opam repository add dune-universe %s" D.Config.duniverse_opam_repo + D.Config.duniverse_opam_repo + in + Fmt.str + "Some dependencies cannot be built with dune!\n\n\ + opam-monorepo requires that all dependencies use dune as their build \ + system.\n\n\ + These dependencies (possibly transitive) don't use dune as their build \ + system:\n\ + %a\n\n\ + %s" + pp_package_name_bulleted_list non_dune_packages dune_universe_state_message let read_opam fpath = let filename = @@ -214,15 +234,21 @@ let could_not_determine_version offending_packages = let interpret_solver_error ~repositories solver = function | `Msg _ as err -> err | `Diagnostics d -> - (match D.Opam_solve.unavailable_versions_due_to_constraints solver d with - | [] -> () - | offending_packages -> could_not_determine_version offending_packages); - (match D.Opam_solve.not_buildable_with_dune solver d with - | [] -> () - | offending_packages -> - check_dune_universe_repo ~repositories offending_packages); - let verbose = display_verbose_diagnostics (Logs.level ()) in - D.Opam_solve.diagnostics_message ~verbose solver d + let dependencies_which_don't_build_with_dune = + D.Opam_solve.not_buildable_with_dune solver d + in + if List.length dependencies_which_don't_build_with_dune > 0 then + `Msg + (error_message_when_dependencies_don't_build_with_dune ~repositories + dependencies_which_don't_build_with_dune) + else ( + (match + D.Opam_solve.unavailable_versions_due_to_constraints solver d + with + | [] -> () + | offending_packages -> could_not_determine_version offending_packages); + let verbose = display_verbose_diagnostics (Logs.level ()) in + D.Opam_solve.diagnostics_message ~verbose solver d) let dirname_of_fpath fpath = fpath |> Fpath.to_string |> OpamFilename.Dir.of_string diff --git a/test/bin/no-build-command.t/run.t b/test/bin/no-build-command.t/run.t index 4ea7474a6..6af2b8ca9 100644 --- a/test/bin/no-build-command.t/run.t +++ b/test/bin/no-build-command.t/run.t @@ -21,8 +21,8 @@ Attempt to generate a lockfile for a package which depends on depend-without-dune (this should fail due to the transitive dependency on without-dune which has a build command but doesn't depend on dune) - $ opam-monorepo lock test-depend-without-dune.opam 2>&1 | grep -o "Doesn't build with dune" - Doesn't build with dune + $ opam-monorepo lock test-depend-without-dune.opam 2>&1 | grep -o "Some dependencies cannot be built with dune!" + Some dependencies cannot be built with dune! Attempt to generate a lockfile for a package which depends only on depend-with-dune