-
Notifications
You must be signed in to change notification settings - Fork 133
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
Better fallbacks #177
base: master
Are you sure you want to change the base?
Better fallbacks #177
Conversation
Most importantly, this commit adds a fallback for when no package_info.json file exists, as is the case for the pipsi venv when installed through get-pipsi.py. I also moved all the fallback code to get_package_info. In doing so, I improved the list_everything fallbacks which prior to this change gave missing data (like None for version). I also believe that putting all the fallback code in one place will make it easier to maintain. Additionally, I changed list_everything to actually make use of the name stored in package_info, which was currently unused. There are probably other places that could make use of it too, since if all the code just goes by the venv name, the name data might as well not even exist.
Call it as a method, also convert from generator to list.
@benburrill can you see if #161 solves the same issue? |
I haven't tested it, but it seems to have a similar goal. One difference I see is that my PR will automatically fill in missing package info even if the file exists, whereas #161 mostly just falls back to A even better solution than either PR might be to get rid of |
We share the same goal of fixing the packaging_info.json compatibility and not existing problem. Actually #161 doesn't fall back to None if that file does not exists, in whatever case, a PackageInfo instance will always be created, either read from the json file, or infer from the environment info. Please take a look at this two commit messages :) |
This was also my first thought after finding out its original purpose (#78). But then I considered some other functionalities, like I want pipsi to install different versions of same package at the same time, if this is achieved, then directory name would not be simply seen as the package name, at that time package_info.json should be useful for storing the meta data of package info of the directory it lies in. So instead of removing it I chose to enhance it for future use (hopefully) :) |
I think you misunderstand. I know that your PackageInfo class falls back appropriately when the file does not exist. My concern (and it could be argued that this isn't such a big deal) is that when the file does exist,
I know this is just an example, but would that even be useful? Since the point of pipsi is to install scripts, the scripts from one version of the package would shadow scripts from the other version. |
Since get-pipsi.py doesn't create a
package_info.json
file,pipsi list
will currently result in an error when pipsi is installed from master. This PR fixes that bug, as well as generally improving pipsi's package_info fallbacks. See 749c46c for more info.I didn't bother to make get-pipsi.py actually create the package info file, but it might make sense to do that as well at some point.