fix(--debugger): Ensure that imports or venv_site_package files are propagated for debugger target (#3483)
https://github.com/bazel-contrib/rules_python/commit/a94bd0fdde426bf30efed7c819422d74b404cc18
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: https://github.com/bazel-contrib/rules_python/issues/3481
---------
Co-authored-by: Shayan Hoshyari <hoshyari@adobe.com>
diff --git a/python/private/common.bzl b/python/private/common.bzl
index a593e97..c31aeb3 100644
--- a/python/private/common.bzl
+++ b/python/private/common.bzl
@@ -161,12 +161,8 @@
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 @@
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 @@
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
diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
index ea00eed..f9c9122 100644
--- a/python/private/py_executable.bzl
+++ b/python/private/py_executable.bzl
@@ -37,6 +37,7 @@
load(
":common.bzl",
"collect_cc_info",
+ "collect_deps",
"collect_imports",
"collect_runfiles",
"create_binary_semantics_struct",
@@ -293,7 +294,8 @@
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 @@
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 @@
# * 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 @@
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,9 +1023,6 @@
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,
@@ -1026,6 +1030,8 @@
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 @@
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)
diff --git a/tests/base_rules/py_executable_base_tests.bzl b/tests/base_rules/py_executable_base_tests.bzl
index 58251c6..2af5406 100644
--- a/tests/base_rules/py_executable_base_tests.bzl
+++ b/tests/base_rules/py_executable_base_tests.bzl
@@ -19,6 +19,7 @@
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_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 @@
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",
)
