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

Mark conflict in zarith_js_stubs #24681

Merged
merged 4 commits into from
Jan 22, 2024

Conversation

raphael-proust
Copy link
Collaborator

Calling the extract function makes a runtime failure: [failure] ml_z_extract_small not implemented

See also janestreet/zarith_stubs_js#10

Note that the error is only triggered when calling the missing function, so it's not necessarily a strict conflict, but there is precedent in the other zarith version for similar issues.

@@ -14,7 +14,7 @@ depends: [
"dune" {>= "2.0.0"}
]
# The conflict is a run-time failure when versions do not match:
conflicts: "zarith" {< "1.12"}
conflicts: [ "zarith" {< "1.12" | >= "1.13" } ]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
conflicts: [ "zarith" {< "1.12" | >= "1.13" } ]
conflicts: [ "zarith" {< "1.12" | >= "1.13" } ]

Is there any compatible zarith version in this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess zarith.1.12 is the only version compatible with that release of zarith-stubs-js. But I'm a bit tired so I don't really trust myself on having gotten the comparison and operators right.

Btw, the previous constraint is from #21056

Copy link
Member

Choose a reason for hiding this comment

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

I was sleeping, I saw equal signs on both sides :)

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
conflicts: [ "zarith" {< "1.12" | >= "1.13" } ]
conflicts: [ "zarith" {!= "1.12" } ]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I pushed essentially this but added a comment too

Copy link
Member

Choose a reason for hiding this comment

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

The original < and >= was a better constraint, since it allows for any variants of zarith.1.12 to also match (e.g. zarith.1.12+dune, that sort of thing)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted to < and >=

@raphael-proust
Copy link
Collaborator Author

Well it seems there are more conflicts because there's a # Fatal error: exception Failure("ml_z_mul_overflows not implemented") with zarith.1.12 and zarith_stubs_js.0.12.0. But it might actually be that 0.12.0 is not actually copletely compatible with any of the zarith version? I'm not sure. I'll look into it bit more later.

@raphael-proust
Copy link
Collaborator Author

See also #24755

@raphael-proust
Copy link
Collaborator Author

So there are still CI errors. Most of which are irrelevant (they are transient errors, broken tezos packages, or some such) and a few of them are relevant (the data-encoding.1.0.0 error). However,

@mseri
Copy link
Member

mseri commented Jan 22, 2024

Thanks a lot

@mseri mseri merged commit cd449b2 into ocaml:master Jan 22, 2024
1 of 2 checks passed
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
* Mark conflict in zarith_js_stubs

* More conflicts in more versions

* Add comments explaining conflicts

* revert to < and >= constraints

see ocaml#24681 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants