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

Added cloudi.2.0.7 #24689

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Added cloudi.2.0.7 #24689

merged 1 commit into from
Nov 22, 2023

Conversation

okeuday
Copy link
Contributor

@okeuday okeuday commented Oct 27, 2023

No description provided.

@okeuday
Copy link
Contributor Author

okeuday commented Oct 27, 2023

Only macos-homebrew-ocaml-5.1-arm64 (experimental) and macos-homebrew-ocaml-4.14-arm64 (experimental) failed due to not having GMPlib for zarith to use.

@mseri
Copy link
Member

mseri commented Oct 30, 2023

Strange, gmp should be there. I think homebrew uses a different path on arm, maybe the build script is looking in the wrong place

@okeuday
Copy link
Contributor Author

okeuday commented Oct 31, 2023

@mseri The failure occurs in the zarith-1.13 release that is included in the repository, for compilation in isolation. So, the GMPlib search fix should occur as part of zarith (its build system is custom and isn't yet attempting to use autoconf/automake or something similar to avoid problems).

I added big integer support to the Erlang External Term Format implementation used by this source code when OCaml still included big integer support with num-1.1, due to big integer support being a part of OCaml at that time. I could remove big integer support if you think that is a good approach to resolve the problem (by removing the use of zarith).

I would like to think that zarith could be a part of OCaml in the future, but the current OCaml direction appears to consider big integer use as unimportant for OCaml use.

@mseri mseri merged commit 5b1a81f into ocaml:master Nov 22, 2023
@mseri
Copy link
Member

mseri commented Nov 22, 2023

Thanks!

@mseri
Copy link
Member

mseri commented Nov 22, 2023

Perhaps I was too fast here at merging. What is the point of requiring zarith but also vendoring it?

@okeuday
Copy link
Contributor Author

okeuday commented Nov 22, 2023

@mseri The approach is intentional to catch problems early and improve the in-place compilation in the main repository (https://github.com/CloudI/CloudI/tree/develop/src/api/ocaml) as necessary. Using this approach, there is always the assurance that the build of the OCaml CloudI API is the same as in the main repository.

depends: [
"ocaml" {>= "4.03.0"}
"base-unix"
"zarith" {>= "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.

In this case you don't really need this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While it might not be strictly necessary, I wanted to be transparent about the dependency on zarith, so it appears in the cloudi package information.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the clarification

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.

2 participants