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

[EricCousineau-TRI]

🚀 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: EricCousineau-TRI/repro@96e1336

[aignas]

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]

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]

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

[aignas]

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]

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:

rules_python/python/private/pypi/generate_whl_library_build_bazel.bzl

Lines 85 to 87 in ecad092

	# This makes this directory a top-level in the python import
	# search path for anything that depends on this.
	imports = ["site-packages"],

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]

very loose code: main...EricCousineau-TRI:rules_python:issue-2071

[aignas]

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: main...EricCousineau-TRI:rules_python:issue-2071 -- Reply to this email directly or view it on GitHub: #2071 (comment) You are receiving this because you commented. Message ID: ***@***.***>

[groodt]

Closing this issue as a duplicate of #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.