fix(--debugger): Ensure that imports or venv_site_package files are propagated for debugger target (bazel-contrib#3483)

bazel-contrib@a94bd0f
recently added support for injecting dependencies for easier use of
debugger. It allows injecting deps via
`--@rules_python//python/config_settings:debugger=<target>`. While the
runfiles from `<target>` were inherited in the final binary, the
`imports` or `venv_site_packages` were missing. Hence making the
debugger target unusable for various corner cases (e.g. when it uses
`imports = ...` or when it is coming from pip hub and
`venv_site_packages` are on).

This PR fixes that, and extends the unit test to include this situation.

Fixes: bazel-contrib#3481

---------

Co-authored-by: Shayan Hoshyari <[email protected]>

Author: shayanhoshyari

python/private/common.bzl

@@ -161,12 +161,8 @@ def collect_cc_info(ctx, extra_deps = []):
     Returns:
         CcInfo provider of merged information.
     """
-    deps = ctx.attr.deps
-    if extra_deps:
-        deps = list(deps)
-        deps.extend(extra_deps)
     cc_infos = []
-    for dep in deps:
+    for dep in collect_deps(ctx, extra_deps):
         if CcInfo in dep:
             cc_infos.append(dep[CcInfo])
 
@@ -175,17 +171,19 @@ def collect_cc_info(ctx, extra_deps = []):
 
     return cc_common.merge_cc_infos(cc_infos = cc_infos)
 
-def collect_imports(ctx):
+def collect_imports(ctx, extra_deps = []):
     """Collect the direct and transitive `imports` strings.
 
     Args:
         ctx: {type}`ctx` the current target ctx
+        extra_deps: list of Target to also collect imports from.
 
     Returns:
         {type}`depset[str]` of import paths
     """
+
     transitive = []
-    for dep in ctx.attr.deps:
+    for dep in collect_deps(ctx, extra_deps):
         if PyInfo in dep:
             transitive.append(dep[PyInfo].imports)
         if BuiltinPyInfo != None and BuiltinPyInfo in dep:
@@ -479,3 +477,19 @@ def runfiles_root_path(ctx, short_path):
         return short_path[3:]
     else:
         return "{}/{}".format(ctx.workspace_name, short_path)
+
+def collect_deps(ctx, extra_deps = []):
+    """Collect the dependencies from the rule's context.
+
+    Args:
+        ctx: rule ctx
+        extra_deps: list of Target to also collect dependencies from.
+
+    Returns:
+        list of Target
+    """
+    deps = ctx.attr.deps
+    if extra_deps:
+        deps = list(deps)
+        deps.extend(extra_deps)
+    return deps

python/private/py_executable.bzl

@@ -37,6 +37,7 @@ load(":cc_helper.bzl", "cc_helper")
 load(
     ":common.bzl",
     "collect_cc_info",
+    "collect_deps",
     "collect_imports",
     "collect_runfiles",
     "create_binary_semantics_struct",
@@ -293,7 +294,8 @@ def _create_executable(
         runtime_details,
         cc_details,
         native_deps_details,
-        runfiles_details):
+        runfiles_details,
+        extra_deps):
     _ = is_test, cc_details, native_deps_details  # @unused
 
     is_windows = target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints)
@@ -323,6 +325,7 @@ def _create_executable(
             add_runfiles_root_to_sys_path = (
                 "1" if BootstrapImplFlag.get_value(ctx) == BootstrapImplFlag.SYSTEM_PYTHON else "0"
             ),
+            extra_deps = extra_deps,
         )
 
         stage2_bootstrap = _create_stage2_bootstrap(
@@ -486,7 +489,7 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
 # * https://snarky.ca/how-virtual-environments-work/
 # * https://github.com/python/cpython/blob/main/Modules/getpath.py
 # * https://github.com/python/cpython/blob/main/Lib/site.py
-def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root_to_sys_path):
+def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root_to_sys_path, extra_deps):
     create_full_venv = BootstrapImplFlag.get_value(ctx) == BootstrapImplFlag.SCRIPT
     venv = "_{}.venv".format(output_prefix.lstrip("_"))
 
@@ -587,7 +590,11 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root
         VenvSymlinkKind.BIN: bin_dir,
         VenvSymlinkKind.LIB: site_packages,
     }
-    venv_app_files = create_venv_app_files(ctx, ctx.attr.deps, venv_dir_map)
+    venv_app_files = create_venv_app_files(
+        ctx,
+        deps = collect_deps(ctx, extra_deps),
+        venv_dir_map = venv_dir_map,
+    )
 
     files_without_interpreter = [pth, site_init] + venv_app_files.venv_files
     if pyvenv_cfg:
@@ -1016,16 +1023,15 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment =
     default_outputs.add(precompile_result.keep_srcs)
     default_outputs.add(required_pyc_files)
 
-    imports = collect_imports(ctx)
-
-    runtime_details = _get_runtime_details(ctx)
     extra_deps = []
 
     # The debugger dependency should be prevented by select() config elsewhere,
     # but just to be safe, also guard against adding it to the output here.
     if not _is_tool_config(ctx):
         extra_deps.append(ctx.attr._debugger_flag)
 
+    imports = collect_imports(ctx, extra_deps = extra_deps)
+    runtime_details = _get_runtime_details(ctx)
     cc_details = _get_cc_details_for_binary(ctx, extra_deps = extra_deps)
     native_deps_details = _get_native_deps_details(
         ctx,
@@ -1058,6 +1064,7 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment =
         cc_details = cc_details,
         native_deps_details = native_deps_details,
         runfiles_details = runfiles_details,
+        extra_deps = extra_deps,
     )
     default_outputs.add(exec_result.extra_files_to_build)
 

tests/base_rules/py_executable_base_tests.bzl

@@ -19,6 +19,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
 load("@rules_testing//lib:truth.bzl", "matching")
 load("@rules_testing//lib:util.bzl", rt_util = "util")
 load("//python:py_executable_info.bzl", "PyExecutableInfo")
+load("//python:py_info.bzl", "PyInfo")
 load("//python:py_library.bzl", "py_library")
 load("//python/private:common_labels.bzl", "labels")  # buildifier: disable=bzl-visibility
 load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")  # buildifier: disable=bzl-visibility
@@ -172,9 +173,11 @@ def _test_executable_in_runfiles_impl(env, target):
     ])
 
 def _test_debugger(name, config):
+    # Using imports
     rt_util.helper_target(
         py_library,
         name = name + "_debugger",
+        imports = ["."],
         srcs = [rt_util.empty_file(name + "_debugger.py")],
     )
 
@@ -187,24 +190,70 @@ def _test_debugger(name, config):
             labels.DEBUGGER: "//{}:{}_debugger".format(native.package_name(), name),
         },
     )
+
+    # Using venv
+    rt_util.helper_target(
+        py_library,
+        name = name + "_debugger_venv",
+        imports = [native.package_name() + "/site-packages"],
+        experimental_venvs_site_packages = "@rules_python//python/config_settings:venvs_site_packages",
+        srcs = [rt_util.empty_file("site-packages/" + name + "_debugger_venv.py")],
+    )
+
+    rt_util.helper_target(
+        config.rule,
+        name = name + "_subject_venv",
+        srcs = [rt_util.empty_file(name + "_subject_venv.py")],
+        config_settings = {
+            # config_settings requires a fully qualified label
+            labels.DEBUGGER: "//{}:{}_debugger_venv".format(native.package_name(), name),
+        },
+    )
+
     analysis_test(
         name = name,
         impl = _test_debugger_impl,
         targets = {
             "exec_target": name + "_subject",
             "target": name + "_subject",
+            "target_venv": name + "_subject_venv",
         },
         attrs = {
             "exec_target": attr.label(cfg = "exec"),
         },
+        config_settings = {
+            labels.VENVS_SITE_PACKAGES: "yes",
+            labels.PYTHON_VERSION: "3.13",
+        },
     )
 
 _tests.append(_test_debugger)
 
 def _test_debugger_impl(env, targets):
+    # 1. Subject
+
+    # Check the file from debugger dep is injected.
     env.expect.that_target(targets.target).runfiles().contains_at_least([
         "{workspace}/{package}/{test_name}_debugger.py",
     ])
+
+    # #3481: Ensure imports are setup correcty.
+    meta = env.expect.meta.derive(format_str_kwargs = {"package": targets.target.label.package})
+    env.expect.that_target(targets.target).has_provider(PyInfo)
+    imports = targets.target[PyInfo].imports.to_list()
+    env.expect.that_collection(imports).contains(meta.format_str("{workspace}/{package}"))
+
+    # 2. Subject venv
+
+    # #3481: Ensure that venv site-packages is setup correctly, if the dependency is coming
+    # from pip integration.
+    env.expect.that_target(targets.target_venv).runfiles().contains_at_least([
+        "{workspace}/{package}/_{name}.venv/lib/python3.13/site-packages/{test_name}_debugger_venv.py",
+    ])
+
+    # 3. Subject exec
+
+    # Ensure that tools don't inherit debugger.
     env.expect.that_target(targets.exec_target).runfiles().not_contains(
         "{workspace}/{package}/{test_name}_debugger.py",
     )