-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rfc: disable libev build by default in the discovery scripts #618
Conversation
This simplifies the configure scripts to: - disable libev build by default unless it is supplied on the configure line. This does not affect most installations such as via opam (where the flag is explicitly specified depending on the presence of conf-libev) or OS packaging (where libev is typically a concrete dependency specified in the package data). However, this does make the build of Lwt much more portable by default when libev is not available. - remove writing of setup.data, which seems to be vestigial from the oasis build days. Simplifies the discovery script. - remove the auto-detection of libev on opam, which is not necessary since the opam package specifies it. Due to an unfortunate bug in opam 2.0.0 (ocaml/opam#3525) this is always true and so has different behaviour on opam1 vs 2. Signed-off-by: Anil Madhavapeddy <anil@recoil.org>
What does this refer to? The same commit that added auto-detection of Lines 18 to 20 in 38bf913
Removing that configure command was the purpose of adding the auto-detection of |
Before the auto-detection was added, we had Lines 18 to 21 in 88c7457
|
Oops, I hadn't noticed that particular change. I think we need a scheme where the configure phase can be run by package builds, but doesn't have to be (so it can be embedded easily in a bigger monorepo). So if the opam package still explicitly runs the configure.ml as it used to and libev defaults to false when not specified, that would leave most users unchanged but the defaults more portable. At least, I think it will... |
On the Lwt side, the consensus is that Lwt is much less useful without libev. We really want to compile with it if available on the target, and we want to make that the default in as many situations as possible. |
How about keeping the current detection, and just not terminating the build if libev isn't detected? We could print a warning in discover.ml and continue as if |
Yeah, Lwt should use libev if available, not use it if not available, and only fail if a flag was specified but libev is not correspondingly present or absent. Would you like to do that in this PR, or do you prefer I fix it? |
That sounds like a good solution to me. If you do have time to fix it to your satisfaction, I'd appreciate it -- otherwise I can look at it next week! |
That code is a holdover from OASIS days. Cherry-picked from #618.
This simplifies the configure scripts to:
disable libev build by default unless it is supplied on the configure line. This does not affect most installations such as via opam (where the flag is explicitly specified depending on the presence of conf-libev) or OS packaging (where libev is typically a concrete dependency specified in the package data). However, this does make the build of Lwt much more portable by default when libev is not available.
remove writing of setup.data, which seems to be vestigial from the oasis build days. Simplifies the discovery script.
remove the auto-detection of libev on opam, which is not necessary since the opam package specifies it. Due to an unfortunate bug in opam 2.0.0 (opam2 returns zero exit code too often for opam list ocaml/opam#3525) this is always true and so has different behaviour on opam1 vs 2.
The overall motivation for this PR is for embedding Lwt in the experimental ocaml platform monorepo, which bootstraps on all supported OCaml platforms with just a C compiler. It only needs libev for utop, and portability is significantly simpler if libev (and hence pkg-config) are not hard dependencies of this package for that purpose.
I'm opening this PR to discuss other options for out-of-the-box portability as well, in case I'm missing something far simpler!