comby-tools/comby

Reduce set of library dependency (e.g., gmp, sqlite3, libev)

aryx opened this issue · 11 comments

aryx commented

Why does comby depend on gmp, sqlite3 and libev?

I would like to link with comby, but the less library dependencies it brings, the better.
Would it be possible to split comby in 2 packages where one is just the core library and matcher,
and another one with advanced features that requires gmp/sqlite3/libev?

aryx commented

For reference here is one of our CI test failing because of the mising dependencies: https://github.com/returntocorp/semgrep/pull/2844/checks?check_run_id=2244532897

I could add the library in our CI container, it's just it feels weird to require to install sqlite3 or gmp (??) for playing with semgrep/comby.

aryx commented

Maybe a comby-core.opam (or comby-lib) and a comby.opam packages?

Yeah this makes a lot of sense--up till now there hasn't been a good reason for me to split off a library because there wasn't a use case for it :-). The dependencies come from a mix of the application-level logic (interactive mode => lwt => libev, parallel library/zip support => sqlite, zlib). It looks like that build is having trouble with Core too, and I can probably factor that out too (or at least, Core_kernel). Would that also help?

Anyway let me give this a shot in the next day or three.

aryx commented

Excellent. Regarding Core i dont remember a pb but yep using Core_kernel or Base sounds good (lower priority though than the gmp/sqlite/libev thing)

aryx commented

And where does the dependency to gmp comes from? Core??

It's coming from the comby-server => tls => crypto stuff => zarith => gmp dependency, also something not needed in the library. Currently things are definitely a bit bloated as I was packaging everything in that first attempt to for opam 😄

I've reduced it down in comby-kernel to a small set of dependencies in #271. I will look at it tomorrow with fresh eyes and then PR to opam.

  "dune" {>= "2.7.0"}
  "ocaml" {>= "4.08.1"}
  "angstrom" {>= "0.15.0"}
  "core_kernel"
  "mparser"
  "mparser-pcre"
  "ppx_deriving"
  "ppx_deriving_yojson" {>= "3.6.0"}
  "pcre"
  "bisect_ppx" {with-test & dev & >= "2.5.0"}
aryx commented

Excellent! I see you use both angstrom and mparser. Which one do you prefer to use in the end? Just curious.

Up on opam, but I'll be AFK for a couple of hours, so will check up on that a bit later.

angstrom and mparser

I'm experimenting with both :-) I started with mparser and then did another (incomplete) implementation of the match engine in angstrom. It's a bit off a toss up still. mparser has built in support for turning PCRE into combinators while angstrom doesn't, so I'd have to add that. Between the two, both have some nice nice helper combinators that I miss in the other :-) angstrom perf is somewhat better than mparser but I haven't done any optimization/profiling here to say anything conclusive, so that's on me, not really a reflection of the library. I'd probably lean toward angstrom (?) but @murmour says he's working on a v2 mparser release so we'll see.

@aryx up on opam :-) I'll leave the issue open until whenever you've had a chance to confirm things are working

aryx commented

Already used the new package. Works perrfect. Thanks!