rez icon indicating copy to clipboard operation
rez copied to clipboard

Fix multiple errors resolving rez pip dependencies

Open tlm2021 opened this issue 8 years ago • 4 comments
trafficstars

This fixes four errors involving dependency resolution when using rez pip.

  1. If the python package already includes a pip dependency for a package, rez pip fails
  2. get_distribution_name is called twice when parse_name_and_version successfully returns a package name.
  3. Support for pip dependency version strings with multiple version specifications
  4. Support for '!=' in pip dependency strings.

Package dependencies included in Python package

When doing rez-bind python, it includes in the resulting python package any packages that were already installed via pip.

Later, when using rez-pip, if you install a package that has a requirement that is already included in your python package, the rez-pip command will error as it tries to pair that requirement with a distributions.

Example:

>> pip install six
>> rez-bind python
searching /home/tmosley/dev-local/rez/install/lib/python2.7/site-packages/rez-2.14.1-py2.7.egg/rez/bind...
creating package 'python' in /home/tmosley/packages...
...

>> rez pip -i pkg-requires-six
13:10:32 INFO     Using pip-9.0.1 (/home/tmosley/packages/pip/9.0.1/package.py[0])
...

Requirement already satisfied: six>=1.9.0 in ./packages/python/2.7.5/platform-linux/arch-x86_64/os-CentOS-7.4.1708/python/extra (from pkg-requires-six)
...

Traceback (most recent call last):
  File "/home/tmosley/dev-local/rez/install/bin/rez/rez", line 4, in <module>
    run()
  File "/home/tmosley/dev-local/rez/install/lib/python2.7/site-packages/rez-2.14.1-py2.7.egg/rez/cli/_main.py", line 118, in run
    returncode = run_cmd()
  File "/home/tmosley/dev-local/rez/install/lib/python2.7/site-packages/rez-2.14.1-py2.7.egg/rez/cli/_main.py", line 110, in run_cmd
    return opts.func(opts, opts.parser, arg_groups[1:])
  File "/home/tmosley/dev-local/rez/install/lib/python2.7/site-packages/rez-2.14.1-py2.7.egg/rez/cli/pip.py", line 46, in command
    release=opts.release)
  File "/home/tmosley/dev-local/rez/install/lib/python2.7/site-packages/rez-2.14.1-py2.7.egg/rez/pip.py", line 264, in pip_install_package
    requirements.extend(_get_dependencies(requirement, distributions))
  File "/home/tmosley/dev-local/rez/install/lib/python2.7/site-packages/rez-2.14.1-py2.7.egg/rez/pip.py", line 76, in _get_dependencies
    name = get_distrubution_name(name)
  File "/home/tmosley/dev-local/rez/install/lib/python2.7/site-packages/rez-2.14.1-py2.7.egg/rez/pip.py", line 47, in get_distrubution_name
    pip_to_rez_name = pip_name.lower().replace("-", "_")
AttributeError: 'NoneType' object has no attribute 'lower'

Support for pip dependency strings with multiple versions

Multiple pip dependency versions would end up just being joined together, creating a malformed string. EG:

'my_package!=1.3.2,>=1.3'   ...   'my_package->=1.3!=1.3.2'

Now these are split into two distinct version strings:

'my_package!=1.3.2,>=1.3'  ...   ['my_package->=1.3', '!babel-1.3.2']

Support for exclusions

pip exclusions didn't translate to a valid string. Now they do

my_package!=1.2.3   ...   !my_package-1.2.3

tlm2021 avatar Oct 27 '17 21:10 tlm2021

Thank you @tlm2021.

Hi @lambdaclan, @JeanChristopheMorinPerso would be great to review this PR and incorporate these fixes in #628.

I've not yet had a chance to review #628 but I did note JC's comment (...VersionRange('>=2.6, !=3.0.*, !=3.1.*, !=3.2.*, <4') will unfortunately not work). I see there's a similar issue that's fixed here.

It may make sense to have a separate utility function able to convert from pip requirements syntax to rez. Specifically it's clear from https://www.python.org/dev/peps/pep-0508/ that there are other cases also that would have to be converted to rez equivalent (eg mypkg-1.2.*). As JC points out, the biggest difference is that conflict operator is expressed as part of version range syntax in pep508 (eg mypkg-!1.0.0) whereas in rez it is not (it's a separate request for !mypkg-1.0.0).

nerdvegas avatar Jun 23 '19 10:06 nerdvegas

Sure, I'll be happy to review. I will have time tomorrow for this.

I will start working on #628 once we get #602 merged :+1:

lambdaclan avatar Jun 25 '19 07:06 lambdaclan

Hey guys. FYI, if memory serves this solution worked for most packages, but I still encountered some it tripped up on. Unfortunately, the company I did this work for folded over a year ago so I don't have quick access to the test environment I was using. And I can't remember what packaging feature gave it trouble.

Will respond to notes...but might be slow getting spun back up. :)

tlm2021 avatar Jun 25 '19 17:06 tlm2021