aspect-build / rules_py

More compatible Bazel rules for running Python tools and building Python projects
Apache License 2.0
84 stars 23 forks source link

[Bug]: Unexpected entries in `*.venv.pth` in the generated venv? #360

Closed tgeng closed 2 months ago

tgeng commented 3 months ago

What happened?

Consider the following file tree and content

> tree              
.
├── bar
│   ├── BUILD
│   └── my_bar.py
└── foo
    ├── BUILD
    └── my_foo.py

> cat bar/BUILD 
load("@aspect_rules_py//py:defs.bzl", aspect_py_binary = "py_binary")

py_binary(
    name = "my_bar",
    srcs = ["my_bar.py"],
    deps = [
        "//tgeng_test/foo:my_foo",
    ],
)

aspect_py_binary(
    name = "aspect_my_bar",
    srcs = ["my_bar.py"],
    deps = [
        "//tgeng_test/foo:aspect_my_foo",
    ],
)

> cat foo/BUILD 
load("@aspect_rules_py//py:defs.bzl", aspect_py_library = "py_library")

package(
    default_visibility = ["//visibility:public"],
)

py_library(
    name = "my_foo",
    srcs = ["my_foo.py"],
)

aspect_py_library(
    name = "aspect_my_foo",
    srcs = ["my_foo.py"],
)

> cat foo/my_foo.py 
def my_foo_fun():
    print("my_foo_fun")                                                                                                                                                                                                                                                                                                                                                          

> cat bar/my_bar.py 
from my_foo import my_foo_fun

my_foo_fun()

Then you can see different results between native py_binary and aspect's py_binary: official py_binary fails since my_foo is not qualified and hence cannot be found. Yet with aspect_py_binary, my_foo is found.

> bazel run //tgeng_test/bar:my_bar
Traceback (most recent call last):
  File "/private/var/tmp/_bazel_tgeng/150c7d194ffdb03b97b1f29c98a198fa/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/tgeng_test/bar/my_bar.runfiles/_main/tgeng_test/bar/my_bar.py", line 1, in <module>
    from my_foo import my_foo_fun
ModuleNotFoundError: No module named 'my_foo'

> bazel run //tgeng_test/bar:aspect_my_bar
my_foo_fun

Looking into the generated venv, I see the following.

> cat bazel-bin/tgeng_test/bar/aspect_my_bar.runfiles/.aspect_my_bar.venv/lib/python3.10/site-packages/aspect_my_bar.venv.pth 
../../../..
../../../../_main/tgeng_test/foo
../../../../_main
../../../../_main/tgeng_test/bar

But I think the ../../../../_main/tgeng_test/foo should not be there.

Version

Development (host) and target OS/architectures:

Output of bazel --version:

> bazel version                    
Bazelisk version: development
Build label: 7.1.0
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Mon Mar 11 17:55:51 2024 (1710179751)
Build timestamp: 1710179751
Build timestamp as int: 1710179751

Version of the Aspect rules, or other relevant rules from your WORKSPACE or MODULE.bazel file:

0.7.3

Language(s) and/or frameworks involved:

How to reproduce

No response

Any other information?

No response

mattem commented 3 months ago

I think this is due to the Aspect py_library adding imports = ["."] by default. Does setting imports = [] on //foo:my_foo resolve this?

tgeng commented 2 months ago

Yep I can confirm passing imports = [] fixed it. Thanks!

Can imports = [] be set as the default then? I assume more people trying to adopt rules_py will encounter similar issues. Also adding every directory to PYTHONPATH seems to be a questionable default behavior.

mattem commented 2 months ago

Yes, this diff set the default to []. https://github.com/aspect-build/rules_py/pull/367