-
-
Notifications
You must be signed in to change notification settings - Fork 645
refactor: optimize venv building for namespace packages #3454
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
refactor: optimize venv building for namespace packages #3454
Conversation
|
Ah drat. The regex_match function is in Bazel 7, but not exposed. Well, no biggie, we can just check for |
|
There we go, fixed. |
|
Can't merge this yet; need to check on a bug it might introduced where top-level files that are merged via conflicts aren't being linked correct anymore i.e. |
|
Well, there is a bug, but its not due to this PR. I'm not sure if its pre-existing or was introduced by #3448. In any case, right now, the conflict merge logic won't produce the correct result if the same file is provided twice. It treats all the entries as directories, so tries to group the conflicting files under a directory named after the file. In practice, this would mostly only affect pkgutil namespace packages with non-trivial logic in |
|
Sent #3458 to fix that bug |
…into refactor.optimize.venv.namespace.packages
….namespace.packages
….namespace.packages
….namespace.packages
…into fix.conflict.merging.files
….namespace.packages
|
dependent PR merged, PTAL |
When implicit namespace packages are used, it's common for multiple distributions
to install into the same directory, triggering the expensive conflict merging
logic. This can be observed wit our doc builds, where
sphinxcontribis anamespace package that 7 distributions install into.
To fix, treat top-level directories that have an importable name and don't have an
__init__looking file as implicit namespace packages and mark them as disallowedfrom being directly linked. The importable name check is to exclude dist-info
directories.