[ctypes] Add support for cstubs codegen errno parameter by droyo · Pull Request #5827 · ocaml/dune

droyo · 2022-06-04T02:24:59Z

This parameter dates back to 2016, and switches the generated
code between returning just the C function's return value, to a tuple
of (retval, errno).

Signed-off-by: David Arroyo david@aqwari.net

rgrinberg · 2022-06-05T20:30:36Z

Summoning @mbacarella

mbacarella · 2022-06-05T22:02:55Z

awesome!

lgtm, though can you add a blackbox test for it @droyo ?

@rgrinberg since this adds a config directive, should we bump the ctypes extension syntax version from 0.1 to 0.2?

rgrinberg · 2022-06-05T22:52:20Z

since this adds a config directive, should we bump the ctypes extension syntax version from 0.1 to 0.2?

defo

droyo · 2022-06-10T20:08:54Z

Sorry for the delay @mbacarella , I was away from my computer. I added a blackbox test, but it is affected by #5809 (this was actually the change that made me find the issue to begin with). I've not yet rebased it against your tentative fix.

droyo · 2022-06-10T20:15:51Z

I have locally rebased this against #5816 and verified the test passes.

rgrinberg · 2022-06-14T14:29:37Z

@droyo could you sign off on the commits?

mbacarella · 2022-06-14T18:27:26Z

@droyo thanks for the tests. do you want to take a crack at making the errno_policy flag a 0.2-only feature?

droyo · 2022-06-16T10:16:55Z

@droyo could you sign off on the commits?

done. I also squashed the commits into one.

@droyo thanks for the tests. do you want to take a crack at making the errno_policy flag a 0.2-only feature?

done, sorry for the delay. I hope I did it right.

mbacarella · 2022-06-16T21:51:27Z

src/dune_rules/ctypes_stanza.ml

+let syntax =
+  Dune_lang.Syntax.create ~name ~desc:"the ctypes extension"
+    [ ((0, 1), `Since (3, 0))
+    ; ((0, 2), `Since (3, 0)) ]


should this line say `Since (3, 3) ?
@rgrinberg

or 3.4 now, rather?

Yes indeed.

mbacarella · 2022-06-19T17:23:23Z

not sure why the DCO check is failing, there appears to be a sign-off?

EDIT: oh, maybe the workflow needs approved to re-run the DCO check (even though that check costs $0)

This parameter dates back to 2016, and switches the generated code between returning just the C function's return value, to a tuple of (retval, errno). Signed-off-by: David Arroyo <david@aqwari.net>

droyo · 2022-06-19T20:24:35Z

not sure why the DCO check is failing, there appears to be a sign-off?

I think it was because I was missing the < > brackets around the email in the commit message. I fixed that and it's passing now.

mbacarella · 2022-06-19T21:46:52Z

The build tests fail on ubuntu and macos 4.14.x because:

+  File "dune-project", line 2, characters 14-17:
+  2 | (using ctypes 0.2)
+                    ^^^
+  Error: Version 0.2 of the ctypes extension is not supported until version 3.4

fine, but why does ubuntu 4.13 succeed?

rgrinberg · 2022-06-20T00:25:37Z

Merged. Thanks

but why does ubuntu 4.13 succeed?

We don't run the tests on older versions (CI is slow enough already)

mbacarella · 2022-06-20T00:30:38Z

was merged to main here 5821266

@esope

…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.4.0) CHANGES: - Make `dune describe` correctly handle overlapping implementations for virtual libraries (ocaml/dune#5971, fixes ocaml/dune#5747, @esope) - Building the `@check` alias should make sure the libraries and executables don't have dependency cycles (ocaml/dune#5892, @rgrinberg) - [ctypes] Add support for the `errno` parameter using the `errno_policy` field in the ctypes settings. (ocaml/dune#5827, @droyo) - Fix `dune coq top` when it is invoked on files from a subdirectory of the directory containing the associated stanza (ocaml/dune#5784, fixes ocaml/dune#5552, @ejgallego, @rlepigre, @Alizter) - Fix hint when an invalid module name is found. (ocaml/dune#5922, fixes ocaml/dune#5273, @emillon) - The `(cat)` action now supports several files. (ocaml/dune#5928, fixes ocaml/dune#5795, @emillon) - Dune no longer uses shimmed `META` files for OCaml 5.x, solely using the ones installed by the compiler. (ocaml/dune#5916, @dra27) - Fix handling of the `(deps)` field in `(test)` stanzas when there is an `.expected` file. (ocaml/dune#5952, ocaml/dune#5951, fixes ocaml/dune#5950, @emillon) - Ignore insignificant filesystem events. This stops RPC in watch mode from flashing errors on insignificant file system events such as changes in the `.git/` directory. (ocaml/dune#5953, @rgrinberg) - Fix parsing more error messages emitted by the OCaml compiler. In particular, messages where the excerpt line number started with a blank character were skipped. (ocaml/dune#5981, @rgrinberg) - env stanza: warn if some rules are ignored because they appear after a wildcard rule. (ocaml/dune#5898, fixes ocaml/dune#5886, @emillon) - On Windows, XDG_CACHE_HOME is taken to be the `FOLDERID_InternetCache` if unset, and XDG_CONFIG_HOME and XDG_DATA_HOME are both taken to be `FOLDERID_LocalAppData` if unset. (ocaml/dune#5943, fixes ocaml/dune#5808, @nojb)

droyo requested a review from christinerose as a code owner June 4, 2022 02:25

christinerose approved these changes Jun 6, 2022

View reviewed changes

droyo force-pushed the ctypes-errno branch 2 times, most recently from 738009a to 3c8f570 Compare June 14, 2022 12:50

rgrinberg added this to the 3.3.0 milestone Jun 14, 2022

droyo force-pushed the ctypes-errno branch from 3c8f570 to fd420da Compare June 16, 2022 10:14

rgrinberg requested a review from mbacarella June 16, 2022 18:48

rgrinberg modified the milestones: 3.3.0, 3.4.0 Jun 17, 2022

mbacarella approved these changes Jun 18, 2022

View reviewed changes

droyo force-pushed the ctypes-errno branch 2 times, most recently from 64a024e to dadddb1 Compare June 19, 2022 13:01

mbacarella approved these changes Jun 19, 2022

View reviewed changes

Add support for cstubs errno_policy parameter

530c1e6

This parameter dates back to 2016, and switches the generated code between returning just the C function's return value, to a tuple of (retval, errno). Signed-off-by: David Arroyo <david@aqwari.net>

droyo force-pushed the ctypes-errno branch from dadddb1 to 530c1e6 Compare June 19, 2022 20:23

rgrinberg closed this Jun 20, 2022

emillon mentioned this pull request Jul 20, 2022

[new release] xdg, stdune, ordering, ocamlc-loc, fiber, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.4.0) ocaml/opam-repository#21845

Merged

Conversation

droyo commented Jun 4, 2022

Uh oh!

rgrinberg commented Jun 5, 2022

Uh oh!

mbacarella commented Jun 5, 2022

Uh oh!

rgrinberg commented Jun 5, 2022

Uh oh!

droyo commented Jun 10, 2022

Uh oh!

droyo commented Jun 10, 2022

Uh oh!

rgrinberg commented Jun 14, 2022

Uh oh!

mbacarella commented Jun 14, 2022

Uh oh!

droyo commented Jun 16, 2022

Uh oh!

mbacarella Jun 16, 2022

Choose a reason for hiding this comment

Uh oh!

mbacarella Jun 17, 2022

Choose a reason for hiding this comment

Uh oh!

rgrinberg Jun 19, 2022

Choose a reason for hiding this comment

Uh oh!

droyo Jun 19, 2022

Choose a reason for hiding this comment

Uh oh!

mbacarella commented Jun 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

droyo commented Jun 19, 2022

Uh oh!

mbacarella commented Jun 19, 2022

Uh oh!

rgrinberg commented Jun 20, 2022

Uh oh!

mbacarella commented Jun 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mbacarella commented Jun 19, 2022 •

edited

Loading