Skip to content

Commit 40ade10

Browse files
committed
Re-update src/unix/config/discover.exe to be able to cross-compile lwt
The default case of [C_library_flags.detect] puts systematically [-I/usr/include] and [-L/usr/lib] even in a cross-compilation context which is wrong because [/usr/include/pthread.h] can clash [cross-env/pthread.h] for instance. The default case about underlying libraries (libev or pthread) should be available only from a restrictive set of flags instead of a pervasive one. This patch restricts this set to what we really need instead to pervasively picks some random flags. A clarification about the recognition of [pthread] was made too to really understand the behavior of [discover.exe] and give a chance to be more reproducible afterwards
1 parent 0cec6b4 commit 40ade10

File tree

1 file changed

+51
-19
lines changed

1 file changed

+51
-19
lines changed

src/unix/config/discover.ml

+51-19
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ sig
160160

161161
val ws2_32_lib : Configurator.t -> unit
162162

163+
val set_c_flags : string list -> unit
164+
val set_link_flags : string list -> unit
163165
val c_flags : unit -> string list
164166
val link_flags : unit -> string list
165167
val add_link_flags : string list -> unit
@@ -254,9 +256,14 @@ struct
254256
(fun path -> Sys.file_exists (path // "include" // header))
255257
(Lazy.force search_paths)
256258
in
259+
(* NOTE: for the cross-compilation sake, we should not arbitrarily
260+
* include ([-I]) some paths which can clash some cross-compilation's
261+
* definitions with host's definitions. The default case about flags
262+
* should always be less than more - and we should put these flags
263+
* only we really require them. *)
257264
extend
258-
["-I" ^ (path // "include")]
259-
["-L" ^ (path // "lib"); "-l" ^ library]
265+
[] (* ["-I" ^ (path // "include")] *)
266+
[] (* ["-L" ^ (path // "lib"); "-l" ^ library] *)
260267
with Not_found ->
261268
()
262269

@@ -268,6 +275,9 @@ struct
268275
else
269276
extend unicode ["-lws2_32"]
270277

278+
let set_c_flags lst = c_flags := lst
279+
let set_link_flags lst = link_flags := lst
280+
271281
let c_flags () =
272282
!c_flags
273283

@@ -463,26 +473,48 @@ struct
463473
}
464474
|}
465475
in
466-
(* On some platforms, pthread is included in the standard library, but
467-
linking with -lpthread fails. So, try to link the test code without
468-
any flags first.
469-
470-
If that fails and we are not targetting Android, try to link with
471-
-lpthread. If *that* fails, search for libpthread in the filesystem.
472-
473-
When targetting Android, compiling without -lpthread is the only way
474-
to link with pthread, and we don't to search for libpthread, because
475-
if we find it, it is likely the host's libpthread. *)
476-
match compiles ~c_flags:[] ~link_flags:[] context code,
477-
compiles context code with
478-
| Some true, Some true -> Some true
479-
| Some false, Some true
480-
| Some true, Some false -> Some true
476+
(* To clarify the semantic of the recognition of [pthread]:
477+
1) [pthread] can be _standalone_ (included in the standard library)
478+
depending on the C compiler
479+
1.1) A restrictive context (such as a cross-compilation context)
480+
requires, at least, [-lpthread] but [-I] and [-L] can
481+
disturb the compilation between the host's [pthread] and the
482+
cross-compiled [pthread]. We test above all and for all this
483+
tricky context with **only one** flag [-lpthread]
484+
1.2) On some platforms, if [pthread] is standalone, the linker
485+
fails when we link with [-lpthread]. So we test our code
486+
with **default** flags (such as [-I/usr/include] and
487+
[-L/usr/lib]) and **without** [-pthread]
488+
2) On Android, compiling without [-lpthread] is the only way to link
489+
with [pthread], and we don't to search for [pthread.a], because
490+
if we find it, it is likely the host's [pthread]
491+
3) We finally retest our code with [-lpthread] and basic [-L] and
492+
[-I] flags (from the host system)
493+
494+
NOTE(dinosaure):
495+
- 2) and 1.1) should be merged, we definitely should try to compile
496+
the code **without any flags** and see results - by this way, we
497+
consider that the _toolchain_ leads us about where is
498+
[pthread].
499+
- 3) is too ~vague~ and obviously works but it's difficult to really
500+
understand which [pthread] we really use.
501+
- A question remains about priorities: do we want to prioritize
502+
the [dune]'s context or do we prefer a compilation for the host
503+
system first?
504+
- In anyway, [discover.exe] should be less pervasives (no [ref]
505+
about flags) and more strict and reproducible *)
506+
match (* 1.2 *) compiles context code,
507+
(* 1.1 *) compiles ~c_flags:[] ~link_flags:[ "-lpthread" ] context code with
508+
| _, Some true (* prioritize [dune]'s context and cross-compilation *) ->
509+
C_library_flags.set_c_flags [] ;
510+
C_library_flags.set_link_flags [ "-lpthread" ] ;
511+
Some true
512+
| Some true, _ -> Some true
481513
| _no ->
482-
if !Arguments.android_target = Some true then
514+
if (* 2 *) !Arguments.android_target = Some true then
483515
Some false
484516
else begin
485-
match compiles context code ~link_flags:["-lpthread"] with
517+
match (* 3 *) compiles context code ~link_flags:["-lpthread"] with
486518
| Some true ->
487519
C_library_flags.add_link_flags ["-lpthread"];
488520
Some true

0 commit comments

Comments
 (0)