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

Result conflict and ppxlib #814

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

Halbaroth
Copy link
Collaborator

We have to use ppxlib >= 0.30.0 and prevent conflict with result < 1.5.

We have to use ppxlib >= 0.30.0 and prevent conflict with
result < 1.5.
@bclement-ocp
Copy link
Collaborator

We shouldn't have a dependency on result at all, we don't use it. I thought we solved this by adding a version restriction on fmt instead; do we have other dependencies that bring in result?

We also don't use ppxlib, so we shouldn't have it as a dependency either. What is broken?

@Halbaroth
Copy link
Collaborator Author

Halbaroth commented Sep 13, 2023

ppxlib is used by recent ppx_blob. We have to use recent ppx_blob as older versions are not compatible with Dune > 3. If we don't use a recent version of ppxlib, it doesn't recognize user-defined binding operators:
https://v2.ocaml.org/manual/bindingops.html
I guess the AST of OCaml is not correct into too old ppxlib libraries.

I will check for the result dependency.

EDIT: if you prefer, I can add ppx_lib < 0.30.0 as a conflict clause instead of a dependency. It makes more sense.

@bclement-ocp
Copy link
Collaborator

ppxlib is used by recent ppx_blob. We have to use recent ppx_blob as older versions are not compatible with Dune > 3. If we don't use a recent version of ppxlib, it doesn't recognize user-defined binding operators:

Ah, so ppx_blob is the one that probably doesn't have proper dependency constraints, OK. Adding a conflicts clause on old ppxlib (with a comment that it's for ppx_blob) makes more sense then, good idea!

@bclement-ocp
Copy link
Collaborator

I think result is logs through ocplib-simplex, OK for that then :'(

Copy link
Collaborator

@bclement-ocp bclement-ocp left a comment

Choose a reason for hiding this comment

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

Let's merge this as is then, since #815 was merged already it will make things easier for us. We can clean up the ppxlib dependency into a conflicts once we are done with 2.5.0

@Halbaroth Halbaroth merged commit 5eca9cd into OCamlPro:next Sep 13, 2023
14 checks passed
Halbaroth added a commit to Halbaroth/alt-ergo that referenced this pull request Sep 15, 2023
We have to use ppxlib >= 0.30.0 and prevent conflict with
result < 1.5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants