enthought / sat-solver

Default Repo description from terraform module
Other
3 stars 1 forks source link

Fix 7z requirement parsing error #260

Closed JCorson closed 7 years ago

JCorson commented 7 years ago

I am not terribly crazy about this fix, but it is the best I have come up with so far. The main issue seems to be that if a package name starts with a number, it is parsed as a VersionToken. The difficulty is that we actually want some valid version constraint strings to also be valid package names. This fix just adds a check to ensure the distribution does not actually match the _DISTRIBUTION_NAME_R pattern before raising the error.

cournape commented 7 years ago

@JCorson why not fixing the _R_DISTRIBUTION_NAME regex instead ?

JCorson commented 7 years ago

@cournape I think I may have come up with a better way. The issue is more that the _VERSION_R is matching package names that start with a number than an issue with _DISTRIBUTION_NAME_R itself. https://github.com/enthought/sat-solver/pull/260/commits/1d25be67bc45979686cf5cb1aa475c6ea0f8c9ac changes _VERSION_R to only match if the group ends with a word boundary and is not followed by a space. Does that make sense for this case?

johntyree commented 7 years ago

I think pinning the _VERSION_R to the end of the string is pretty sensible. :+1:

cournape commented 7 years ago

@johntyree long time no see ;) Since you're here, I dare asking: do you remember the rationale for the fairly cryptic regular expressions _DISTRIBUTION_NAME_R and _VERSION_R ?

cournape commented 7 years ago

@JCorson sounds good