Avoid C namespace pollution by keeping implementations static #36
Conversation
As reported in #35, some of the C files in this library (e.g. src/poly1305-donna.c) are also present in other libraries, causing name collisions for the functions they export. This commit applies a `static` modifier to all C functions exported by these files, and includes directly the C files in the stub-*.c files. In the end, the only functions exported are now the OCaml/C stub functions, prefixed by `caml_`. The `static` modifier is hidden behind a `EXPORT` macro, so that the original visibility can be restored by compiling with `-DEXPORT=`, if that's ever needed. While we're at it: removed the remaining `$Id` comments.
|
While I was at it: I also removed the remaining This PR is easier to read if whitespace changes (coming from re-indenting) are ignored. |
| -DBLAKE3_NO_SSE2 -DBLAKE3_NO_SSE41 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_AVX512 | ||
| -DBLAKE3_USE_NEON=0 | ||
| (:include flags.sexp)) | ||
| (names aesni | ||
| arcfour |
There was a problem hiding this comment.
removing these names from the dune file makes them unavailable in dune's _build directory, leading to the failures you're seeing in ocaml/opam-repository#26209
There was a problem hiding this comment.
It works just fine locally when I do dune build, so I don't understand what OPAM does that causes the build to fail. Any idea?
At any rate, these .c files are used as include files, so I don't want Dune to build them and include them in the library.
There was a problem hiding this comment.
does it also work locally if you run dune clean && dune build -p cryptokit?
I have 2 hypotheses:
- you had a stale build so those files were already in the build directory
- dune only filters those files out in the release archive
There was a problem hiding this comment.
It works with dune build but not with dune build -p cryptokit. I miss the old Makefile...
There was a problem hiding this comment.
BTW, what do you mean by "the release archive"? These .c files are sources used at build-time but are not part of what's installed. I'm totally confused :-(
There was a problem hiding this comment.
I think we invalidated hypothesis 2, and I'm not sure it would hold either
There was a problem hiding this comment.
@xavierleroy I'm working on a fix, will send a PR shortly
As reported in #35, some of the C files in this library (e.g. src/poly1305-donna.c) are also present in other libraries, causing name collisions for the functions they export.
This commit applies a
staticmodifier to all C functions exported by these files, and includes directly the C files in the stub-*.c files.In the end, the only functions exported are now the OCaml/C stub functions, prefixed by
caml_.The
staticmodifier is hidden behind aEXPORTmacro, so that the original visibility can be restored by compiling with-DEXPORT=, if that's ever needed.