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

Permit local version string to be empty (e.g. "1.0+" equals "1.0") #150

Closed
wants to merge 3 commits into from

Conversation

paulproteus
Copy link

This permits "2.7.15+" to be a valid Python version for pythonfinder. For details, see #149. It includes a test. I think that this is a safe change -- 2.7.15+foo would be a valid version, as would 2.7.15, so permitting 2.7.15+ to equal 2.7.15 seems OK to me.

Steps to reproduce (copy-pasting from #149)

  • Create any Pipfile and pipenv environment
  • Run pipenv install
  • Get a Python binary whose version number is 2.7.15+, as on Ubuntu 18.10
  • See a crash

I've tested that this fixes the issue for me.

Close #149

Before

Trying to run pyfinder --findall crashes.

  asheesh@obscura:/tmp/banana$ bin/pyfinder --findall
Traceback (most recent call last):
  File "bin/pyfinder", line 11, in <module>
    load_entry_point('pythonfinder', 'console_scripts', 'pyfinder')()
  File "/tmp/banana/local/lib/python2.7/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/tmp/banana/local/lib/python2.7/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/tmp/banana/local/lib/python2.7/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/tmp/banana/local/lib/python2.7/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/tmp/banana/local/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/tmp/banana/pythonfinder/src/pythonfinder/cli.py", line 33, in cli
    versions = [v for v in finder.find_all_python_versions()]
  File "/tmp/banana/local/lib/python2.7/site-packages/backports/functools_lru_cache.py", line 137, in wrapper
    result = user_function(*args, **kwds)
  File "/tmp/banana/pythonfinder/src/pythonfinder/pythonfinder.py", line 136, in find_all_python_versions
    major=major, minor=minor, patch=patch, pre=pre, dev=dev, arch=arch, name=name
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 328, in find_all_python_versions
    values = list(self.get_pythons(sub_finder))
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 279, in get_pythons
    reverse=True
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 277, in <genexpr>
    (p for p in self._filter_paths(finder) if p.is_python),
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 264, in <genexpr>
    pth for pth in unnest(finder(p) for p in self.path_entries if p is not None)
  File "/tmp/banana/pythonfinder/src/pythonfinder/utils.py", line 138, in unnest
    for el in target:
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 264, in <genexpr>
    pth for pth in unnest(finder(p) for p in self.path_entries if p is not None)
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/mixins.py", line 76, in find_all_python_versions
    path_filter = filter(None, (sub_finder(p) for p in self.children.values()))
  File "/tmp/banana/local/lib/python2.7/site-packages/cached_property.py", line 35, in __get__
    value = obj.__dict__[self.func.__name__] = self.func(obj)
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 494, in children
    for child_key, child_val in self._gen_children():
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 487, in _gen_children
    yield (child.as_posix(), PathEntry.create(path=child, **pass_args))
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 567, in create
    _new = cls(**creation_args)
  File "<attrs generated init b5a14ef20d54b43281ba439a6be98c24578e233d>", line 16, in __init__
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/path.py", line 510, in get_py_version
    py_version = PythonVersion.from_path(path=self, name=self.name)
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/python.py", line 418, in from_path
    instance_dict = cls.parse(py_version.strip())
  File "/tmp/banana/pythonfinder/src/pythonfinder/models/python.py", line 366, in parse
    raise ValueError("Not a valid python version: %r" % version)
ValueError: Not a valid python version: <LegacyVersion('2.7.15+')>

After

Running pyfinder --findall treats 2.7.15+ as 2.7.15.

asheesh@obscura:/tmp/banana$ bin/pyfinder --findall
Found python at the following locations:
python3.6: 3.6.6 (None) @ /usr/bin/python3.6
python3.6m: 3.6.6 (None) @ /usr/bin/python3.6m
python2.7: 2.7.15 (None) @ /usr/bin/python2.7

@di
Copy link
Member

di commented Nov 16, 2018

A version like 2.7.15+ is not a valid version identifier, per PEP 440:

Local version labels MUST start and end with an ASCII letter or digit.

@paulproteus
Copy link
Author

Thanks @di for the feedback -- good point!

This is resolved to my satisfaction within pythonfinder -- see sarugaku/pythonfinder#42 -- so I'll close this now. Cheers!

@paulproteus paulproteus deleted the issue-149 branch November 19, 2018 05:24
@svenpanne
Copy link

A version like 2.7.15+ is not a valid version identifier, per PEP 440:

Local version labels MUST start and end with an ASCII letter or digit.

As correct as this statement may be, it is highly irrelevant: Ubuntu is one of the most widely used distros (depending on the survey even the most widely used one), and the current cosmic release ships with a Python claiming a version of 2.7.15+, so the PyPA tools should better deal with that fact. Perhaps an issue can be opened for the Ubuntu package maintainers, but this won't change the current state of affairs.

@di
Copy link
Member

di commented Nov 19, 2018

This API is used to parse versions of Python packages, not versions of Python itself.

It was being misused downstream, but has been fixed, as @paulproteus mentioned.

@svenpanne
Copy link

Hmmm, then I misunderstood things, sorry. But note that sarugaku/pythonfinder#42 still misuses the API then, it just add more confusing code around that, trying to handle more stuff, but that doesn't work: "2.7.15+" is parsed as a LegacyVersion, so the attempted fix is not executed at all...

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

Successfully merging this pull request may close these issues.

Accept empty string as local identifier (e.g. 1.0+ == 1.0)
3 participants