Skip to content
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

Support instrumenting jbuilder projects #15

Open
Leonidas-from-XIV opened this issue Jan 12, 2018 · 3 comments
Open

Support instrumenting jbuilder projects #15

Leonidas-from-XIV opened this issue Jan 12, 2018 · 3 comments

Comments

@Leonidas-from-XIV
Copy link

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
Copy link
Contributor

aantron commented Jan 12, 2018

@Leonidas-from-XIV, Jbuilder and Bisect_ppx currently don't work so well together. Here is the Jbuilder issue for tracking this: ocaml/dune#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: ocaml/dune#57.

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

@simonjbeaumont
Copy link
Owner

@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
Copy link
Contributor

aantron commented Jan 19, 2018

@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.

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

No branches or pull requests

3 participants