virtualfish
virtualfish copied to clipboard
vf upgrade can sometimes fail
-
[x] I am using Fish shell version 3.1 or higher.
-
[x] I am using Python version 3.6 or higher.
-
[x] I have searched the issues (including closed ones) and believe that this is not a duplicate.
-
[x] If related to a plugin, I prefixed the issue title with the name of the plugin.
-
OS version and name: Arch
-
Fish shell version: 3.3.1
-
VirtualFish version: 2.5.4
Issue
While upgrading some venvs for Python 3.10, one of them caused virtualfish to spew fish error traces due to pip somehow having gone missing. Not sure how it ended up like that (whether it was due to vf or not), but it seems like a good idea to add a check that the command exists before we run it.
https://github.com/justinmayer/virtualfish/pull/218#issuecomment-995066782
A few lines up from the proposed change, the following line invokes the
__vfsupport_check_python --pip
function precisely for this purpose, checking to see whether the selected virtual environment's Python and Pip are behaving correctly.__vfsupport_check_python --pip "$venv_path/bin/python"
Taking another look at the existing code in this section of the
__vf_upgrade
function, the following thoughts occurred to me:1. There are currently two bare `pip` invocations that should probably be replaced with `python -m pip`. Let's assume for the moment that I will fix that. 2. If the `__vfsupport_check_python --pip` check fails and we can't determine which packages are installed by looking at the virtual environment's `[…].dist-info` directory names, we fall back to running `python -m pip freeze` even though we've already determined that `python -m pip` is broken, which doesn't make a lot of sense? That could be one possible reason that the check passed but then you later encountered an error.
I'm not sure what to do about the latter. The intention is to rebuild the virtual environment along with the packages that were previously installed, and if it's not possible to determine which packages were installed, "rebuilding" would instead result in merely creating a new virtual environment with no packages installed. I suppose that's okay if a warning is printed to that effect, but I want to avoid situations in which users think a virtual environment was successfully rebuilt with all previously-installed packages but instead later find out that it is empty.