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

Better candidate rejection info for exact constraints #170

Open
craigfe opened this issue May 29, 2021 · 5 comments
Open

Better candidate rejection info for exact constraints #170

craigfe opened this issue May 29, 2021 · 5 comments
Labels
solver-errors For all solver error related issues and features upstream Fix has to be implemented upstream

Comments

@craigfe
Copy link
Contributor

craigfe commented May 29, 2021

Here's an example of an error from opam monorepo lock:

               - ocamlformat -> (problem)
                   tezos-tooling zdev requires = 0.10
                   Rejected candidates:
                     ocamlformat.0.18.0: Incompatible with restriction: = 0.10
                     ocamlformat.0.17.0: Incompatible with restriction: = 0.10
                     ocamlformat.0.16.0: Requires ppxlib >= 0.18.0 & < 0.22.0
                     ocamlformat.0.15.1: Requires ppxlib >= 0.18.0 & < 0.22.0
                     ocamlformat.0.15.0: Requires ocaml-migrate-parsetree >= 1.7.3 & < 2.0.0
                     ...

In this case, I have a local opam file that's specifying an exact requirement on ocamlformat.0.10. That requirement can't be met, but this error message doesn't tell me why because it's hidden in the ellipses ....

If there's an exact local constraint on the version of a project, and that version can't be installed, then as a user that's the one I'm interested in fixing. Perhaps the error message could be changed to highlight that case? I'm thinking of something like:

               - ocamlformat -> (problem)
                   tezos-tooling zdev requires = 0.10, but this version cannot be selected:
                     ocamlformat.0.10: Requires ppxlib >= 0.18.0 & < 0.22.0
@NathanReb
Copy link
Contributor

We definitely need to do something about the error messages but I'm unsure what should and/or can be done by opam-monorepo and what needs to go in 0install. I'll take a look to see if there's something simple enough we can do to try and filter the rejections a bit, eventually defaulting to print everything again if don't find something sensible in there.

@raphael-proust
Copy link

I have a similar kind of unhelpful error message:

               - core -> (problem)
                   patdiff v0.15.0 requires >= v0.15 & < v0.16
                   Rejected candidates:
                     core.v0.11.3: Doesn't build with dune
                     core.v0.11.2: Doesn't build with dune
                     core.v0.11.1: Doesn't build with dune
                     core.v0.11.0: Doesn't build with dune
                     core.v0.10.0: Doesn't build with dune
                     ...

(when trying to lock a tagged version of patdiff)

I see two possible improvements:

  • Filter the list of rejection to only include those that are compatible with the version constraint. (The rationale is that lock is specifically invoked to work within a set of constraints and it is expected that it will reject anything that's out of bound. Being expected, it doesn't need to be shown to the user.) As an optional addition, the ellipsis could mention something along the lines of ... (plus 5 other packages) (plus 36 packages outside of the version constraints).

  • Sort the list of rejection explicitly by relevance of the rejection. Relevance is a bit vague and (because I don't know all the reasons that might be given) I'm not sure it can be made less vague.

@NathanReb
Copy link
Contributor

CC @Leonidas-from-XIV, I think the proper fix for this would also benefit from the upstream changes to 0install we mentioned while looking at #215 last week.

@raphael-proust indeed we'd ideally like to filter out the irrelevant versions here. Note that you should be able to run opam monorepo lock -vvv in the meantime to see the full error diagnostic.

@Leonidas-from-XIV
Copy link
Member

@raphael-proust The reason why you're seeing so many complaints about dune buildability is because as I learned while investigating #215 (leading to #248) is that the rejection on version is done after we reject on grounds of dune-buildability. So when rejecting for dune we'd need to know what version is requested, test whether it would violate the version constraint and then decide to not reject it, so it will later get rejected by 0install. Recovering the version constraint is quite tricky since the solver doesn't pass it along so my best bet so far has been parsing it out of the OPAM file which is rather fragile.

Honestly, I am not quite sure why 0install solves the dependencies in a way where it first excludes custom predicates and only then applies the version filtering - given even if we decide that a candidate is fine it might still get rejected on version grounds, so it seems like the candidate filtering is done on an unnecessarily big set of candidates and could make more sense to do in reverse: first filter versions and then run custom candidate filters. But I have no insight into the solver - I assume there is a reason for the current behavior that is probably quite apparent.

@raphael-proust
Copy link

I guess that monorepo simply reports the errors as returned by the solver. So the issue could either be fixed by changing the solver (or at least the output of the solver) or post-processing that output (although depending on the output that might be too brittle to do).

I admit I don't know how monorepo manages things in the background so I can't make concrete suggestions or comment meaningfully on different approaches.

@tmattio tmattio added bug Something isn't working solver-errors For all solver error related issues and features upstream Fix has to be implemented upstream and removed bug Something isn't working labels Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solver-errors For all solver error related issues and features upstream Fix has to be implemented upstream
Projects
None yet
Development

No branches or pull requests

5 participants