search

simonjbeaumont / ocaml-travis-coveralls

:jeans: Push your coverage metrics to Coveralls.io from Travis-ci
4 stars 3 forks source link

Support instrumenting jbuilder projects #15

Open Leonidas-from-XIV opened 6 years ago

Leonidas-from-XIV commented 6 years ago

Thanks for your scripts. I've been using them to report coverage on my oasis projects. But along with migrating to a more standard build I decided to replace it with jbuilder. Unfortunately these scripts don't support instrumenting jbuild files to add bisect_ppx, could you add such support?

I suppose the cleanest way would be to use sexplib, read in the jbuild files, edit and write them out, but that requires building an OCaml executable as part of these scripts which might add quite the complication.

aantron commented 6 years ago

@Leonidas-from-XIV, Jbuilder and Bisect_ppx currently don't work so well together. Here is the Jbuilder issue for tracking this: https://github.com/janestreet/jbuilder/issues/57. You have probably seen the Bisect_ppx docs about how to use it with Jbuilder: https://github.com/aantron/bisect_ppx/blob/master/doc/advanced.md#Jbuilder. You can see links to Bisect_ppx+Jbuilder "in action" listed in this comment: https://github.com/janestreet/jbuilder/issues/57.

I suspect you're actually aware of all this, but just in case :)

simonjbeaumont commented 6 years ago

@Leonidas-from-XIV thanks for the request and thanks @aantron for the useful pointers.

Unfortunately the company I work for now is not allowing me to develop these opensource projects. I am allowed to review and merge pull-requests and fix bugs but I will not be able to add this support myself.

I would happily support any efforts toward this goal.

Thanks for making use of this in your project(s) to date.

aantron commented 6 years ago

@Leonidas-from-XIV You may also be interested in this old jbuild file of Lwt, where we inserted bisect_ppx only if an environment variable was set: https://github.com/ocsigen/lwt/blob/f75fd2f661cdb686a10e9505afbe9b4055cc1604/src/core/jbuild#L15-L17.

We since replaced that with including bisect_ppx unconditionally, but passing it the -conditional flag, and deleting the dependency on bisect_ppx when tagging a release.