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

Fix --best-effort regression introduced in #4975 #5261

Merged
merged 3 commits into from
Sep 1, 2022

Conversation

LasseBlaauwbroek
Copy link
Contributor

In PR #4796 I fixed a problem with --best-effort. Subsequently #4975 seems to
have broken this again. Not sure if this is the correct fix. There may also be
additional places where this is going wrong...

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.

Certainly rebasing #4975 reintroduced it, my mistkae. Thanks for re-fixing it!
If we can add a test it would be better to avoid that.

@LasseBlaauwbroek
Copy link
Contributor Author

(I'm not familiar with the testing code so if someone else wants to do that I'd be grateful)

@kit-ty-kate
Copy link
Member

kit-ty-kate commented Aug 29, 2022

(I'm not familiar with the testing code so if someone else wants to do that I'd be grateful)

Could you provide a rough test-case (rough steps, something that fails in your case) so we can try add it to the reftests on our side?

@LasseBlaauwbroek
Copy link
Contributor Author

The test from #4796 opam install coq-graph-theory.0.8 --best-effort I guess. Also, if it fits within the testing framework, you could count how many instances of opam-query: 1 occur in the cudf file.

kit-ty-kate added a commit to LasseBlaauwbroek/opam that referenced this pull request Aug 31, 2022
…ying to install specific versions of packages

See ocaml#5261
kit-ty-kate and others added 3 commits August 31, 2022 14:36
…ying to install specific versions of packages

See ocaml#5261
In PR ocaml#4796 I fixed a problem with `--best-effort`. Subsequently ocaml#4975 seems to
have broken this again. Not sure if this is the correct fix. There may also be
additional places where this is going wrong...
@kit-ty-kate kit-ty-kate added KIND: BUG PR: WIP Not for merge at this stage STATE: READY TO MERGE and removed PR: WIP Not for merge at this stage labels Aug 31, 2022
@kit-ty-kate
Copy link
Member

Thanks a lot!

@kit-ty-kate kit-ty-kate merged commit 6dbac8d into ocaml:master Sep 1, 2022
@LasseBlaauwbroek LasseBlaauwbroek deleted the refix-best-effort branch September 1, 2022 23:16
@rjbou rjbou added this to PR in progress in Opam 2.2.0 via automation Sep 9, 2022
@rjbou rjbou added this to the 2.2.0~alpha milestone Sep 9, 2022
@rjbou rjbou moved this from PR in progress to Done in Opam 2.2.0 Sep 9, 2022
Leonidas-from-XIV pushed a commit to Leonidas-from-XIV/opam that referenced this pull request Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Opam 2.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants