Chore: Add path to PYTHONPATH in correct order#282
Chore: Add path to PYTHONPATH in correct order#282
Conversation
|
Does this enforce our dependencies in front of PYTHONPATH? or which python paths are enforcing at the start here? Because we'll want to make sure that a studio can still sneak in e.g. Houdini's own libs at start of Or better, if AYON application enviroments or tool environments prepend something - I do want to actually get whatever I'm prepending before the AYON vars 🤔 |
These are addons and dependency package dependencies. |
Would they still go after app envs in the final resolution? Because the whole |
Didn't think about it, true. In that case we probably should replace existing addons paths instead. |
Didn't we technically solve it with the change in core, or what is still the issue this is trying to solve? |
We had to fix it there because this is wrong... This is the source of the issue. We should not keep e.g. |
| addons_dir = Path(get_addons_dir()) | ||
| dependencies_dir = Path(get_dependencies_dir()) | ||
| dep_dir_idx = None | ||
| python_paths = [] | ||
| for idx, path in enumerate(os.getenv("PYTHONPATH", "").split(os.pathsep)): | ||
| if not path: | ||
| continue | ||
|
|
||
| p_path = Path(path) | ||
| # Ignore addons | ||
| if p_path.is_relative_to(addons_dir): | ||
| continue | ||
|
|
||
| # Ignore dependencies dir and store index of the path | ||
| if p_path.is_relative_to(dependencies_dir): | ||
| if dep_dir_idx is None: | ||
| dep_dir_idx = idx | ||
| continue | ||
| python_paths.append(path) | ||
|
|
||
| addon_paths, dep_package_path = distribution.get_python_paths() | ||
|
|
||
| sys.path.insert(0, dep_package_path) | ||
| if dep_dir_idx is not None: | ||
| python_paths.insert(dep_dir_idx, dep_package_path) | ||
| else: | ||
| python_paths.append(dep_package_path) |
There was a problem hiding this comment.
Can you elaborate a bit on the what and how? :) Why does it need to be before the dependencies dir path?
There was a problem hiding this comment.
It removes all paths in PYTHONPATH that leads to addons dir or dependecy packages dir.
(ignore paths %LOCALAPPDATA%/Ynput/AYON/addons/* & %LOCALAPPDATA%/Ynput/AYON/dependency_packages/*)
If there was dependency package, then remember it's position, and replace it with current dependency package dir path.
start.py
Outdated
| for idx, path in enumerate(os.getenv("PYTHONPATH", "").split(os.pathsep)): | ||
| if not path: | ||
| continue | ||
|
|
||
| p_path = Path(path) | ||
| # Ignore addons | ||
| if p_path.is_relative_to(addons_dir): | ||
| continue | ||
|
|
||
| # Ignore dependencies dir and store index of the path | ||
| if p_path.is_relative_to(dependencies_dir): | ||
| if dep_dir_idx is None: | ||
| dep_dir_idx = idx | ||
| continue | ||
| python_paths.append(path) |
There was a problem hiding this comment.
I'm not convinced the idx here will work, or at least in any meaningful way, because the idx will be shifted wrong due to some paths being skipped here while the idx counter continues forward. As such, the index in python_paths list would not match it? Because we're essentially 'inserting it' at the new python_paths that we're generating 🤔
Why do need the whole idx tracking, seems redundant/wrong? Seems we want to put in same order - but since we're building the full list - doesn't that just mean to .append directly here?
There was a problem hiding this comment.
From my perspective we should always prepend it... But you said it is not good idea 🙂
2 participants
Changelog Description
Make sure to prepend the paths to PYTHONPATH.
Additional info
We're addin them with correct order in
sys.pathwhich works for AYON launcher process but any child process that is not AYON launcher process will usePYTHONPATHwhich might have paths from some of previous processes.Testing notes:
Not sure how to test it, we discovered this issue during project bundles testing which was fixed with ynput/ayon-core#1710 .