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

Overhaul the Windows C stubs and update them for Unicode #5190

Merged
merged 8 commits into from
Jul 29, 2022

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Jul 21, 2022

Slightly easier seen commit-by-commit. This PR:

  • Removes some cruft from an earlier build system where the stubs were always built (including on Linux) if #ifdef _WIN32
  • Removes a small amount of code related to support OCaml < 4.06
  • Uses a somewhat less naïve application of OCaml's GC laws, which results in both shorter and technically more efficient code w.r.t. the GC
  • Adds missing uses of caml_string_is_c_safe (this is a guard which stops OCaml strings containing \0 from being passed to C functions which expect null-terminated strings) and releasing the runtime lock on a couple of longer-lived functions

Finally, the main focus is to switch the PR to use Unicode properly, in particular for the environment stubs implementation.

@dra27 dra27 added this to the 2.2.0~alpha milestone Jul 21, 2022
@dra27 dra27 added this to PR in progress in Opam 2.2.0 via automation Jul 21, 2022
@rjbou rjbou added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Jul 25, 2022
OCaml 4.06 is already the minimum required version, so process UTF-8
correctly in the system calls.
@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Jul 27, 2022
@dra27 dra27 merged commit eb5be54 into ocaml:master Jul 29, 2022
Opam 2.2.0 automation moved this from PR in progress to Done Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Opam 2.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants