-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ZArith version 1.14 #26226
Conversation
packages/zarith/zarith.1.14/opam
Outdated
[ | ||
"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] |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YOLO. See latest commit.
By popular demand.
The CI logs look good. Thanks a lot! |
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