diff --git a/python/private/common.bzl b/python/private/common.bzl index a593e97558..c31aeb383f 100644 --- a/python/private/common.bzl +++ b/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 diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index ea00eed17b..f9c91225b4 100644 --- a/python/private/py_executable.bzl +++ b/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,9 +1023,6 @@ 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, @@ -1026,6 +1030,8 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = 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) diff --git a/tests/base_rules/py_executable_base_tests.bzl b/tests/base_rules/py_executable_base_tests.bzl index 58251c60a0..2af5406ced 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: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", )