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

Fix symlinked venv ns-package calcs. #2165

Merged
merged 9 commits into from
Jul 2, 2023
Merged

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jul 2, 2023

Previously namespace packages were handled only amongst 3rd-party deps.
Any ns-package also split across PEX user sources would lead to one of
the 3rd-party dep members of the namespace package getting its cached
wheel install contaminated with the PEX user source that was a member of
the ns-package.

Fixes #2160

IPython can inject ~PS1-style prefixes depending on the IPython version
and Python version in-play. Avoid having to worry about this with a
dedicated test communication channel separate from stdio.
It turns out PyPy 7.3.12 has them! In particular hpy.
Previously namespace packages were handled only amongst 3rd-party deps.
Any ns-package also split across PEX user sources would lead to one of
the 3rd-party dep members of the namespace package getting its cached
wheel install contaminated with the PEX user source that was a member of
the ns-package.

Fixes pex-tool#2160
@jsirois
Copy link
Member Author

jsirois commented Jul 2, 2023

The only commits to review are from 0927f43 on - the rest gets CI green and is off for review in #2162.

assert ProjectName("top_level") == top_level.metadata.project_name
assert Version("0.1.0") == top_level.metadata.version

assert [os.path.join(top_level.location, "top_level", "lib.py")] == [
Copy link
Member Author

Choose a reason for hiding this comment

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

This test fails pre-fix for 2/9 cases as expected: symlinked venv {packed,zipapp}

Use pkgutil namespace packages and older setuptools config style.
@jsirois jsirois requested a review from benjyw July 2, 2023 14:44
Copy link
Collaborator

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

I only reviewed 0927f43, the rest looked like they're covered in other PRs.


top_level_library = os.path.join(top_level_project, "top_level", "lib.py")
with safe_open(top_level_library, "w") as fp:
fp.write("OG = 'Henry Barber'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Respect!

Copy link
Collaborator

Choose a reason for hiding this comment

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

(I had to look him up though)

Copy link
Member Author

Choose a reason for hiding this comment

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

I grew up trying to climb Barber routes, sometimes succeeding. It's been a long time but just got on another one many years later out at the Ophir Wall. Even if not physically challenged it's almost certain he'll have provided a mental one!

@jsirois jsirois merged commit 363676a into pex-tool:main Jul 2, 2023
26 checks passed
@jsirois jsirois deleted the issues/2160 branch July 2, 2023 16:42
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.

Bad installed_wheels dir when a wheel's top_level.txt dirname collides with sources
2 participants