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.

Sign up for GitHub

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

Jump to bottom

Support git+https / git+git style dependency in 3rdparty Python #3063

Closed
leozc opened this issue Mar 18, 2016 · 11 comments · Fixed by #10728
Closed

Support git+https / git+git style dependency in 3rdparty Python #3063

leozc opened this issue Mar 18, 2016 · 11 comments · Fixed by #10728
Assignees
@Eric-Arellano
Labels
enhancement
Milestone
2.0.0 blocker

Comments

@leozc
Copy link

leozc commented Mar 18, 2016

in requirements.txt
git+https://github.com/daviddrysdale/python-phonenumbers.git@daed536e4c90438f78e70f3a2b2f2f4421f0f15d#egg=python-phonenumbers

OUTPUT:

Exception caught: (<class 'pants.build_graph.build_graph.TransitiveLookupError'>)

Exception message: ('Expected version spec in', 'git+https://github.com/daviddrysdale/python-phonenumbers.git@daed536e4c90438f78e70f3a2b2f2f4421f0f15d#egg=python-phonenumbers', 'at', '+https://github.com/daviddrysdale/python-phonenumbers.git@daed536e4c90438f78e70f3a2b2f2f4421f0f15d#egg=python-phonenumbers')
while executing BUILD file BuildFile(3rdparty/python/BUILD, FileSystemProjectTree(/foo/repo))
Loading addresses from '3rdparty/python' failed.
....
@kwlzn
Copy link
Member

kwlzn commented Mar 18, 2016

this is essentially a duplicate of pex-tool/pex#93 + a pex release and version bump.

@cburroughs
Copy link
Contributor

Also related: #2679

@kwlzn kwlzn mentioned this issue Mar 31, 2016
Closed
@ghost
Copy link

ghost commented Mar 31, 2016

@kwlzn @cburroughs I'd like to work on this. We use pants to package Python and this has been an issue for us. Have there been any plans on how this needs to be implemented?

@kwlzn
Copy link
Member

kwlzn commented Mar 31, 2016

@copyconstructor that'd be awesome - have heard of at least a few folks wanting this feature recently.

afaict, no plans have been discussed. however, the meat of the change would happen against pex - the library that pants uses under the hood to build python artifacts. the tracking issue under pex is: pex-tool/pex#93

once this lands in a pex release, the scope of this ticket for the pants-side consumption should be more or less to simply version bump the pex requirement and test it out end to end.

I can help you get the change shipped in pex and released to pypi for consumption. Feel free to submit a PR and continue the discussion on pex-tool/pex#93.

@ghost
Copy link

ghost commented May 10, 2016

@kwlzn Sorry for the dilatory response - If no one's been working on this as yet, I'd love to hop in next week when I'm on vacation and see what I can come up with. Please let me know if this is something that's up for grabs. Thanks!

@kwlzn
Copy link
Member

kwlzn commented May 18, 2016

@copyconstructor definitely up for grabs - feel free.

@kwlzn kwlzn added the python label May 25, 2016
@tispratik
Copy link

Just checking if this is in the works?

@eirenik0
Copy link

eirenik0 commented Feb 22, 2019 •
edited
Loading

@tispratik no, it isn't

@adeandrade adeandrade mentioned this issue Sep 30, 2019
Closed
@stuhood
Copy link
Member

stuhood commented May 29, 2020

This is resolved as of pants 1.26.x via pex 2!

@stuhood stuhood closed this as completed May 29, 2020
@jsirois
Copy link
Contributor

jsirois commented May 29, 2020 •
edited
Loading

Not so fast, still some work to do on the Pants side:

pants/src/python/pants/python/python_requirement.py

Lines 8 to 33 in acde9be

class PythonRequirement:
"""A Pants wrapper around pkg_resources.Requirement.
Describes an external dependency as understood by `pip`. It takes a single non-keyword argument
of the `Requirement`-style string, e.g.
python_requirement('django-celery')
python_requirement('tornado==2.2')
python_requirement('kombu>=2.1.1,<3.0')
Pants resolves the dependency _and_ its transitive closure*. For example, `django-celery` also
pulls down its dependencies: `celery>=2.5.1`, `django-picklefield>=0.2.0`, `ordereddict`,
`python-dateutil`, `kombu>=2.1.1,<3.0`, `anyjson>=0.3.1`, `importlib`, and `amqplib>=1.0`.
To let other Targets depend on this `python_requirement`, put it in a
`python_requirement_library`.
:API: public
"""
def __init__(
self, requirement: str, name=None, repository=None, use_2to3=False, compatibility=None
) -> None:
# TODO(wickman) Allow PythonRequirements to be specified using pip-style vcs or url
# identifiers, e.g. git+https or just http://...
self._requirement = Requirement.parse(requirement)

pkg_resources doesn't understand anything except traditional requirement strings and line 33 will blow up.

@jsirois jsirois reopened this May 29, 2020
@jsirois jsirois mentioned this issue May 29, 2020
Closed
@Eric-Arellano Eric-Arellano mentioned this issue Jul 17, 2020
Closed
@jsirois jsirois mentioned this issue Jul 24, 2020
Open
@darthveda-ai
Copy link

Having support for URL to wheel files in third party dependency would be very helpful. Lack of this support currently, has become a blocker for adoption of pants across the repo in my usecase. A gentle bump from my end to increase its priority.

@Eric-Arellano Eric-Arellano added this to the 2.0.0rc0 milestone Sep 2, 2020
@Eric-Arellano Eric-Arellano self-assigned this Sep 3, 2020
@Eric-Arellano Eric-Arellano mentioned this issue Sep 3, 2020
Merged
Eric-Arellano added a commit that referenced this issue Sep 4, 2020
@Eric-Arellano
Formalize support for VCS-style requirements via PEP 440 (#10728)
5886865 
Since 2016, users have been asking for a way to install wheels from a URL, e.g. from Git.

There are two approaches to VCS-style requirements:

1) Pip's proprietary syntax, e.g. `git+https://github.com/pypa/pip.git#egg=pip`. This is what most people are familiar with. https://pip.pypa.io/en/stable/reference/pip_install/#vcs-support
2) PEP 440's standardized "direct references", e.g. `pip@ git+https://github.com/pypa/pip.git`.

Turns out, ever since we upgraded to Pex 2.0, approach #2 has worked to achieve this goal. But few users knew about it (including us), and approach #1 has continued to not work.

We use setuptools to parse our requirements before passing to Pex. Why? We need to normalize it to, for example, strip comments from `requirements.txt`; and because we need the parsed information to do things like create a module mapping for dependency inference. However, `pkg_resources.Requirement.parse()` chokes on Pip's VCS style, and only understands PEP 440.

Both the Pip VCS and PEP 440 approaches achieve the same end goal. We could use some custom libraries like https://github.com/sarugaku/requirementslib and https://github.com/davidfischer/requirements-parser to allow for Pip's proprietary format, but that adds new complexity to our project.

Nevertheless, few users are familiar with PEP 440, so we eagerly detect if they're trying to use the Pip style and have a nice error message for how to instead use PEP 440:

```
▶ ./pants list 3rdparty/python: --no-print-exception-stacktrace
13:25:59.67 [ERROR] 1 Exception encountered:

  MappingError: Failed to parse 3rdparty/python/BUILD:
Invalid requirement 'git+https://github.com/django/django.git#egg=django' in 3rdparty/python/requirements.txt at line 32: Parse error at "'+https:/'": Expected stringEnd

It looks like you're trying to use a pip VCS-style requirement?
Instead, use a direct reference (PEP 440).

Instead of this style:

    git+https://github.com/django/django.git#egg=django
    git+https://github.com/django/django.git@stable/2.1.x#egg=django
    git+https://github.com/django/django.git@fd209f62f1d83233cc634443cfac5ee4328d98b8#egg=django

Use this style, where the first value is the name of the dependency:

    Django@ git+https://github.com/django/django.git
    Django@ git+https://github.com/django/django.git@stable/2.1.x
    Django@ git+https://github.com/django/django.git@fd209f62f1d83233cc634443cfac5ee4328d98b8
```

Closes #3063.

### Additional benefit: doesn't allow `--editable`

With Pip VCS-style requirements, it's common to use `-e` or `--editable`, which changes how the distribution is downloaded so that the user can make edits to it. That is not sensible in Pants, and we are unlikely to ever support this feature.

PEP 440 style does not support `-e`, so, when users migrate to PEP 440, they won't have the opportunity to inadvertently use `-e` and for it to not work like they expect.

[ci skip-rust]
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Eric-Arellano Eric-Arellano

Labels
enhancement
Projects
None yet
Milestone
2.0.0 blocker
Development

Successfully merging a pull request may close this issue.

Formalize support for VCS-style requirements via PEP 440 Eric-Arellano/pants
9 participants
@cburroughs @stuhood @tispratik @jsirois @eirenik0 @leozc @kwlzn @Eric-Arellano @darthveda-ai