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

VCS Resolvable #770

Closed
wants to merge 1 commit into from
Closed

VCS Resolvable #770

wants to merge 1 commit into from

Conversation

adeandrade
Copy link

@adeandrade adeandrade commented Sep 30, 2019

We rely on the pip package to export the repo into a temporary location and let ResolvableDirectory do the rest of the work.

@adeandrade
Copy link
Author

adeandrade commented Sep 30, 2019

Described in: #93 and pantsbuild/pants#3063

There's a problem with setuptools in the way it is parsing package dependencies (defined in a setup.py). The requirements returned do not have the URL, just the package name. This is limiting the usage of VCS URLs in PEX to requirements.txt files, for now.

I'll probably try to fix it when I have a use case for it.

Also, there's a problem with redbaron parsing a package dependency of pip. The syntax seems to be valid so it must be a bug with the parser. Bumping the version of redbaron did not help. Another dependency of pip adds imports by tinkering with sys.modules. These imports do not get vendorized. As a workaround for both problems, we drop the --upgrade flag for pip in the vendorizing script, so files don't get overwritten, and we also enhance the script to ignore the vendorization of certain files.

@adeandrade adeandrade force-pushed the vcs-support branch 14 times, most recently from 5c5cc02 to e046eba Compare October 1, 2019 14:21
@obendidi
Copy link

obendidi commented Oct 20, 2019

hello, thanks for your work, any news on when this would land?
would be glad to help if you give me directions to complete your work :)

@benjyw benjyw requested a review from jsirois October 20, 2019 16:59
@jsirois
Copy link
Member

jsirois commented Oct 20, 2019

@adeandrade vendoring pip is pretty heavyweight just to get vcs resolvables. It's not out of the question as a move for pex to make, but if we make it it would seem to make sense to let pip take over resolution completely - folks expect pex resolves to work just like pip resolves and sometimes they do not. This would fix manylinux2010+ support and other similar issues. See:

Pip is a large package though, adding ~5MB IIRC to pex which currently sits at 3MB + due to vendoring setuptools.

I feel like this move warrants wider discussion in the pants #pex slack channel in reference to a separate tracking issue here on GitHub. I'll file an issue and start up a discussion. This move has been a possibility for a while now and I think it's time to sanity check the idea with the community.

@jsirois
Copy link
Member

jsirois commented Feb 25, 2020

Thanks for helping spur movement here @adeandrade. Now that Pex 2+ uses pip internally, VCS requirements are fully supported. I'm going to close this PR as a result.

@jsirois jsirois closed this Feb 25, 2020
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.

3 participants