Instrumented binaries with ppx_bisect do not produce coverage files when ran through cram

[undu]

Bisect_ppx recently implemented the instrumentation stanzas present in dune 2.7+, it doesn't seem to work properly when used along cram tests.

When cram tests run executables instrumented with bisect_ppx, no coverage found is found in the directory tree of the project.

When the binaries are ran using dune exec or through a rule, the coverage files are produced as expected.

I've prepared a small repository that reproduced the issue: https://gitlab.com/unduthegun/cram-coverage

@aantron is aware of the issue and suspects a dune might be deleting the coverage files somehow, but this doesn't happen then there's a rule executing the binaries.

[nojb]

This is because cram rules use sandboxing unconditionally which results in the generated coverage files being lost after execution. Am not sure if there is a technical reason why sandboxing is required or if this could be relaxed. cc @rgrinberg

[rgrinberg]

@nojb on a related note, do you think we could extend the instrumentation mechanism to make it aware of the targets bisect is producing? For example, we could a field like (targets %{module_name}.bisect).

@aalekseyev this seems like a general usability issue with sandboxing. Perhaps there should be a way to specify that you'd like to pull some targets out of the sandbox back to where the user expects them to be.

[aalekseyev]

Yeah, indeed.

[nojb]

@nojb on a related note, do you think we could extend the instrumentation mechanism to make it aware of the targets bisect is producing? For example, we could a field like (targets %{module_name}.bisect).

One problem currently is that the generated files are not naturally associated to modules or libraries or ... rather the instrumented executable generates one or more monolithic files which contain the bisection information for everything that went into the executable (and had instrumentation turned on). Also the filename has some random component.

If the filenames of the generated files were more canonical, maybe the user could add them as explicit targets of the relevant rules; it is not very clever, but perhaps it wouldn't be too bad either...

[aantron]

bisection

Coverage* :) Bisect is already confusingly named, we don't want to spread the name around :)

Is there a place in _build or elsewhere these files can go? The project root is also fine. That was typical in Ocamlbuild days, and was often the case with Dune back when test cases were sometimes run without dune runtest. Does dune runtest/dune exec set any Dune-specific environment variables the Bisect-generated runtime could use to find such a place?

[nojb]

Is there a place in _build or elsewhere these files can go? The project root is also fine. That was typical in Ocamlbuild days, and was often the case with Dune back when test cases were sometimes run without dune runtest. Does dune runtest/dune exec set any Dune-specific environment variables the Bisect-generated runtime could use to find such a place?

One approach is to use the "cookie" mechanism, eg by adding the following to the definition of the ppx:

(kind (ppx_rewriter (cookies (destdir %{env:COVERAGE_DEST_DIR=}))))

The ppx could use the value of the destdir cookie to know where to put the generated files (but note that this is evaluated at compilation-time).

The user of the ppx would specify the environment variable to tell the ppx where to put the generated files.

The ppx could use the environment variable directly, but doing it this way allows dune to keep track of the dependency on the value environment variable, so that the ppx would be recompiled if it changes.

[aantron]

I would prefer an approach that doesn't ask the user to define an environment variable for ordinary usage. Ideally Dune and Bisect would "talk" to each other somehow (or follow some conventions).

[rgrinberg]

The ppx could use the value of the destdir cookie to know where to put the generated files (but note that this is evaluated at compilation-time).

I'm not sure it's a good idea to let ppx rewriters break our "sandboxing".

Perhaps we should discuss this at the dev meeting. I don't have any alternatives to suggest right now unfortunately.

[aantron]

I agree with @rgrinberg. The .coverage files are more like output rather than build targets (i.e. they are more like writing to STDERR, for example, which a program run by dune exec is allowed to do without messing with the sandbox). We just need somewhere for these files to go, that Bisect and Dune can in some way agree on.

[aantron]

I guess one immediate workaround I could add on the Bisect side is to look in the ancestor paths of the working directory for a dune-project file, and write .coverage files there. But this seems somewhat hacky and fragile, so I'll wait for now.

[aantron]

I just converted Bisect_ppx itself to use Dune cram tests, and it is generating coverage files in that repo's usage:

dune build --force --instrument-with meta_bisect_ppx @runtest

[undu]

I've tried that command in gitlab.com/unduthegun/cram-coverage but doesn't generate the coverage files in the directory. stracing it I can see the coverage file being deleted by dune:

lstat("_build/.sandbox/8f5c22cea72c88afeea454979b9a7fb9/default/hello.t", {st_mode=S_IFLNK|0777, st_size=24, ...}) = 0
unlink("_build/.sandbox/8f5c22cea72c88afeea454979b9a7fb9/default/hello.t") = 0
lstat("_build/.sandbox/8f5c22cea72c88afeea454979b9a7fb9/default/hello.exe", {st_mode=S_IFLNK|0777, st_size=26, ...}) = 0
unlink("_build/.sandbox/8f5c22cea72c88afeea454979b9a7fb9/default/hello.exe") = 0
lstat("_build/.sandbox/8f5c22cea72c88afeea454979b9a7fb9/default/bisect396080174.coverage", {st_mode=S_IFREG|0644, st_size=55, ...}) = 0
unlink("_build/.sandbox/8f5c22cea72c88afeea454979b9a7fb9/default/bisect396080174.coverage") = 0
rmdir("_build/.sandbox/8f5c22cea72c88afeea454979b9a7fb9/default") = 0

[aantron]

I just had need for this myself. I was able to get coverage files output into the root directory of the project using the BISECT_FILE workaround:

BISECT_FILE=`pwd`/bisect dune runtest --force --instrument-with bisect_ppx

[nojb]

The issue seems to have been worked around, and there isn't a clear alternative that would not break sandboxing, so it looks like the issue could be closed.

[nojb]

I am going to close it but we can reopen (or open a new issue) if needed.