Skip to content

Conversation

@avsm
Copy link

@avsm avsm commented May 27, 2014

The libffi-based Async_SSL bindings exhibit linking failures in Conduit/Cohttp due to an ocamlfind ordering bug. This changeset (via @yallop) switches the bindings to use the new C stub generation mode present in Ctypes 0.3 and higher.

The build uses Make since it's an awful lot easier to get working than ocamlbuild for the staging required for C bindings. The result finally allows Cohttp+Async+SSL secure http requests to work, fixing mirage/ocaml-cohttp#130

yallop and others added 10 commits April 10, 2014 01:02
C stubs (and the corresponding OCaml wrapper) are now generated in an
early stage of compilation, then linked into the library.

There's no longer a dependency on libffi.
include `<openssl/err.h>` to define `ERR_get_error`
Disable deprecation warnings for now, but revisit in the near future...
@ghost
Copy link

ghost commented May 28, 2014

Thanks, we'll merge it. I'll try with ocamlbuild though, to avoid having to maintain several different build systems. And once we have the jenga rules to build it it shouldn't be too hard to port them to ocamlbuild.

@avsm
Copy link
Author

avsm commented Jun 8, 2014

Any luck with ocamlbuild support? We're going to stick with a Makefile for sanity reasons -- I think that's the best way to build C libraries, and this is what you effectively have with C stub generation.

@ghost
Copy link

ghost commented Jun 9, 2014

I didn't got the time to finish it yet. Internally we can't use a Makefile, we need to convert the rules to jenga.

@ghost
Copy link

ghost commented Jun 9, 2014

I gave it another try. The ocamlbuild support was actually straightforward. I'll try to release that soon.

@ericbmerritt
Copy link

@avsm any updates here?

@avsm
Copy link
Author

avsm commented Jul 11, 2014

yep, ocaml/opam-repository#2313 now has the stub-generation async_ssl packaged up. being tested.

On 11 Jul 2014, at 15:38, Eric Merritt notifications@github.com wrote:

@avsm any updates here?


Reply to this email directly or view it on GitHub.

@ericbmerritt
Copy link

@avsm it looks like ocaml/opam-repository#2313 merged. How goes the testing?

@avsm
Copy link
Author

avsm commented Aug 5, 2014

Am travelling today/tomorrow -- it should be possible to edit the conduit build script to reenable async_ssl and verify the linking error has gone. I'm planning to issue a working Async SSL conduit just as soon as I'm back on solid ground...

On 4 Aug 2014, at 16:05, Eric Merritt notifications@github.com wrote

@avsm it looks like ocaml/opam-repository#2313 merged. How goes the testing?


Reply to this email directly or view it on GitHub.

@travisbrady
Copy link

Is there some way to get this built from source currently? The opam solution is preferable obviously but I'll take what I can get.

@bmillwood
Copy link
Contributor

I believe both from-source builds and OPAM should work absolutely fine now. Be aware that while we haven't merged this pull request, the changes are in there, I just forgot to close the issue when they were included.

Please let me know of any issues you have, or if you don't have any so I can close this :)

@travisbrady
Copy link

Thank you, @bmillwood. For the curious, I was able to get everything working with fresh clones of async_ssl, conduit and cohttp with the following patch applied to cohttp:

diff --git a/async/cohttp_async.ml b/async/cohttp_async.ml
index e9ff717..46ae2a3 100644
--- a/async/cohttp_async.ml
+++ b/async/cohttp_async.ml
@@ -26,11 +26,11 @@ module Net = struct
     match Uri_services.tcp_port_of_uri ~default:"http" uri with
     | None -> raise (Failure "Net.connect") (* TODO proper exception *)
     | Some port -> begin
-        let mode =
           match Uri.scheme uri with
-          | Some "https" -> `SSL
-          | _ -> `TCP in
-        Async_conduit.Client.connect ?interrupt ~mode ~host ~port ()
+          | Some "https" ->
+                Async_conduit.Client.connect ?interrupt (`SSL (host, port))
+          | _ ->
+                Async_conduit.Client.connect ?interrupt (`TCP (host, port))
       end
 end

@bmillwood
Copy link
Contributor

All right, then I'm going to go ahead and close this issue.

@bmillwood bmillwood closed this Aug 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants