Prepare for Dune integration

[aantron]

ocaml/dune#57 has an initial solution. Among other issues, coverage output and/or source code are likely to appear in _build/coverage rather than _build/default, so the Dune integration on the Bisect end should be slightly adjusted to search for files in directories other than _build/default.

Outstanding upstream issues:

• Dune + Bisect integration runs tests twice ocaml/dune#3472
• Crash with (bisect_ppx) and (lang dune 2.5) ocaml/dune#3473
• Implement instrumentation RFC ocaml/dune#3535

Then:

• Adjust default coverage search paths
• Update Dune instructions
• Deprecate --conditional

[hannesm]

Hey @aantron, I was interested in this (and it looks like dune 2.6.0 with that feature was released some time ago). What is the status of bisect and dune (an update to the README and instructions would be highly welcome). Thanks :)

[aantron]

@hannesm It is currently waiting for the release of ocaml/dune#3535 :)

[hannesm]

thanks @aantron, I was not aware of that PR -- looking forward that it finally is getting there :)

[hannesm]

friendly ping; AFAICT that has been merged and is part of the dune 2.7.0 release merged recently to opam-repository. thanks :)

[undu]

I've gotten the instrumentation integration working, but it requires dune 2.7. What are the opinions regarding this new requirement to enable this integration?

[aantron]

If the change is backwards-compatible, in that the existing older integration method still works, we can ask users to require "dune" >= 2.7 in the Bisect README. As I recall, we used to do this for earlier versions of Dune for other reasons. @undu, is this what you mean?

[undu]

Yes, that's what I meant.

While the previous integration still works, the new integration requires ppx_bisect to be built using at least dune 2.7.0, it doesn't matter if the new or the old integration is used.

[aantron]

Is there a change to the code of Bisect that requires Dune 2.7.0 even if the old integration is being used? If yes, then we have to require Dune 2.7.0 in Bisect. If not, we can ask the user to require Dune at least 2.7.0 in the README if they are using the new integration. If that becomes a pain, we can still move the constraint up to Bisect later. However, in the past, it has seemed to work well, because we just put the Dune dependency into the example of how to depend on Bisect as well, i.e.

depends: [
  "bisect_ppx" {dev & >= "2.5.0"}
  "dune" {>= "2.7.0"}
]

[undu]

Is there a change to the code of Bisect that requires Dune 2.7.0 even if the old integration is being used?

Yes. It's the field in the ppx's dune that marks it as instrumentation.

(instrumentation.backend (ppx bisect_ppx))

[aantron]

Ah right, then constraint in Bisect's opam it is.

[aantron]

@undu I've pulled your commits into a PR (#331). I'll finish it off with some testing and updating the README. Thanks for getting most of the change done!

[undu]

Thanks for continuing the work! I feel bad I dropped the ball there.

[aantron]

Ok, this is now in master. I'll finish editing the README (needed to merge without it to break dependency cycles with outside example projects), then solve some of the outstanding bugs, and then release this in Bisect_ppx 2.5.0. Thanks again, @undu.

[hannesm]

@aantron (and @undu) thanks for your work on this. it would be great to get the dune support released to opam-repository, anything I can do to help getting there? I saw the README was updated with the new instructions...

[aantron]

@hannesm I just need to get around to fixing some of the other issues, and then do the release. It's definitely coming up :)

[aantron]

This is now in opam (and npm) in Bisect_ppx 2.5.0. I've updated all the starter repos and the README.