fix: make coverage work with bootstrap=script (#2574)

The script based bootstrap wasn't expanding the coverage template
variable, which prevented
coverage from activating. This was introduced when it was switched to
the venv layout.

To fix, expand the `%coverage_tool%` template variable as done
elsewhere.

Tested manually, per repro instructions in #2572. While I did devise a
way to mostly
test this without an integration test, it was thwarted by some other
bugs.

Along the way, improve some of the bootstrap debug output and fix a
comment.

Fixes #2572

CHANGELOG.md

@@ -65,6 +65,8 @@ Unreleased changes template.
   fixes [#2554](https://github.com/bazelbuild/rules_python/issues/2554).
 * (runfiles) Runfile manifest and repository mapping files are now interpreted
   as UTF-8 on all platforms.
+* (coverage) Coverage with `--bootstrap_impl=script` is fixed
+  ([#2572](https://github.com/bazelbuild/rules_python/issues/2572)).
 
 {#v0-0-0-added}
 ### Added

python/private/py_executable.bzl

@@ -561,6 +561,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
         template = runtime.site_init_template,
         output = site_init,
         substitutions = {
+            "%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime),
             "%import_all%": "True" if ctx.fragments.bazel_py.python_import_all_repositories else "False",
             "%site_init_runfiles_path%": "{}/{}".format(ctx.workspace_name, site_init.short_path),
             "%workspace_name%": ctx.workspace_name,
@@ -578,6 +579,17 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
 def _map_each_identity(v):
     return v
 
+def _get_coverage_tool_runfiles_path(ctx, runtime):
+    if (ctx.configuration.coverage_enabled and
+        runtime and
+        runtime.coverage_tool):
+        return "{}/{}".format(
+            ctx.workspace_name,
+            runtime.coverage_tool.short_path,
+        )
+    else:
+        return ""
+
 def _create_stage2_bootstrap(
         ctx,
         *,
@@ -593,23 +605,14 @@ def _create_stage2_bootstrap(
         sibling = output_sibling,
     )
     runtime = runtime_details.effective_runtime
-    if (ctx.configuration.coverage_enabled and
-        runtime and
-        runtime.coverage_tool):
-        coverage_tool_runfiles_path = "{}/{}".format(
-            ctx.workspace_name,
-            runtime.coverage_tool.short_path,
-        )
-    else:
-        coverage_tool_runfiles_path = ""
 
     template = runtime.stage2_bootstrap_template
 
     ctx.actions.expand_template(
         template = template,
         output = output,
         substitutions = {
-            "%coverage_tool%": coverage_tool_runfiles_path,
+            "%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime),
             "%import_all%": "True" if ctx.fragments.bazel_py.python_import_all_repositories else "False",
             "%imports%": ":".join(imports.to_list()),
             "%main%": "{}/{}".format(ctx.workspace_name, main_py.short_path),

python/private/site_init_template.py

@@ -163,7 +163,7 @@ def _maybe_add_path(path):
         if cov_tool:
             _print_verbose_coverage(f"Using toolchain coverage_tool {cov_tool}")
         elif cov_tool := os.environ.get("PYTHON_COVERAGE"):
-            _print_verbose_coverage(f"PYTHON_COVERAGE: {cov_tool}")
+            _print_verbose_coverage(f"Using env var coverage: PYTHON_COVERAGE={cov_tool}")
 
         if cov_tool:
             if os.path.isabs(cov_tool):
@@ -185,7 +185,7 @@ def _maybe_add_path(path):
             coverage_setup = True
         else:
             _print_verbose_coverage(
-                "Coverage was enabled, but python coverage tool was not configured."
+                "Coverage was enabled, but the coverage tool was not found or valid. "
                 + "To enable coverage, consult the docs at "
                 + "https://rules-python.readthedocs.io/en/latest/coverage.html"
             )
@@ -194,3 +194,4 @@ def _maybe_add_path(path):
 
 
 COVERAGE_SETUP = _setup_sys_path()
+_print_verbose("DONE")

python/private/stage2_bootstrap_template.py

@@ -106,8 +106,8 @@ def print_verbose(*args, mapping=None, values=None):
 
 def print_verbose_coverage(*args):
     """Print output if VERBOSE_COVERAGE is non-empty in the environment."""
-    if os.environ.get("VERBOSE_COVERAGE"):
-        print(*args, file=sys.stderr, flush=True)
+    if is_verbose_coverage():
+        print("bootstrap: stage 2: coverage:", *args, file=sys.stderr, flush=True)
 
 
 def is_verbose_coverage():
@@ -271,6 +271,7 @@ def _run_py(main_filename, *, args, cwd=None):
 
 @contextlib.contextmanager
 def _maybe_collect_coverage(enable):
+    print_verbose_coverage("enabled:", enable)
     if not enable:
         yield
         return
@@ -283,7 +284,9 @@ def _maybe_collect_coverage(enable):
     unique_id = uuid.uuid4()
 
     # We need for coveragepy to use relative paths.  This can only be configured
+    # using an rc file.
     rcfile_name = os.path.join(coverage_dir, ".coveragerc_{}".format(unique_id))
+    print_verbose_coverage("coveragerc file:", rcfile_name)
     with open(rcfile_name, "w") as rcfile:
         rcfile.write(
             """[run]
@@ -318,6 +321,7 @@ def _maybe_collect_coverage(enable):
         finally:
             cov.stop()
             lcov_path = os.path.join(coverage_dir, "pylcov.dat")
+            print_verbose_coverage("generating lcov from:", lcov_path)
             cov.lcov_report(
                 outfile=lcov_path,
                 # Ignore errors because sometimes instrumented files aren't