Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

py_library imports ignored when linking into pex_binary #36

Open
evanj opened this issue May 28, 2017 · 8 comments
Open

py_library imports ignored when linking into pex_binary #36

evanj opened this issue May 28, 2017 · 8 comments

Comments

@evanj
Copy link
Contributor

evanj commented May 28, 2017

TL;DR: If I have a py_library(..., srcs=["foo.py"], imports=["."]), then with the standard Bazel Python rules I can use import foo to reference it. If I link this py_library into a pex_binary, it ignores the imports part, so the files get packed into the pex as full/workspace/path/foo.py, instead of as foo.py in the root of the pex.

Expected result: I should be able to change standard Bazel py_binary into a pex_binary and have it magically work. This probably means shuffling files around, or packaging each py_library into a wheel, and packaging those together into the pex?

Additional details

I have a Python mono-repo that is set up something like this:

  • package_foo/setup.py - A "standard" setuptools setup.py to build a wheel
  • package_foo/foo/__init__.py - Python package foo
  • somebinary/bin.py - A Python program that uses import foo

(Imagine more packages and binaries)

Each subdirectory is a "standard" Python package. We are still evaluating the various monorepo build tools (Pants, Buck, Bazel), so this lets us make it work fairly easily. I'd like to just "drop in" the appropriate BUILD rules, but I can't figure out how to make pex_binary work in this case, since I need to "fix" the import path somehow.

PS. Thanks for creating these rules! Being able to directly link wheels is great.

@evanj
Copy link
Contributor Author

evanj commented Jun 17, 2017

Turns out this is currently impossible due to bazelbuild/bazel#2617 ; Bazel does not expose the imports attribute. As a result, it is impossible to write Skylark rules to do this.

@benley
Copy link
Owner

benley commented Jun 17, 2017

Aye, it's true. Sorry I forgot to say as much when you opened this issue, because I spent a while figuring it out last fall. Let's keep this issue open because I'm pretty sure bazelbuild/bazel#2791 will be merged eventually, and then it can actually work :-)

@benley
Copy link
Owner

benley commented Jun 17, 2017

#35 should fix this when bazel's support is ready

@evanj
Copy link
Contributor Author

evanj commented Dec 11, 2017

In the meantime, since this still hasn't landed, I've implemented a pythonroot attribute that lets me say "subdirectory //other_python_root" is a "root" of the Python import tree. I then rewrite the pex manifest to make that work. If there is interest: I'd be happy to contribute a patch back to this project.

@c4urself
Copy link

c4urself commented Jan 8, 2018

@evanj -- I'd definitely be interested in a patch but looks like it'd be similar to (#42)? I have a similar project (large-ish python monorepo) that I'd like to use Bazel with. Would be really interested in hearing your findings compared to Buck/Pants.

@evanj
Copy link
Contributor Author

evanj commented Jan 9, 2018

Yes, it is very similar, although my version adds a pythonroot attribute to pex_library to be able to move any code around in the bazel workspace, without consuming tests/binaries needing to be aware.

As far as using Bazel entirely: The jury is still out, but so far: we've been able to get most of our stuff working. It definitely is reliable: if you can get it to work on your machine, it seems to work on everyone's machine, which is really the issue we wanted to solve. It also allows us to easily express complex dependencies between parts of the code base (which we also wanted to enable). The one caveat: if you consume very large third-party libraries (e.g. pandas, which depends on scipy and numpy), rebuilding any pex rules is really slow. I'm testing a hack to make this slightly better.

My take: Bazel's Python support is pretty bad at the moment, but I see a heck of a lot more "momentum" behind Bazel (eg. Dropbox, Stripe, others) so we are giving it a shot.

@c4urself
Copy link

c4urself commented Jan 9, 2018

@evanj would you be able to share your patch?

evanj pushed a commit to evanj/bazel_rules_pex that referenced this issue Jan 10, 2018
This is a workaround since the imports attribute is not available to
Skylark rules. See discussion:

benley#36
benley#42
@evanj
Copy link
Contributor Author

evanj commented Jan 10, 2018

@c4urself Sorry for the delay. See the branch at https://github.com/evanj/bazel_rules_pex/tree/pythonroot . I did not send a pull request for this, since it isn't totally clear to me if this is the "right" design for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants