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

Restricting newest jedi version allowed (and making it possible to configure this requirement) #402

Open
Mekk opened this issue Dec 31, 2020 · 3 comments

Comments

@Mekk
Copy link

Mekk commented Dec 31, 2020

Context: everybody who fresh-installed anaconda-mode since December 25h (jedi 0.18.0 release) gets broken installation (see #401 )

At the moment, anaconda-mode installs needed python modules with loose requirement, for example jedi is installed with jedi>=0.13.0. Either exact tested version should be specified, or range with two-side restrictions (say jedi>=0.13.0,<0.18.0).

Preferably, some customizable variable could let one patch this requirement if necessary (anaconda-jedi-version…)

code-wise it's about constructing missing_dependencies variable.

@Mekk
Copy link
Author

Mekk commented Dec 31, 2020

Code-wise: the responsible lines are
https://github.com/pythonic-emacs/anaconda-mode/blob/master/anaconda-mode.el#L116
and
https://github.com/pythonic-emacs/anaconda-mode/blob/master/anaconda-mode.el#L157
and
https://github.com/pythonic-emacs/anaconda-mode/blob/master/anaconda-mode.el#L172

If it's to stay more-or-less this way, I'd suggest specyfying both sides, sth like

jedi_dep = ('jedi', '0.13.0', '0.18')   # min req. version, min illegal version
# …
missing_dependencies.append('{0}>={1},<{2}'.format(*jedi_dep))
# …
assert jedi.__version__ >= jedi_dep[1] and jedi.__version__ < jedi_dep[2], 'Jedi version should be >= %s and < %s, current version: %s' % (jedi_dep[1], jedi_dep[2], jedi.__version__,)

except the assert is invalid (text comparison doesn't work properly here, one must compare versions numerically, there are various libraries which can do it, I am not sure which one is appropriate here)

dakra added a commit to dakra/anaconda-mode that referenced this issue Jan 1, 2021
Fix compatibility issues with jedi version 0.18.
Require specific version of jedi and service_factory.
Fall back to jedi 0.17.2 on python 2.

See https://jedi.readthedocs.io/en/latest/docs/changelog.html

Fix pythonic-emacs#401
Related pythonic-emacs#402
@Mekk
Copy link
Author

Mekk commented Jan 7, 2021

Patch above solves most important part of the issue, by forcing specific jedi version when one is to be installed. Still, there are two minor issues to consider:

a) Patched version switches to strict version (installs jedi==0.17.2 or jedi==0.18.0 depending on python version). Mayhaps it would make sense to allow for patchnumber upgrade (say, to install 0.17.3/0.18.1 if such version is released). Not 100% sure though.

b) Patch does not properly fix assert statement, leaving it as

assert jedi.__version__ >= jedi_dep[1], 'Jedi version should be >= %s, current version: %s' % (jedi_dep[1], jedi.__version__,)

(

assert jedi.__version__ >= jedi_dep[1], 'Jedi version should be >= %s, current version: %s' % (jedi_dep[1], jedi.__version__,)
)
The problem is that in case very old jedi (say 0.8.0) is installed, this assert will be glad to accept such version without complaints, as alphabetically "0.8.0" >= "0.18.0").
I am not sure about this part, mayhaps it should be dropped altogether or mayhaps instead of failing anaconda should upgrade to proper version, but if it is to stay, alphabetical comparison is incorrect. Sth like distutils.version.LooseVersion(jedi.__version__) >= distutils.version.LooseVersion(jedi_dep[1]) or mayhaps packaging.version.LegacyVersion, or even simple twoliner

def versiontuple(v):
    return tuple(int(x) for x in v.split("."))

dakra added a commit to dakra/anaconda-mode that referenced this issue Jan 8, 2021
Fix compatibility issues with jedi version 0.18.
Require specific version of jedi and service_factory.
Fall back to jedi 0.17.2 on python 2.

See https://jedi.readthedocs.io/en/latest/docs/changelog.html

Fix pythonic-emacs#401
Related pythonic-emacs#402
@dakra
Copy link
Member

dakra commented Jan 8, 2021

Thanks for you comment.

I went with the distutils version as that's the most stable.
Your tuple function would e.g. break with "0.18a" etc.

I'm leaving this ticket open for now as I also think it would be
nice to at least automatically update the patch version or
make it configurable what jedi version should be installed.
(PRs welcome ;) )

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

2 participants