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

Kill pkg_resources finders monkey-patching. #827

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Dec 12, 2019

We no longer used any of this in production except for one PEXBuilder
API which is converted to use pex.pip to install zipped wheels when
encountered so that they can be found by standard pkg_resources
finders.

Tests that use zipped wheels are similarly updated.

We no longer used any of this in production except for one `PEXBuilder`
API which is converted to use `pex.pip` to install zipped wheels when
encountered so that they can be found by standard `pkg_resources`
finders.

Tests that use zipped wheels are similaraly updated.
Comment on lines +330 to +334
spawn_install_wheel(
wheel=dist,
install_dir=dist_path,
target=DistributionTarget.for_interpreter(self.interpreter)
).wait()
Copy link
Contributor

Choose a reason for hiding this comment

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

This part I'm unclear on. Is this an optimization or necessary to get things working?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its needed to kick our custom zipped-wheel finder to the curb. It's not an optimization since all wheels added to a pex are installed, not zipped (this goes back to a few commits after wheel support was added way back). Really, the zipped wheel finder support was only ~needed in tests and now that's killed too.

@jsirois jsirois merged commit 1b7689e into pex-tool:master Dec 12, 2019
@jsirois jsirois deleted the finders/gut branch December 12, 2019 19:47
This was referenced Dec 27, 2019
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