Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve depext unavailable messages from the solver #4678

Merged
merged 4 commits into from May 27, 2021

Conversation

dra27
Copy link
Member

@dra27 dra27 commented May 24, 2021

Well, hopefully improve. At the moment, if you install a conf package directly, you get the depext availability message from before solving:

dra@thor:~/opam$ opam install conf-capnproto
[ERROR] Package conf-capnproto depends on the unavailable system package
        'capnproto libcapnp-dev'. You can use `--no-depexts' to attempt
        installation anyway.

although the message is not properly formatted. If, as is more likely, the conf package is pulled in via a dependency, the message comes after solving and is a little more intimidating:

dra@thor:~/opam$ opam install capnp-rpc-unix
[ERROR] Package conflict!
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-lwt < 0.2 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-lwt >= 0.2 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-net < 0.6.0 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-net = 0.6.0 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-net = 0.7.0 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-net = 0.8.0 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-net = 0.9.0 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-net = 1.0 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.
  * Missing dependency:
    - capnp-rpc-unix → capnp-rpc-net >= 1.1 → conf-capnproto
    depends on the unavailable system package 'capnproto libcapnp-dev'. Use
      `--assume-depexts' to attempt installation anyway.

No solution found, exiting

(and also features the same formatting problem).

This PR fixes the formatting of multiple missing depexts, so you now get:

dra@thor:~/opam$ opam install conf-capnproto
[ERROR] Package conf-capnproto depends on the unavailable system packages
        'capnproto' and 'libcapnp-dev'. You can use `--no-depexts' to attempt
        installation anyway.

and also spots that all the chains refer to the same missing dependency and reduces that to one explanation. It doesn't display any chain with that, because I don't think it's useful - all possible solutions required this depext, so the package name should be obvious to the user. Conversely, picking the right chain to display is extremely tricky, if even possible. You now get the slightly less scary-looking:

dra@thor:~/opam$ opam install capnp-rpc-unix
[ERROR] Package conflict!
  * Missing dependency:
    - conf-capnproto
    depends on the unavailable system packages 'capnproto' and
      'libcapnp-dev'. Use `--assume-depexts' to attempt installation anyway.

No solution found, exiting

The reduction should be extended to group the explanations first, but I wanted to get some feedback on the refactoring before doing that too.

@dra27 dra27 added this to the 2.1.0~rc milestone May 24, 2021
@dra27 dra27 requested review from AltGr and rjbou May 24, 2021 11:08
@dra27 dra27 added this to PR in Progress in Opam 2.1.x via automation May 24, 2021
@dra27 dra27 force-pushed the depext-chains branch 2 times, most recently from cf4cacc to c0ebdb0 Compare May 24, 2021 16:14
dra27 added 3 commits May 24, 2021 21:52
If the missing package in each case is the same, then don't bother with
the chains, simply display the details of the missing depext (since it
was *always* required).
@dra27 dra27 closed this May 25, 2021
Opam 2.1.x automation moved this from PR in Progress to Done May 25, 2021
@dra27 dra27 reopened this May 25, 2021
Opam 2.1.x automation moved this from Done to PR in Progress May 25, 2021
Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

src/solver/opamCudf.ml Outdated Show resolved Hide resolved
Opam 2.1.x automation moved this from PR in Progress to PR Finalised May 25, 2021
@rjbou rjbou moved this from PR Finalised to PR To review in Opam 2.1.x May 25, 2021
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
@dra27 dra27 moved this from PR To review to PR Finalised in Opam 2.1.x May 27, 2021
@dra27 dra27 merged commit 3a16738 into ocaml:master May 27, 2021
Opam 2.1.x automation moved this from PR Finalised to Done May 27, 2021
@dra27 dra27 deleted the depext-chains branch May 27, 2021 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Opam 2.1.x
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants