bazel-contrib / bazel-contrib/rules_python
Using external / imported python extension module DSOs with py_test, py_binary
- Dominant language
- Starlark
- Stars
- 688
- Forks
- 721
- Avg merge
- 14h 49m
- Merged PRs (30d)
- 81
Description
# 🚀 feature request
### Relevant Rules
This describes a modifcation to the `py_executable`-based rules and intersects with
- `py_test`
- `py_binary`
- `py_library`
- `cc_import` / `cc_library`
### Description
When a `py_test` depends on a python extension module that was not built within bazel, the `DT_NEEDED` entries cannot be resolved in a hermetic environment because the imported extension module (DSO) will be missing or have an incorrect runpath / rpath.
I believe this is substantially different from #2562 such that it warrants its own feature request.
I've provided a minimal test case here: https://github.com/wade-arista/bazel-demo/tree/min-py-ext-case#overview where it _simulates_ the external dynamic libs by using `chrpath` to remove RPATH and then re-imports the built libs. I also brought it up [on slack](https://bazelbuild.slack.com/archives/CA306CEV6/p1737601588845769).
### Describe the solution you'd like
Given some new `py_xxx` argument or `--@rules_python//python/config_settings` flag, we would inspect `cc_info.linking_context.linker_inputs.to_list()` from inside [`_create_run_environment_info()`](https://github.com/bazelbuild/rules_python/blob/466da1d9710289bfb01061b9be7bb124132996e0/python/private/py_executable.bzl#L1707) and inject / extend `LD_LIBRARY_PATH` in the `RunEnvironmentInfo.environment` with the path(s) to the dependent dynamic libraries (see the "Sample Patch" below for details).
Sample Patch
```python
diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
index da7127e0..1247b445 100644
--- a/python/private/py_executable.bzl
+++ b/python/private/py_executable.bzl
@@ -1628,7 +1628,7 @@ def _create_providers(
),
),
create_instrumented_files_info(ctx),
- _create_run_environment_info(ctx, inherited_environment),
+ _create_run_environment_info(ctx, inherited_environment, cc_info),
PyExecutableInfo(
main = main_py,
runfiles_without_exe = runfiles_details.runfiles_without_exe,
@@ -1701,8 +1701,9 @@ def _create_providers(
providers.extend(extra_providers)
return providers
-def _create_run_environment_info(ctx, inherited_environment):
+def _create_run_environment_info(ctx, inherited_environment, cc_info):
expanded_env = {}
+ ld_library_path = {}
for key, value in ctx.attr.env.items():
expanded_env[key] = _py_builtins.expand_location_and_make_variables(
ctx = ctx,
@@ -1710,6 +1711,21 @@ def _create_run_environment_info(ctx, inherited_environment):
expression = value,
targets = ctx.attr.data,
)
+
+ for inp in cc_info.linking_context.linker_inputs.to_list():
+ for lib in inp.libraries:
+ dl = lib.dynamic_library
+ if not dl:
+ continue
+ lib_path = dl.short_path.removesuffix(dl.basename).rstrip("/")
+ ld_library_path[lib_path] = True
+
+ if ld_library_path:
+ parts = []
+ parts += [expanded_env["LD_LIBRARY_PATH"]] if "LD_LIBRARY_PATH" in expanded_env else []
+ parts += sorted(ld_library_path.keys())
+ expanded_env["LD_LIBRARY_PATH"] = ":".join(parts)
+
return RunEnvironmentInfo(
environment = expanded_env,
inherited_environment = inherited_environment,
```
### Describe alternatives you've considered
1. I think this would also be doable using a wrapper-generating rule, but it's not super clear to me how well it would interact with the existing sh / python wrapper script for py executable rules.
2. Instead of setting `RunEnvironmentInfo.environment`, this could be added to the existing wrapper (shell or python depending on `bootstrap_impl`).
Contributor guide
Assessment
This issue has not been assessed yet.