Remove the threaded/unthreaded split in ctypes-foreign.

[yallop]

Since #80, linking programs that use ctypes.foreign against the threads library has been optional: the package system would pick an appropriate implementation of the Foreign module according to whether the downstream program used threads.

This split into separate "unthreaded" and "threaded" implementations has mostly worked smoothly, but it's occasionally caused problems (e.g. #552) and it makes the code more difficult to maintain; all changes to ctypes-foreign (e.g. #595) have to take care to update both the unthreaded and threaded implementations consistently. Furthermore, since the split makes use of some more obscure parts of the build and package systems, the current design makes moving to a new system (#588) more challenging.

It seems likely that most environments that support ctypes-foreign (i.e. that support libffi) also support threads, in which case there's comparatively little value in continuing to support the unthreaded variant.

This PR therefore removes the split, of making ctypes-foreign-unthreaded the only implementation of the ctypes-foreign package. There should be no downstream breakages, but packages which require ctypes-foreign will now implicitly pull in threads as a transitive dependency. (Packages which use only ctypes and ctypes-stubs should be completely unaffected.)

[yallop]

There should be no downstream breakages, but packages which require ctypes-foreign will now implicitly pull in threads as a transitive dependency

This may in fact cause downstream breakages, as the Travis build failure suggests:

ocamlfind: [WARNING] Package `threads': Linking problems may arise because of the missing -thread or -vmthread switch

[aalekseyev]

In a program that uses a ctypes.foreign.threaded library directly, I'm seeing dune complaining with a message:

Library "ctypes.foreign.threaded" not found

My understanding is that I should switch to ctypes.foreign, instead. Is that right?

I have no problem doing that, but I'm curious, since you said that there should be no downstream breakages. Is there supposed to be some mechanism preventing this error that's malfunctioning in my setup?

[yallop]

My understanding is that I should switch to ctypes.foreign, instead. Is that right?

Yes, that should fix the issue.

I have no problem doing that, but I'm curious, since you said that there should be no downstream breakages.

When I wrote that I expected clients to refer to ctypes.foreign (rather than ctypes.foreign.unthreaded or ctypes.foreign.threaded) and let the build system pick the appropriate implementation. Apparently not all users do things that way.

(We could perhaps add ctypes.foreign.threadedas a kind of alias to ctypes.foreign to avoid the problem.)

[aalekseyev]

Having such an alias could simplify the migration, although ctypes.foreign is clearly a better name. I'll see if I can tweak our code to work with both 1.16 and 1.18. Thank you!

[ivg]

Just as feedback, it looks like that even when ctypes.foreign is used a failure in the downstream package is still possible, apparently the culprit is the findlib.dynload package,

/home/runner/.opam/4.11.1+flambda/bin/ocamlfind opt -o _build/bap/generate -linkpkg -package bap,bap-plugins,ctypes.stubs,ctypes.foreign,findlib.dynload,bap-main _build/bap/lib/bindings.cmx _build/bap/stub_generator/generate.cmx
Warning: : [WARNING] Package `threads': Linking problems may arise because of the missing -thread or -vmthread switch
File "/tmp/findlib_initlf31e30.ml", line 1:
Error: No implementations provided for the following modules:
         Thread referenced from /home/runner/.opam/4.11.1+flambda/lib/ctypes/ctypes-foreign.cmxa(Foreign)
         Mutex referenced from /home/runner/.opam/4.11.1+flambda/lib/ctypes/ctypes-foreign.cmxa(Foreign)