Skip to content

fix: pkgutil namespace package __init__.py collisions when multiple wheels share a namespace stub#1270

Open
xangcastle wants to merge 2 commits into
mainfrom
xangcastle/main-namespace-collision
Open

fix: pkgutil namespace package __init__.py collisions when multiple wheels share a namespace stub#1270
xangcastle wants to merge 2 commits into
mainfrom
xangcastle/main-namespace-collision

Conversation

@xangcastle

@xangcastle xangcastle commented Jul 7, 2026

Copy link
Copy Markdown
Member

This PR fixes false-positive package collisions for namespace packages like backports where each wheel ships an equivalent __init__.py that only declares a pkgutil or pkg_resources namespace. The wheel metadata extractor now detects these stubs, marks the top-level as a namespace package, and the venv assembly merges the entries with individual symlinks instead of treating it as a collision. site_merge.py also learns to treat equivalent namespace stubs as the same file during physical merges as a defensive fallback.


Changes are visible to end-users: yes/no

  • Searched for relevant documentation and updated as needed: yes/no
  • Breaking change (forces users to change their own code or config): yes/no
  • Suggested release notes appear below: yes/no

Test plan

  • Covered by existing test cases
  • New test cases added
  • Manual testing; please provide instructions so we can reproduce:

@aspect-workflows

aspect-workflows Bot commented Jul 7, 2026

Copy link
Copy Markdown

✨ Aspect Workflows Tasks

📅 Tue Jul 7 19:51:55 UTC 2026

✅ 36 successful tasks

  • ✅ buildifier · ⏱ 23.4s · 🐙 GitHub Actions · ☑️ Check
    💬 Format complete (clean)
  • ✅ gazelle · ⏱ 21.7s · 🐙 GitHub Actions · ☑️ Check
    💬 Gazelle complete (clean)
  • ✅ test-e2e-bazel-8 [test] · ⏱ 3m 44s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (192/192 passed)
  • ✅ test-e2e-bazel-9 [test] · ⏱ 3m 40s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (186/186 passed)
  • ✅ test-e2e-interpreter-input-validation-bazel-8 [test] · ⏱ 22.1s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-e2e-interpreter-input-validation-bazel-9 [test] · ⏱ 35.5s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-e2e-interpreter-runtime-metadata-bazel-8 [test] · ⏱ 23.2s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (2/2 passed · 2 cached)
  • ✅ test-e2e-interpreter-runtime-metadata-bazel-9 [test] · ⏱ 50s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (2/2 passed)
  • ✅ test-e2e-interpreter-toolchain-settings-bazel-8 [test] · ⏱ 18.3s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-e2e-interpreter-toolchain-settings-bazel-9 [test] · ⏱ 45.3s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-e2e-rules-proto-grpc-python-bazel-8 [test] · ⏱ 1m 55s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-e2e-rules-proto-grpc-python-bazel-9 [test] · ⏱ 1m 23s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-debugger-bazel-8 [test] · ⏱ 26.9s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed · 1 cached)
  • ✅ test-examples-debugger-bazel-9 [test] · ⏱ 1m 28s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-dev_deps-bazel-8 [test] · ⏱ 27.8s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed · 1 cached)
  • ✅ test-examples-dev_deps-bazel-9 [test] · ⏱ 36.9s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-django-bazel-8 [test] · ⏱ 30.6s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-django-bazel-9 [test] · ⏱ 41.1s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-multi_version-bazel-8 [test] · ⏱ 37.3s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (2/2 passed)
  • ✅ test-examples-multi_version-bazel-9 [test] · ⏱ 1m 1s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (2/2 passed)
  • ✅ test-examples-protobuf-bazel-8 [test] · ⏱ 1m 37s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-protobuf-bazel-9 [test] · ⏱ 2m 41s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-py_binary-bazel-8 [test] · ⏱ 25.8s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-py_binary-bazel-9 [test] · ⏱ 39.5s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-py_pex_binary-bazel-8 [test] · ⏱ 23.6s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-py_pex_binary-bazel-9 [test] · ⏱ 46.8s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-py_venv-bazel-8 [test] · ⏱ 28.6s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (3/3 passed)
  • ✅ test-examples-py_venv-bazel-9 [test] · ⏱ 28s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (3/3 passed)
  • ✅ test-examples-pytest-bazel-8 [test] · ⏱ 1m 24s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (9/9 passed)
  • ✅ test-examples-pytest-bazel-9 [test] · ⏱ 1m 28s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (9/9 passed)
  • ✅ test-examples-uv_pip_compile-bazel-8 [test] · ⏱ 24.2s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-uv_pip_compile-bazel-9 [test] · ⏱ 39.1s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-virtual_deps-bazel-8 [test] · ⏱ 28s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-examples-virtual_deps-bazel-9 [test] · ⏱ 51.5s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed)
  • ✅ test-root-bazel-8 [test] · ⏱ 3m 14s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (227/227 passed)
  • ✅ test-root-bazel-9 [test] · ⏱ 3m 33s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (226/226 passed)

⏱ Last updated Tue Jul 7 19:55:34 UTC 2026 · 📊 GitHub API quota 1,392/15,000 (9% used, resets in 34m)
🚀 Powered by Aspect CLI (v2026.28.2)  |  Aspect Build · X · LinkedIn · YouTube

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

py_binary startup benchmark

Version Mean (ms) Median (ms) ± stddev vs BCR vs main Build (s)
BCR 1.11.7 (baseline) 188.055 186.282 ±8.911 42.84
HEAD main 59.297 59.039 ±1.401 -68.5% 10.54
This PR 59.905 59.789 ±1.452 -68.1% +1.0% 7.59

Measured with hyperfine --warmup 5 --runs 50 on Linux
Gate: PR vs HEAD main (threshold: 10%). BCR is shown only as a historical baseline.
Build time: cold bazel build //:bench with isolated output base, no disk cache.

sys.path quality

Version sys.path entries distinct site-packages roots duplicate realpaths
BCR 1.11.7 (baseline) 6 1 0
HEAD main 7 2 0
This PR 7 2 0

sys.path quality measured by bench_syspath inside the assembled venv. Duplicate realpaths indicate symlink redundancy; many distinct site-packages roots suggest an inefficient venv layout.

Bazel analysis benchmark

Version Mean (ms) Median (ms) ± stddev vs BCR vs main Packages Targets
BCR 2.0.0-alpha.4 (baseline) 8265.123 8310.329 ±162.528 101 301
HEAD main 7675.629 7623.453 ±232.226 -7.1% 101 301
This PR 7801.374 7802.500 ±173.370 -5.6% +1.6% 101 301

Measured with hyperfine --warmup 1 --runs 10 on Linux
Gate: PR vs HEAD main (threshold: 10%). BCR is shown only as a historical baseline.
Command: cold bazel build --nobuild //workspace/... with isolated output base, no disk cache.

Auxiliary metrics

Version Loaded packages Configured targets
BCR 2.0.0-alpha.4 (baseline) 101 301
HEAD main 101 301
This PR 101 301

@xangcastle xangcastle force-pushed the xangcastle/main-namespace-collision branch from 794953c to 57025ec Compare July 7, 2026 19:51
@xangcastle xangcastle changed the title fix: reproducing package collision fix: pkgutil namespace package __init__.py collisions when multiple wheels share a namespace stub Jul 7, 2026
@xangcastle xangcastle requested a review from jbedard July 7, 2026 21:30
@xangcastle xangcastle marked this pull request as ready for review July 7, 2026 21:30
# Only emitted for wheels that contribute to a namespace — keeps the
# generated BUILD files (and their e2e snapshots) unchanged for the
# common regular-top-level-only case.
if pkgutil_namespace_top_levels_by_whl:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we have no snapshot tests covering this case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding a new one

@jbedard

jbedard commented Jul 7, 2026

Copy link
Copy Markdown
Member

Do we want to merge #1205 first to avoid waiting for that bot to rebase?

"""
if filecmp.cmp(str(src_file), str(dest), shallow=False):
return True
if src_file.name != "__init__.py" or dest.name != "__init__.py":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both are __init__.py files should we only do the _is_namespace_init_stub() calls and not filecmp.cmp()?

Today (just like in the rust version) we are reading the file twice, so 2x the IO operations, when both files are __init__.py. I think when analyzing the AST there's no point doing it twice?

try:
tree = ast.parse(content)
except SyntaxError:
return False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the file was empty? Do we have a test for that?

continue
meaningful.append(stmt)

if len(meaningful) != 1:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is an empty body not an "init ast"?

@jbedard

jbedard commented Jul 7, 2026

Copy link
Copy Markdown
Member

Robot feedback, possibly all 7 cases should have unit tests and be fixed?

Bugs / uncovered stub forms (false negatives → spurious collision errors)

1. The try/except form — the function's own documented example fails. Line 90 requires stmt.handlers[0].type is None, i.e. a bare except:. The canonical setuptools fallback stub uses except ImportError: (handler type is ast.Name), so the entire ast.Try branch rejects the exact code shown in its own comment at lines 297–301 of the diff. This is the standard pkg_resources-with-pkgutil-fallback template (shipped verbatim in protobuf's google/__init__.py lineage — confirmed the pattern in protobuf-3.0.0's wheel):

try:
    __import__('pkg_resources').declare_namespace(__name__)
except ImportError:
    __path__ = __import__('pkgutil').extend_path(__path__, __name__)

2. Plain import pkgutil / import pkg_resources statements are never filtered out. The pre-filter only drops from pkgutil import extend_path, so these common two-liners leave len(meaningful) == 2 and fail:

import pkgutil
__path__ = pkgutil.extend_path(__path__, __name__)
Consequence: "pkgutil form 2 (module attribute)" in _is_pkgutil_extend_path_call is dead code — it can never be reached, because the import pkgutil that makes pkgutil.extend_path meaningful always fails the count check first. Same for import pkg_resources + pkg_resources.declare_namespace(__name__).

3. from pkg_resources import declare_namespace is unhandled twice over. The import filter only recognizes module == "pkgutil", and _is_pkg_resources_declare_namespace_call has no bare-Name form (asymmetric with extend_path, which accepts extend_path(...) as a plain name):

from pkg_resources import declare_namespace
declare_namespace(__name__)

4. Multi-statement try/handler bodies — the zope/paste/PasteDeploy-style stub with imports inside the branches fails the len(...) == 1 checks:

try:
    import pkg_resources
    pkg_resources.declare_namespace(__name__)
except ImportError:
    from pkgutil import extend_path
    __path__ = extend_path(__path__, __name__)

5. Aliased import: from pkgutil import extend_path as ep is dropped by the filter (name matches), but __path__ = ep(...) isn't recognized. Related: import_seen (lines 48/60) is assigned
and never read — dead variable, and it means a pkgutil import paired with a pkg" the filter step.

Crashes (worse than false negatives)

6. AttributeError on bare try/except with non-expression statements. Lines 95–9 handlers[0].body[0].value without checking node type. A colliding __init__.pylike a compat shim:

try:
    import simplejson
except:
    pass
raises AttributeError: 'Import' object has no attribute 'value' — confirmed. _is_namespace_init_stub (line 202) only catches SyntaxError, and merge() has no guard, so the whole site_merge action dies with a traceback instead of reporting the collision.

7. UnicodeDecodeError on non-UTF8 files (in the wrapper _are_files_equivalent, a __init__.py with a # -*- coding: latin-1 -*- cookie and non-ASCII commentcrashes read_text() — confirmed. Should be read_text(errors=...) or bytes-compare-first (byte-identical is already handled, but differing non-UTF8 files crash).

@jbedard

jbedard commented Jul 7, 2026

Copy link
Copy Markdown
Member

What does native uv/pip/python do though? Can we include references to those docs or implementations?

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

Successfully merging this pull request may close these issues.

2 participants