justinmayer / virtualfish

Fish shell tool for managing Python virtual environments
MIT License
1.06k stars 100 forks source link

VirtualFish requires Fish 3.1 or higher. Current version: 3.1.2-699-ga9bfe7f16 #177

Closed zx8 closed 4 years ago

zx8 commented 4 years ago

Caused by https://github.com/justinmayer/virtualfish/pull/171


Issue

I use the master version of fish and update it every few days. It seems the new version check does not work for versions of fish installed this way.

>>> from packaging import version
>>> print(version.parse("3.1") < version.parse("3.1.2-699-ga9bfe7f16"))
False
justinmayer commented 4 years ago

Thanks for the heads-up. I would have thought the version parser would be smarter than that. In any case, adding the following line to the version comparison stanza should fix this problem:

fish_version = fish_version.partition("-")[0]

I'll issue a patched release in a day or two.

zx8 commented 4 years ago

Thanks for the speedy fix!

Any chance you could cut a new release so I can update?

justinmayer commented 4 years ago

@zx8: Sorry for the delay. I included this fix in the just-released VirtualFish 2.2.4.

zx8 commented 4 years ago

@justinmayer I'm still getting a warning using VirtualFish v2.3.0...

WARNING: VirtualFish requires Fish 3.1.0 or higher. Current version:
3.1.2

If the version I'm using is sufficient, I don't see the need for a warning at all?

justinmayer commented 4 years ago

Well, that's just it — as you can see via the relevant Python code, you shouldn't see a warning if you are using Fish 3.1+.

You didn't mention what you did to trigger that warning. I assume it was vf install?

Perhaps the version check logic is faulty in some way, but I can't reproduce the results you are seeing. What do you see when you do the following steps I just did to test the logic? Does it help you understand where the logic is falling down?

~/P/virtualfish on master ➤ workon virtualfish
(virtualfish) ~/P/virtualfish on master ➤ python
Python 3.7.8 (default, Jul  7 2020, 15:09:35)
>>> import subprocess
>>> from packaging import version
>>> minimum_fish_version = "3.1.0"
>>> cmd = ["fish", "-c", "echo $version"]
>>> fish_version = subprocess.check_output(cmd).decode("utf-8").strip()
>>> print(fish_version)
3.1.2
>>> fish_version = fish_version.partition("-")[0]
>>> print(fish_version)
3.1.2
>>> if version.parse(fish_version) < version.parse(minimum_fish_version):
...             log.warning(
...                 "{}WARNING: VirtualFish requires Fish {} or higher. Current version: {}{}".format(
...                 vcolors.RED, minimum_fish_version, fish_version, vcolors.NORMAL))
...
>>> # [JM: No warning was shown here because the installed version is above the minimum.]
zx8 commented 4 years ago
>>> import subprocess
>>> from packaging import version
>>> minimum_fish_version = "3.1.0"
>>> cmd = ["fish", "-c", "echo $version"]
>>> fish_version = subprocess.check_output(cmd).decode("utf-8").strip()
>>> print(fish_version)
3.1.2-999-g8d25ed962
>>> fish_version = fish_version.partition("-")[0]
>>> print(fish_version)
3.1.2
>>> if version.parse(fish_version) < version.parse(minimum_fish_version):
...   print("foo")
...
foo

I should note I'm building fish from master every few days to help test the latest (unreleased) versions, so I don't have a fixed point release installed.

Running version.parse(fish_version) on its own reveals a bunch of weird/hidden characters:

>>> version.parse(fish_version)
<LegacyVersion('\x1b[3g\x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH    \x1bH\r3.1.2')>

Have raised this @ https://github.com/fish-shell/fish-shell/issues/7181 for assistance.

Edit: Figured it out, see https://github.com/fish-shell/fish-shell/issues/7181#issuecomment-654957648.