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

Git URLs no longer supported in __requires__? #22

Closed
jwodder opened this issue Nov 2, 2017 · 8 comments
Closed

Git URLs no longer supported in __requires__? #22

jwodder opened this issue Nov 2, 2017 · 8 comments

Comments

@jwodder
Copy link

jwodder commented Nov 2, 2017

Somewhere between versions 2.15.1 and 3.0, rwt stopped supporting (specifically, it appears to flat-out ignore) package specifiers of the form git+ssh://git@example.com/repo.git in __requires__ lists, though it continues to support them in requirements.txt files. I see no mention of this change in CHANGES.rst; was it intentional? If not, could we have the old behavior back?

@jaraco
Copy link
Owner

jaraco commented Nov 13, 2017

Here's what changed.

@jaraco
Copy link
Owner

jaraco commented Nov 13, 2017

The changelog suggests in 2.16 that #18 changed the meaning of __requires__.

@jaraco
Copy link
Owner

jaraco commented Nov 13, 2017

Sorry for the inconvenience.

It was an unintentional limitation to constrain the meaning of __requires__. The intention was actually to expand the meaning of it.

And now it seems we've run up against a fundamental difference in the definition of requirements between pip and pkg_resources.

Best I can understand, pkg_resources expects the __requires__ to be package requirement specifications and not dependency links. Indeed, pkg_resources would fail similarly:

$ cat > script.py
__requires__ = 'https://github.com/jaraco/rwt',
import pkg_resources
$ python script.py 
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/packaging/requirements.py", line 90, in __init__
    req = REQUIREMENT.parseString(requirement_string)
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/pyparsing.py", line 1617, in parseString
    raise exc
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/pyparsing.py", line 1607, in parseString
    loc, tokens = self._parse( instring, 0 )
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/pyparsing.py", line 3376, in parseImpl
    loc, exprtokens = e._parse( instring, loc, doActions )
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/pyparsing.py", line 1383, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/pyparsing.py", line 3164, in parseImpl
    raise ParseException(instring, loc, self.errmsg, self)
pkg_resources._vendor.pyparsing.ParseException: Expected stringEnd (at char 5), (line:1, col:6)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 3015, in __init__
    super(Requirement, self).__init__(requirement_string)
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/_vendor/packaging/requirements.py", line 94, in __init__
    requirement_string[e.loc:e.loc + 8]))
pkg_resources.extern.packaging.requirements.InvalidRequirement: Invalid requirement, parse error at "'://githu'"

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "script.py", line 2, in <module>
    import pkg_resources
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 3161, in <module>
    @_call_aside
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 3145, in _call_aside
    f(*args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 3174, in _initialize_master_working_set
    working_set = WorkingSet._build_master()
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 666, in _build_master
    ws.require(__requires__)
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 984, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 826, in resolve
    requirements = list(requirements)[::-1]
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 3008, in parse_requirements
    yield Requirement(line)
  File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/pkg_resources/__init__.py", line 3017, in __init__
    raise RequirementParseError(str(e))
pkg_resources.RequirementParseError: Invalid requirement, parse error at "'://githu'"

My instinct is the best thing to do here is to retain the constraint (to be consistent with pkg_resources, the defacto standard bearer for __requires__) and for projects that need full URLs to specify dependencies, they should use requirements.txt.

Depending on how well that approach satisfies the use case, or rather to the extent that it doesn't, I'd be open to rwt supporting another variable such as __requirements__ that's explicitly meant to be used for pip-based requirements declarations. Based on this comment, I'd want to be sure to get an endorsement from @dstufft (and probably others) before creating a new standard.

I'd also be open to expanding the meaning of __requires__, but that change would need to happen within the larger context of setuptools and pkg_resources.

@jwodder Can you share more about the impact of this change on your use-case and which of these three approaches above would best suit your expectation?

In the meantime, I'll update the changelog to reflect the broader impact of this change.

@benoit-pierre
Copy link
Contributor

I don't think something like git+ssh://git@example.com/repo.git should be supported: instead the format should be the same as for dependency_links; e.g. git+ssh://git@example.com/repo.git#egg=foo-0.42.

Because ideally I think using rwt -- script.py should work the same as python script.py if all the requirements are already available (this currently fails because pip is called with no requirement).

Tentative patch:

 rwt/scripts.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git i/rwt/scripts.py w/rwt/scripts.py
index 115e00c..335eff8 100644
--- i/rwt/scripts.py
+++ w/rwt/scripts.py
@@ -18,9 +18,12 @@ if sys.version_info < (3,):
 
 class Dependencies(list):
 	index_url = None
+	dependency_links = []
 
 	def params(self):
 		prefix = ['--index-url', self.index_url] if self.index_url else []
+		for link in self.dependency_links:
+			prefix.extend(['--find-links', link])
 		return prefix + self
 
 
@@ -73,6 +76,12 @@ class DepsReader:
 			deps.index_url = self._read('__index_url__')
 		except Exception:
 			pass
+		try:
+			raw_links = self._read('__dependency_links__')
+		except Exception:
+			pass
+		else:
+			deps.dependency_links = list(pkg_resources.yield_lines(raw_links))
 		return deps
 
 	def _read(self, var_name):

@benoit-pierre
Copy link
Contributor

Just to be clear, example use case with the patch above:

__requires__ = '''
foo==0.42
'''

__dependency_links__ = '''
git+ssh://git@example.com/repo.git#egg=foo-0.42
'''

[...]

@jaraco
Copy link
Owner

jaraco commented Nov 14, 2017

I like that idea. It follows patterns already in place for things like __index_url__ and honors the shared interfaces of pkg_resources and pip.

if all the requirements are already available, [rwt] currently fails because pip is called with no requirement

It shouldn't fail, but it'll log a warning:

$ rwt -- -c "print('hello world')"
You must give at least one requirement to install (see "pip help install")
hello world

@benoit-pierre
Copy link
Contributor

PR here: #24.

It shouldn't fail, but it'll log a warning:

You're right. IMHO, it would be better if pip was not called if all requirements are already satisfied.

@jaraco
Copy link
Owner

jaraco commented Nov 18, 2017

Huge thanks @benoit-pierre. Released as 3.1.

@jwodder Do let us know if this approach doesn't satisfy your needs.

@jaraco jaraco closed this as completed Nov 18, 2017
jaraco added a commit that referenced this issue Aug 26, 2023
Suppress the spurious error from mypy as reported in python/mypy#15970. Closes #22.
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

No branches or pull requests

3 participants