Use upstream coveragepy's new lcov support

[TLATER]

As explained in 8c3ea99, coveragepy recently added lcov support. This patch set intends to tackle #10660 (and bazelbuild/rules_python#43), and make python coverage fully usable out-of-the box. It builds on work by @ulfjack in #10433, but now with actual support from upstream.

This also incidentally fixes a bug where bazel test would no longer work if the PYTHON_COVERAGE environment variable was set (can report/fix separately if desired), as well as #14436 (this should perhaps be raised in a separate PR, but made much more sense after refactoring here).

See the commit messages for more detailed comments.

This is a draft, but I would like some review of the overall approach, and whether this is workable in this state. I'm raising this as a draft for these reasons:

• A commit to allow coveragepy resolution by label is included. It's not implemented correctly at the moment (see commit message), and I'd like feedback on how this could be achieved better.
• The comments regarding os.execv usage seem to have a strange history (see commit message), and I would like to figure out what happened here before continuing the legacy.
• More consideration for other coverage tools might be more important than I think.
• Testing for this is slim because it depends on coveragepy and I'm unsure how/if I should integrate that into CI.
• The python coverage documentation should be updated.

@c-mita and @lfpino may also be interested.

[TLATER]

Hm, interesting, I'll have to look into those test failures.

[TLATER]

The failing test is //src/test/shell/bazel:python_version_test, the culprit is an accidental f-string. New habits die hard, it seems.

[comius]

cc @rickeylev for Python

[adam-azarchs]

Not to distract from what this PR is trying to accomplish, but it's only part of the puzzle when it comes to hermetic python toolchains. In order to support those, we'd need to also add an attribute to py_runtime to supply a target for coveragepy, and I guess plumb that through to PYTHON_COVERAGE. That sort of thing is particularly important in a remote build scenario where the developer may have no control over whether there exists a system-wide installation of coverage on the remote builder.

[TLATER]

That sounds like a good idea. We were using rules_python to provide it instead, but it also currently lacks toolchain support (and did things rather more impurely than I'm comfortable with).

[rickeylev]

we'd need to also add an attribute to py_runtime to supply a target for coveragepy, and I guess plumb that through to PYTHON_COVERAGE.

I'm not too familiar with how coverage works, so forgive me: why does the py_runtime rule need a coverage attribute? That would imply that a particular runtime and coverage are intrinsically linked (e.g. a particular coverage implementation only works with a particular runtime, and vice versa). IIUC, coverage isn't so strictly tied to a runtime, so it seems more like something that would go on the toolchain or rules (which wire together the runtime with other parts).

[adam-azarchs]

py_runtime is how you set up the python toolchain. A python toolchain setup looks something like

py_runtime(
    name = "python3_runtime",
    files = [":python"],
    interpreter = ":python_interpreter_exe",
    python_version = "PY3",
)

py_runtime_pair(
    name = "python_runtime_pair",
    py3_runtime = ":python3_runtime",
)

toolchain(
    name = "python_toolchain",
    exec_compatible_with = ["@platforms//os:linux"],
    target_compatible_with = ["@platforms//os:linux"],
    toolchain = ":python_runtime_pair",
    toolchain_type = "@bazel_tools//tools/python:toolchain_type",
    visibility = ["//visibility:public"],
)

While one could just put coverage into the files attribute of py_runtime, making it a separate attribute would have a few advantages:

1. No need for the user to muck around with environment variables (which always seems a little but "un-bazely" to me).
2. Only depending on the coverage library when running with code coverage.
3. Being able to supply different coverage libraries to different runtimes, e.g. probably an older version for python 2, if anyone is still using that.

[rickeylev]

Ahhhh, huh. Thanks. I didn't realize py_runtime was, in effect, the toolchain (we don't have toolchains enabled for Python at Google, so this isn't an area I'm very familiar with). Well, that's kind of unfortunate naming, but yeah, putting the coverage settings on the toolchain could make sense. TBH, I don't fully understand the pros/cons of a toolchain over an implicit attribute of the rule, but both are pretty transparent to the user, so I'm not sure if it matters much.

[adam-azarchs]

The advantage of a toolchain is that it can be configured. An implicit attribute of the rule has to be a fixed target. In our use case we very specifically cannot use the automatically-configured python toolchain based on the system-wide installation, because that is usually not the right version of python.

[adam-azarchs]

Adding an attribute to py_runtime doesn't look like it would be that hard to do; I'd probably have already made a PR to do it except it's still in the java side of things, with which I have a lot less experience. But also, as I said earlier, it's kind of orthogonal to this PR which, as far as I understand it, would still be required in order to make things actually work.

[TLATER]

Superseded by @adam-azarchs ' #15590, thanks :)