Skip to content

fix(py_pex_binary): exclude hermetic python toolchain files to prevent AmbiguousDistributionError#930

Open
xangcastle wants to merge 1 commit intomainfrom
xangcastle/PY-14
Open

fix(py_pex_binary): exclude hermetic python toolchain files to prevent AmbiguousDistributionError#930
xangcastle wants to merge 1 commit intomainfrom
xangcastle/PY-14

Conversation

@xangcastle
Copy link
Copy Markdown
Member

Problem

When building a py_pex_binary from a py_binary that uses a hermetic Python toolchain (e.g., from rules_python), the build fails with:

pex.dist_metadata.AmbiguousDistributionError: Found more than one distribution inside external/python3_9_x86_64-unknown-linux-gnu/lib/python3.9/site-packages/:
pip-23.2.1.dist-info/METADATA
setuptools-68.2.2.dist-info/METADATA

This happens because the hermetic Python interpreter includes pip and setuptools in its site-packages, and py_pex_binary was incorrectly including these as dependencies. When Distribution.load() is called on the site-packages directory, it finds multiple .dist-info directories and raises an error.

Solution

Exclude hermetic Python toolchain files from being processed as PEX dependencies:

  1. Added exclusion pattern for the hermetic interpreter repository:

    • Pattern: python_interpreters+ (matches repos like aspect_rules_py++python_interpreters+python_3_11_aarch64_apple_darwin)
  2. Added specific file exclusions for internal toolchain files:

    • _distutils_hack - Part of the interpreter's distutils shim
    • distutils-precedence.pth - Interpreter configuration file

Changes

  • py/private/py_pex_binary.bzl: Added exclusion patterns and specific file filters
  • py/tests/py-pex-binary/BUILD.bazel: Updated test to use hermetic Python 3.11 toolchain to validate the fix

Testing

bazel test //py/tests/py-pex-binary:test__print_modules_pex
//py/tests/py-pex-binary:test__print_modules_pex PASSED

# these will match in bzlmod setup with --incompatible_use_plus_in_repo_names flag flipped.
"rules_python++python+",
"aspect_rules_py+/py/tools/",
"python_interpreters+",
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.

Don't we need the ~ version as well?

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.

Also I think if CI passed then either this line is not needed or it is not being tested...

if dest_path.find(exclude) != -1:
return []

if "_distutils_hack" in f.path:
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 is this?

load("//py:defs.bzl", "py_binary", "py_pex_binary")

# Test that both single-file modules (six) and multi-file modules (cowsay) work with py_pex_binary.
# Also tests that py_pex_binary works with hermetic Python toolchains (python_version = 3.11).
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.

Can that be a separate test? This is replacing an old test, not "also" testing something...

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