bazelbuild / rules_python

Bazel Python Rules
https://rules-python.readthedocs.io
Apache License 2.0
517 stars 531 forks source link

`pip_parse()` does not correctly read `*.pth` files? #2071

Closed EricCousineau-TRI closed 1 week ago

EricCousineau-TRI commented 1 month ago

🚀 feature request

Relevant Rules

package_annotation() from pip.bzl

Description

From docs, as of time of writing, you can add BUILD content, or add / remove files, but doesn't seem like you can change py_library(deps, imports).

For example, rerun.io is an awesome-looking package. But it doesn't work when using using bazel + rules_python @ 0.34.0: https://github.com/EricCousineau-TRI/repro/tree/e084a7434286cf426a71350f79755c7cbab6d6eb/bazel/rules_python_rerun Seems like either the wheel doesn't declare things s.t. rules_python adds ./site-packages/rerun_sdk to the Python path (it only adds ./site-packages).

Describe the solution you'd like

For now, would be nice to have something like

pip_parse(
    ...
    annotations = {
        "rerun-sdk": package_annotation(
            extra_imports = ["site-packages/rerun_sdk"],
        ),
    },
)

Describe alternatives you've considered

For now, will try package_annotation(..., additive_build_content), and include that whenever I use pip("rerun-sdk").

EDIT: This workaround worked: https://github.com/EricCousineau-TRI/repro/commit/96e13366e9cef60bc4c8991b1c65f1d7fbbea71d

aignas commented 1 month ago

imports is not supported, but deps is supported via the whl patching mechanism. You should just patch the whl METADATA file that defines the dependencies and then our code generation will do the right thing. Why do you need to modify imports?

EricCousineau-TRI commented 1 month ago

in this case, it's missing site-packages/rerun_sdk in the PYTHONPATH: https://github.com/EricCousineau-TRI/repro/tree/e084a7434286cf426a71350f79755c7cbab6d6eb/bazel/rules_python_rerun it's not an inter-package dependency, but rather a missing dir on path within the whl itself.

perhaps it's because there's a .pth file that rules_python is not parsing? from above linked repro

$ cd $(bazel info output_base)
$ cat external/pip_deps_rerun_sdk/site-packages/rerun_sdk.pth
rerun_sdk

from docs https://docs.python.org/3/library/site.html it seems like the .pth file should effectively change sys.path (or in this case, imports)?

EricCousineau-TRI commented 1 month ago

(if this seems like a valid thing, I can ~file a new issue~ edit this issue and reword the title)

aignas commented 1 month ago

Thanks for the repro and the logs, this makes it easier to understand. The rules_python not handling the .pth file could be something that is the problem here.

I think ideally the solution would be to investigate how to make rules_python respect the .pth files, but I am not sure how to do it yet. Supporting imports or deps via the annotations does not seem like the right solution here, because it increases the API surface and still forces all of the users to apply workarounds.

EricCousineau-TRI commented 1 month ago

sweet, welcome

I think simplest way is to assume it's non-executable *.pth (maybe check for import sys as simple check) and just have code populate imports: https://github.com/bazelbuild/rules_python/blob/ecad092ffa97ac236fb9a6b33ff7f5af4af80eb6/python/private/pypi/generate_whl_library_build_bazel.bzl#L85-L87

would probably need something like parsed_pth_files as argument to that function, and then the repository rule has to cat those contents accordingly.

EricCousineau-TRI commented 1 month ago

very loose code: https://github.com/bazelbuild/rules_python/compare/main...EricCousineau-TRI:rules_python:issue-2071

aignas commented 1 month ago

This looks like it could work, having such code in whl_library could be a good idea.

If you would like to finish this and submit a PR, feel free to do it. I think the main idea of just reading the pth file in starlark and then appending the imports would be a nice way to do it.

On 20 July 2024 03:01:02 GMT+09:00, Eric Cousineau @.***> wrote:

very loose code: https://github.com/bazelbuild/rules_python/compare/main...EricCousineau-TRI:rules_python:issue-2071

-- Reply to this email directly or view it on GitHub: https://github.com/bazelbuild/rules_python/issues/2071#issuecomment-2239814193 You are receiving this because you commented.

Message ID: @.***>

groodt commented 1 week ago

Closing this issue as a duplicate of https://github.com/bazelbuild/rules_python/issues/2156

Any PR to workaround .pth is welcome, but the PR I've linked above is a more holistic issue to capture the overall friction that rules_python causes for some packages.