infinite module recursion when trying to add black wrapper script

[bsarden]

Good morning,

I'm trying to use rules_python_external and define black==19.10b0 as a dependency. There doesn't seem to be support in bazel for handling wheels that rely on entry_point definitions for their CLI interface, so I created my own. However, when trying to call off to patched_main it seems as though the import tree for black is incorrect.

BUILD file

py_binary(
    name = "black",
    srcs = ["black.py"],
    deps = [
        requirement("black"),
    ],
)

black.py

import black

if __name__ == "__main__":
    # print(dir(black.black.black))
    black.patched_main()

the black module seems to be the only importable attribute in the py_library.

$ bazel run //tools:black
INFO: Analyzed target //tools:black (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //tools:black up-to-date:
  bazel-bin/tools/black
INFO: Elapsed time: 0.144s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__', 'black']
Traceback (most recent call last):
  File "/tmp/bazel/bryce/execroot/com_github_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/com_github_foobar/tools/black.py", line 10, in <module>
    black.patched_main()
AttributeError: module 'black' has no attribute 'patched_main'

[dillon-giacoppo]

It may be a collision on your PYTHONPATH if both the patch and the library are called black, are you able to print out the variable to confirm.

If the entry_point is defined in the wheel's metadata it may be simple to add this support natively, such that importing an executable generates a py_binary and a py_library.

[bsarden]

Here's the contents of my $PYTHONPATH when running bazel run //tools:black:

/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__appdirs
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__attrs
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__click
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__pathspec
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__regex
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__toml
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__typed_ast
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps/pypi__black
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/com_github_brycearden_foobar
/tmp/bazel/bryce/execroot/com_github_brycearden_foobar/bazel-out/k8-fastbuild/bin/tools/black.runfiles/py_deps

I'm actually not able to print out the path, since it seems like the the entry_point script is not being created and the black module just contains more black modules.

i.e. import black.black.black successfully imports...

It's worth noting that I'm on bazel version 2.0.0 with --incompatible_strict_action_env=true (although that doesn't seem to affect this behavior)

[thundergolfer]

This should be easy enough to reproduce on my end, so I'll give it a shot and see what happens.

[dillon-giacoppo]

So there are two parts to this, one is whether we can automatically add a binary shim for entry points as described in the ticket above. The other is why the custom shim fails.

I have managed to get the example working with:

py_binary(
    name = "black",
    srcs = ["main.py"],
    main = "main.py",
    deps = [
        requirement("black"),
    ]
)

I believe the issue only exists when the wrapper script is called black.py, as Bazel is automatically adding the directory of the script to the path. This is not something I think is easily solvable in the packaging script themselves.

It seems that this is a reoccurrence of bazelbuild/bazel#9239. The workspace directory should not be on sys.path and yet is. This is reproducible on my machine by having a trivial py_binary containing print('\n'.join(sys.path)). I get:

/Users/dillon/src/personal/py-pkg-test
/private/var/tmp/_bazel_dillon/6299a8ba042968fb492c5b6fcad12cc5/execroot/foo_workspace/bazel-out/darwin-fastbuild/bin/main.runfiles
/private/var/tmp/_bazel_dillon/6299a8ba042968fb492c5b6fcad12cc5/execroot/foo_workspace/bazel-out/darwin-fastbuild/bin/main.runfiles/bazel_tools
/private/var/tmp/_bazel_dillon/6299a8ba042968fb492c5b6fcad12cc5/execroot/foo_workspace/bazel-out/darwin-fastbuild/bin/main.runfiles/foo_workspace
/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python37.zip
/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7
/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/lib-dynload
/Users/dillon/Library/Python/3.7/lib/python/site-packages
/usr/local/lib/python3.7/site-packages
/usr/local/Cellar/protobuf/3.11.2/libexec/lib/python3.7/site-packages

Interestingly PYTHONPATH variable from os.environ does not show the current workspace directory.

[thundergolfer]

Interestingly PYTHONPATH variable from os.environ does not show the current working directory.

That is interesting. So PYTHONPATH is only a subset of what's in sys.path. This has the details https://docs.python.org/3/library/sys.html#sys.path

[dillon-giacoppo]

It seems to be an open issue bazelbuild/bazel#7091.

The ticket above (bazelbuild/bazel#9239) fixes the sys.path specifically for the Bazel runfiles, it doesn't look as if the fix gets propagated down to the users scripts. bazelbuild/bazel#7091 (comment):

The wrapper script that was updated does indeed prune sys.path[0], however we then do a subprocess.call/os.execv (depending on the OS) which re-invokes Python which in turn re-does the logic to populate the path and adds our directory to sys.path[0], so that does still remain on the path and by-default break us out of the sandbox

Should note, even if this gets fixed, there would still be a naming collision on black due to

black.runfiles/py_deps/pypi__black
black.runfiles/foo_workspace

both being on the path. However, since external dependencies get listed first it would appear to work.

I think that the correct solution here is to rename the wrapper file. Work to auto-generate the wrapper will be tracked in #18.

I'll close this off for now since it is tracked upstream.