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

Support OCaml 5 new Unix API names #953

Merged
merged 4 commits into from
Jun 20, 2022

Conversation

dra27
Copy link
Contributor

@dra27 dra27 commented Jun 13, 2022

The compiler is considering ensuring all symbols in the Unix library are prefixed caml_ for 5.0 (ocaml/ocaml#10926).

This requires:

  • Pre-processing to select unix_exit or caml_unix_exit
  • The removal of redundant declarations for functions in caml/socketaddr.h
  • Version-based checking for the socket_domain_table and socket_type_table internal Unix symbols

@dra27
Copy link
Contributor Author

dra27 commented Jun 14, 2022

The upstream OCaml PR was merged - please could someone kick GitHub actions?

@tmcgilchrist
Copy link
Contributor

Re-running the analysis step to pickup dune 3.3.0 so this works on OCaml 5.0. Otherwise ocaml-ci should be working for this PR. @dra27

@smorimoto
Copy link
Member

I just kicked GitHub Actions.

@dra27 dra27 force-pushed the check-symbols-caml_unix branch from a61aaef to 9d4f2b5 Compare June 18, 2022 08:40
@dra27 dra27 force-pushed the check-symbols-caml_unix branch from eb94587 to 56fba76 Compare June 18, 2022 08:50
@dra27
Copy link
Contributor Author

dra27 commented Jun 18, 2022

The tests require a release of base with janestreet/base#129 (comment) addressed, although temporarily pinning it also revealed a missing symbol shim.

With dra27@9d4f2b5b4, this passed ocaml-ci on 5.0

@raphael-proust raphael-proust added the OCaml.5.0.0-readiness Getting the library ready for OCaml 5.0.0 label Jun 20, 2022
@raphael-proust
Copy link
Collaborator

Tested locally.

It just needs an entry in CHANGES. I'll push a commit for that.

Additionally, we could also add a check for 5.0.0~alpha0 in the CI. We currently already have 4.12+domains and 4.14+domains. Any reason not to do that? There's the fact that we'll want to modify it as the different alphas come in and then as 5.0.0 (non-alpha) is released. But the plus side is we can make sure that further PRs don't break anything.
I'm pushing a commit to that end. Let me know if you are ok with it.

@raphael-proust
Copy link
Collaborator

The ci is somewhat finicky because it's an alpha release. I'm merging this now and may address the CI later.

@dra27
Copy link
Contributor Author

dra27 commented Jun 20, 2022

I wholeheartedly back adding 5.0 testing to CI 🙂

@raphael-proust raphael-proust merged commit fbd4fc5 into ocsigen:master Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCaml.5.0.0-readiness Getting the library ready for OCaml 5.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants