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

Support creating a PEX from a venv. #1361

Open
jsirois opened this issue Jun 14, 2021 · 8 comments
Open

Support creating a PEX from a venv. #1361

jsirois opened this issue Jun 14, 2021 · 8 comments

Comments

@jsirois
Copy link
Member

jsirois commented Jun 14, 2021

This would close the circle and we have all the infra to do this. Currently resolution from a fixed set of Distributions is buried in PEXEnvirronment but could be lifted out. The Virtualenv class would just need to grow iter_distributions to make all this work. That could be implemented via pkg_resources.WorkingSet() in a subprocess using the venv interpreter since a WorkingSet is an iterator of Distibutions.

@jsirois
Copy link
Member Author

jsirois commented Jun 15, 2021

The wrinkle here is only dist-info distributions work with PEXes isolation scheme. That isolation scheme requires a wheel name and that can only be reconstructed from a dist-info distribution with a WHEEL metadata file with a Tag entry. If the venv is populated via Pip - a common case to be sure - any sdists are installed via a different scheme that uses egg-info and provides no way to directly derive a wheel name that I'm aware of.

@jsirois
Copy link
Member Author

jsirois commented Oct 30, 2021

I guess the safe option for sdists is to adopt the most specific platfrom tag of the venv's interpreter. That would treat every distribution in the venv installed from an sdist as if it were platform specific even when this was un-true. The upshot would be the resulting PEX file is not as portable as it should be. For example, if the venv contained one distribution installed from an sdist that was in fact universal (py2.py3), the PEX created from that venv would only be able to run under the venv interpreter (say CPython 3.9 on Linux). To mitigate surprise, a warning could be emitted for each distribution gathered into the PEX that was derived from an sdist and not a wheel.

@Eric-Arellano
Copy link
Contributor

To mitigate surprise, a warning could be emitted for each distribution gathered into the PEX that was derived from an sdist and not a wheel.

That sounds reasonable.

Maybe point out that building a wheel beforehand can provide a workaround. It's likely not obvious to everyone that you can prebuild an sdist and have Pex use the resulting wheel.

@jsirois
Copy link
Member Author

jsirois commented Nov 2, 2021

It's likely not obvious to everyone that you can prebuild an sdist and have Pex use the resulting wheel.

It might not be obvious, but Pex is not involved here. They created the venv with some tool. Pip, Poetry - who knows. What exactly would you say to a Poetry user for example? (I don't think there is actually an answer to that one). How about a Pip user? (There is an answer to that one).

@jsirois
Copy link
Member Author

jsirois commented Nov 2, 2021

I like providing a useful error message. I really don't like educating the world and misleading them as a result down to some arbitrary level of detail. When things get complex, they get complex, The user needs to buck up and read at that point. I guess the real issue is where to draw the line. I favor simple, totally correct advice. I think you favor advice that solves some % of users problems right away like a stack overflow answer thats mostly right? IOW better to get some % of users unblocked thatn worry about confusing some smaller % of users?

@jsirois
Copy link
Member Author

jsirois commented Nov 2, 2021

They created the venv with some tool. Pip, Poetry - who knows. What exactly would you say to a Poetry user for example? (I don't think there is actually an answer to that one). How about a Pip user? (There is an answer to that one).

Thumbs Up.

I really meant for you to answer those. Do you have verbage I'm missing that explains this stuff? If you do and it doesn't lie I'm happy to use it.

@Eric-Arellano
Copy link
Contributor

I think you favor advice that solves some % of users problems right away like a stack overflow answer thats mostly right? IOW better to get some % of users unblocked thatn worry about confusing some smaller % of users?

Ideally, give the right answer to the majority of users while pointing out this might not be the correct prognosis. You may need to do more digging.

pantsbuild/pants#12107 is a good example of what I think is helpful, including discussion about Pants users being engineers and what we can expect them to know. I like Tom's point about data scientists. The error tries to hedge that these are only two likely culprits. (Although it should probably hedge even more!)

--

What exactly would you say to a Poetry user for example?

Good question. I think I would say something like this:

If you need more portability, consider pre-building the sdist and creating your venv with the resulting wheel file rather than an sdist, such as by installing the requirement {req.project_name} with a local file.

I would not give a guide on how to do this with pip and Poetry, I agree with you there. My intent is to give a hint that can help a motivated user know what direction to take, rather than having to brainstorm the workaround themself.

@jsirois
Copy link
Member Author

jsirois commented Jan 17, 2022

I've found an obvious-after-the-fact way to stomp out the need for a long-winded partial truth by actually just re-resolving any non-dist-info dists intransitively using the existing machinery so that this can work on all venvs with no exception. For the distributions that aren't dist-info but should also not be re-resolved - the canonical example being a hard to build sdist - an egg-info -> egg directory (loose egg) -> wheel convert -> wheel install with most specific platform tag of the venv interpreter can be requested. The pip way of spelling this sort of thing in an option would be something like --venv-repository-egg-info-keep [:all:|[project1,[project2,...[projectN]]]]. I'm not sure that's a great precedent for the spelling here, but its an idea that users of Pip's --no-binary / --only-binary options would be familiar with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants