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

ZArith version 1.14 #26226

Merged
merged 4 commits into from
Jul 15, 2024
Merged

ZArith version 1.14 #26226

merged 4 commits into from
Jul 15, 2024

Conversation

xavierleroy
Copy link
Contributor

ocaml/Zarith#148, ocaml/Zarith#149: Fail unmarshaling when it would produce non-canonical big ints
ocaml/Zarith#145, ocaml/Zarith#150: Use standard hash function for Z.hash and add Z.seeded_hash
ocaml/Zarith#140, ocaml/Zarith#147: Add fast path for Z.divisible on small arguments

Comment on lines 19 to 34
[
"sh"
"-exc"
"LDFLAGS=\"$LDFLAGS -L/opt/local/lib -L/usr/local/lib\" CFLAGS=\"$CFLAGS -I/opt/local/include -I/usr/local/include\" ./configure"
] {os = "macos" & os-distribution != "homebrew"}
[
"sh"
"-exc"
"LDFLAGS=\"$LDFLAGS -L/opt/local/lib -L/usr/local/lib\" CFLAGS=\"$CFLAGS -I/opt/local/include -I/usr/local/include\" ./configure"
] {os = "macos" & os-distribution = "homebrew" & arch = "x86_64" }
[
"sh"
"-exc"
"LDFLAGS=\"$LDFLAGS -L/opt/homebrew/lib\" CFLAGS=\"$CFLAGS -I/opt/homebrew/include\" ./configure"
] {os = "macos" & os-distribution = "homebrew" & arch = "arm64" }
[make]
Copy link
Member

Choose a reason for hiding this comment

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

Now that zarith supports pkg-config I think it may be worth just using that. At least on macos we could simply add conf-pkg-config as a dependency and use the same invocation as linux. Maybe this works also on freebsd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is tempting to rely on pkg-config, indeed. It works very well for developing on Linux and macOS. But I'm afraid of breaking what seems to be working today...

At any rate, if we go the pkg-config way, I'd like all the platforms to use it, no special cases if at all possible. But I realize I don't know how things work for Windows.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know about windows, so I would not touch anything there, but according to the tests I made for #23330 (for all packages with a checkmark I was also testing all the revdeps I could install on my computer) there seems to be no breakage

I would suggest to add a commit to use ["./configure"] on all systems and then we see from the CI if this works also on freebsd. Ping @kit-ty-kate, do you know if openbsd would work as well or needs a custom call?

Copy link
Member

Choose a reason for hiding this comment

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

as far as i know openbsd should work out of the box with pkg-config. The base system ships with pkg-config, its only problem is that it doesn't support some of the arguments but i'm not seeing the use of them in zarith so it should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YOLO. See latest commit.

@mseri
Copy link
Member

mseri commented Jul 15, 2024

The CI logs look good. Thanks a lot!

@mseri mseri merged commit eaf6fd7 into ocaml:master Jul 15, 2024
2 of 3 checks passed
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