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

VersionRange breaks if range is not in order #455

Closed
instinct-vfx opened this issue Sep 11, 2017 · 6 comments
Closed

VersionRange breaks if range is not in order #455

instinct-vfx opened this issue Sep 11, 2017 · 6 comments
Labels

Comments

@instinct-vfx
Copy link
Contributor

I am not 100% sure this is a bug or may be expected and/or wanted. As mentioned before i have my own incarnation of rez-pip mostly for windows versions. It fails when installing requests with dependencies because the urllib3 requirement is stated in "inverse" order.
I am unsure where this comes from. It seems fine in the setup.py of requests but it is inverted on pypi already so it is not being interpreted wrong within rez or distlib.

Anyways it is easy to reproduce:

from rez.vendor.version.version import VersionRange
print VersionRange('>=1.21.1,<1.23')
print VersionRange('<1.23,>=1.21.1')

The first one works the second breaks with a Syntax Error:

PackageRequest(u'urllib3-<1.23,>=1.21.1') raised VersionError(u"Syntax error in version range '<1.23,>=1.21.1': Syntax error in version range '<1.23,>=1.21.1'",)

@instinct-vfx
Copy link
Contributor Author

Taking a closer look this is "as designed" but i am not sure if it is intended. It is also _VersionRangeParser breaking of course. I guess my question is if i have to sanitize versions before passing them to _VersionRangeParser or create a PR that adds support for flipped range bounds to version_range_regex

@nerdvegas
Copy link
Contributor

nerdvegas commented Sep 11, 2017 via email

@instinct-vfx
Copy link
Contributor Author

I just took a look and essentially it is a huge regex that does expect lower_bound,upper_bound only. I will take a look but i suck with regexes so i am not sure if i can get that to work. It seems to affect quite a few packages here now though and i wonder if anything on the pip / pypi side has changed? I don't remember that happening before.

@nerdvegas
Copy link
Contributor

nerdvegas commented Sep 12, 2017 via email

@instinct-vfx
Copy link
Contributor Author

A colleague of mine @PatrickFoerster extended the regex in version.py to support both ascending and descending order. With the cases i had this works fine. Here is the MR: #470

@JeanChristopheMorinPerso
Copy link
Member

Fixed by #618

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

Successfully merging a pull request may close this issue.

3 participants