Ctypes binding doesn't support well local headers

[bobot]

Local headers are not well supported with the ctypes, the tests with vendored stanza makes it clear:

dune/test/blackbox-tests/test-cases/ctypes/lib-vendored.t/stubgen/dune

Lines 17 to 21 in d733ec0

	(vendored
	; hack: multiple -I directives to work around cc commands being run from different
	; relative directories. Is there a cleaner way to do this?
	(c_flags ("-Istubgen/vendor" "-Ivendor"))))
	(headers (include "example.h"))

having (headers (local <file>)) or (headers (include_dirs <dir>)) which would add the correct dependencies and the correct -I according to the current working directory would make the vendor feature more usable.

CC @mbacarella

[bobot]

Moreovere I'm not sure that the headers of installed libraries are used by ctypes sub-stanza. For example adding (libraries zarith) doesn't make zarith.h available.

[mbacarella]

I agree this part sucks. Something that would be nice to fix for v0.2.

For example adding (libraries zarith) doesn't make zarith.h available.

This is probable. What machinery in dune sets this up?

[bobot]

What machinery in dune sets this up?

What do you mean? The set of libraries and there location should be easy to get and also adding the right options to the compilation phase of ctypes. With the points of view of the user, the goal would just be to ensures this of the documentation.

[bobot]

I also have difficulties with the compilation of the generated stubs. I had to set (use_standard_c_and_cxx_flags false) because I was unable to get -fpicadded. I don't know how to add :standardto cflags and lflags. At least the documentation of the ctypes sub-stanza should mention it before final 3.0. The ctypes feature is experimental, but I would prefer to document the current known shortcoming so that people don't lose too much time.

CC @voodoos

[mbacarella]

having (headers (local )) or (headers (include_dirs <dir>)) which would add the correct dependencies and the correct -I according to the current working directory would make the vendor feature more usable.

Sounds promising. Let's formalize this a bit more. Right now the headers directive type is:

module Headers : sig
  type t =
    | Include of string list
    | Preamble of string
end

At the very least here we've talked about

module Headers : sig
  type t =
    (* 'preamble' pastes the string into the top of the C stubs, no extra dune rules *)
    | Preamble of string
    (* 'include' emits [#include <file.h>]s, no extra dune rules *)
    | Include of string list
    (* 'local' is like 'include', but also adds a dune rule dep for each file in the list *)
    | Local of string list
    (* `include_dirs` adds a dependency to each dir in the list, and emits a C
        compiler `-I` for each dir as well *)
    | Include_dirs of string list
end

To provide flexibility for arbitrarily weird cases, we would probably want to allow repeated use of (headers ...) stanza, evaluated in the order they appear in the dune-file.

Does this sound encompassing enough?

[bobot]

It seems very nice for the vendor case. For include_dirs, would it be a glob? Perhaps the deps field of #5346 is enough and can stay in a final version. So only local?

Remains something for the ocaml library case:

• we can document and test the %{lib:zarith}/zarith.h, and precise this documentation
• we have something more automatic, but harder to design right.

nitpick: I believe it will be Path.Build.t instead of string for the two new cases.

[bobot]

@mbacarella additionally since 2418f62 we can't use -I %{lib:zarith:.} in order to include lib directory, and ctypes doesn't support include_dirs of foreign_stubs

EDIT: more precisely ìnclude_dirs can be added in foreign_stubs in parallel to a ctypes, but it failed when no (names ...) is given. Perhaps something need to be fixed, but perhaps also include_dirs from a library can be added alsodirectly to ctypes.

[bobot]

Also -Idir breaks when vendoring the library, such contraption are needed: bobot/ocaml-flint@630a749 . Perhaps a %{pwd} would be nice.

[emillon]

I consider that #6883 fixes this since it's now possible to refer to local headers using a relative path. This does not encompass every use case so please open feature requests if necessary.