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

An unusual issue arising from the manner in which rules_python manages namespace packages. #1886

Closed
Agent-Hellboy opened this issue May 7, 2024 · 5 comments

Comments

@Agent-Hellboy
Copy link

Agent-Hellboy commented May 7, 2024

🐞 bug report

Affected Rule

The issue is caused by the rule:

Is this a regression?

Yes, the previous version in which this bug was not present was: ....

Description

A clear and concise description of the problem...

Hi,

I'm relatively new to Bazel and have only started using it a few days ago. I've gone through some introductory documentation.

In rules_python, I've noticed that it adds init.py files to every namespace package. I assume this is done to convert Python packages into Bazel packages with a BUILD file. However, I'm puzzled as to why it adds init.py to namespace package.

Recently, I encountered an issue where an unnecessary init.py file was added. Specifically, it was added to python_local_deps_pyqt5/site-packages/PyQt5/uic/widget-plugins. I'm not sure why this was done, as it doesn't seem necessary.

PyQt5 dynamically loads these plugins at runtime using Python's exec API, which assumes a certain structure. Because of this, You can't simply add the init.py file without proper justification.

It seems that rules_python adds init.py files primarily to include a path variable. However, I believe rules_python should only add init.py files when converting a package into a Bazel package.

If you have any insights into why rules_python behaves this way or how I can resolve the issue with the unnecessary init.py file, I would greatly appreciate your input. (I guess only way is to avoid adding init.py or i need to patch the load_plugin method of pyqt5 library)

🔬 Minimal Reproduction

Install PyQt5 as an external dependency using rules_python. Then, attempt to load a .ui file using uic. This process will require loading a plugin, which may result in an error.

🔥 Exception or Error




 File "/var/tmp/proshan/c7b4a059b4e2be58/cpu/applications/bazel_output_base/execroot/cap/bazel-out/k8-opt-a3-bn_h10/bin/hwio/dma/scheduler/dma_scheduler.runfiles/python_local_deps_pyqt5/site-packages/PyQt5/uic/objcreator.py", line 158, in load_plugin
    raise WidgetPluginError("%s: %s" % (e.__class__, str(e)))
PyQt5.uic.exceptions.WidgetPluginError: : name '__path__' is not defined

🌍 Your Environment

Operating System:

  
Linux aws-devel-02 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
  

Output of bazel version:

  
bazel 6.1.1
  

Rules_python version:

  
rules_python-0.31.0
  

Anything else relevant?

@aignas
Copy link
Collaborator

aignas commented May 13, 2024

I wrote about this in https://anikevicius.lt/blog/python-lazy-loading-and-namespace-pkgs/ and I am not sure if there is a good way to solve this other than have a single folder where we install all of the packages as part of a build action.

I know that @mattem is working a lot in this direction within rules_py but I doubt that there are any plans to contribute any of that work to rules_python.

My initial thinking would be to start looking into ways to make uv useful within rules_python for including the third party libraries, but I am not sure I'll have time in the nearest future to explore this here.

@rickeylev
Copy link
Collaborator

The adding of __init__.py files is controlled by a flag and target-level setting. I forget their name atm. In any case, the basic way it works is a binary looks at all the files in runfiles, and adds any missing init.py files to directories containing .py or .so files.

Yes, this behavior is annoying and causes issues like the one described. We'd like to change this behavior, but it's a fairly large breaking change (the global flag is a large hammer that will work; the binary-level attribute works until another binary triggers the behavior), and haven't come up with a more incremental strategy.

I assume this is done to convert Python packages into Bazel packages with a BUILD file

Sort of. It's mostly from how Bazel works internally at Google (adding empty files to every directory is tedious and gets mixed reception), and related to how to properly model __init__.py files given Bazel's dependency model (they cause circular dependencies / heavy weight dependencies)

@gedalia
Copy link

gedalia commented Aug 9, 2024

I've hit something like this, using nvidia's omniverse and isaac sim libraries, enable_implicit_namespace_pkgs and seems to break the package. I've also found that I need to explicitly move the unpacked packages based on the contents of top_level.txt, and then mess with the sandbox. I'm on an older version of rules_python but I suspect things wouldn't be much different on a newer one.

@Agent-Hellboy
Copy link
Author

Agent-Hellboy commented Aug 12, 2024

@gedalia I patched the method of the dependent library to avoid looking at the init.py, it's because of extra init.py , ignore it. try looking for the flag that controls adding the init.py
The adding of __init__.py files is controlled by a flag and target-level setting

@groodt
Copy link
Collaborator

groodt commented Aug 24, 2024

I'm going to close this issue since it appears that the original poster has a workaround or solution.

For posterity, the bazel flag that @rickeylev mentions is: --incompatible_default_to_explicit_init_py which controls the creation of empty init.py files. See bazelbuild/bazel#7386

Given that this issue relates to a third-party distribution package (PyQT5), it might actually be caused by the treatment that the rules_python whl_installer does to convert namespace packages to pkgutil namespaces. See https://github.com/bazelbuild/rules_python/blob/main/python/private/pypi/whl_installer/namespace_pkgs.py#L74

I think we might find that there are fewer issues with namespace packages if a site-packages layout is adopted by rules_python #2156

@groodt groodt closed this as completed Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants