Fix #191: improve handling of dependencies #201
Conversation
- use one pip call to install dependencies. It allows to correctly handle constraints. - when package is outside of constraints, perform clean installation. Clean installation in most cases is not necessary but with --target it has strange corner cases. - commit semver dependency with code
| if requires_reinstall: | ||
| print(f"INFO: dependencies reuqire update because of {requires_reinstall}, reinstalling...") | ||
| print(f'INFO: removing "{bpy.utils.user_resource("SCRIPTS", path="modules")}"') | ||
| shutil.rmtree(bpy.utils.user_resource("SCRIPTS", path="modules")) |
There was a problem hiding this comment.
This carries a low risk of removing user data... The only think that can be done is to move the dir to backup location or track exactly what files are being installed.
There was a problem hiding this comment.
What makes you think that the risk is low? Sounds like removing a folder that is not specific to the extension is quite risky.
May be better to tell the user in some way that the folder should be removed instead of just doing it.
There was a problem hiding this comment.
i don't think this folder is used often.
The alternative is to track all names of dependencies and remove folder and .dist-info folder. Like jinja and ^jinja.*-.*\.dist-info$. A static list of dependencies is easiest to write.
just a note for myself: I need to further reduce version of wergzeug as 3.0.3 supports python >=3.8
There was a problem hiding this comment.
made a quick implementation, might still have mistakes
Some snipet with alternative:
def list_installed_packages():
target = get_package_install_directory()
command = [str(python_path), "-m", "pip", "freeze", "--disable-pip-version-check", "--exclude-editable", "---path", target]
print("INFO: execute: ", " ".join(_escape_space(c) for c in command))
output = subprocess.check_output(command, cwd=_CWD_FOR_SUBPROCESSES).decode()
for package in output.splitlines():
yield package.split("==")[-1]
| if requires_reinstall: | ||
| print(f"INFO: dependencies reuqire update because of {requires_reinstall}, reinstalling...") | ||
| print(f'INFO: removing "{bpy.utils.user_resource("SCRIPTS", path="modules")}"') | ||
| shutil.rmtree(bpy.utils.user_resource("SCRIPTS", path="modules")) |
There was a problem hiding this comment.
What makes you think that the risk is low? Sounds like removing a folder that is not specific to the extension is quite risky.
May be better to tell the user in some way that the folder should be removed instead of just doing it.
| real_version = Version.parse(package_version(name)) | ||
| assert isinstance(requested_version, Version), requested_version | ||
| assert isinstance(real_version, Version), real_version | ||
| if "==" in name and not (real_version == requested_version): |
There was a problem hiding this comment.
Is there no builtin library that helps with these kinds of checks? It feels a bit weird to have to implement it manually.
There was a problem hiding this comment.
we are stepping into the boots of pip. I am forcing pip to do very specific things to my dependencies:
- dont touch deps when they are in acceptable range and
- oh, thats it, nothing more.
If you read about the --target option you will see that it is not well supported...
The typical way of using pip is lock file or requirement file. Lock file is way to restrictive in our case, and if you use requirement file with -U you will force everyone to upgrade.
Sooo... yes, it is messy. I am not saying it is the best, this is what I came up with
This PR will allow for update/downgrade of dependencies when needed.
pip install --upgradeyou can only downgrade package...This PR should be pretty complete, can not think of anything specific to test.