-
Notifications
You must be signed in to change notification settings - Fork 24
Chore: Add path to PYTHONPATH in correct order #282
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
base: develop
Are you sure you want to change the base?
Changes from 4 commits
2f966ee
bdb68b3
b482afe
5e4bdf1
2f4a9a7
3ea3f52
cb1fba8
6ace6aa
ca718c9
f096eff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,7 @@ | |
| import traceback | ||
| import subprocess | ||
| from contextlib import contextmanager | ||
| from pathlib import Path | ||
| from urllib.parse import urlencode, urlparse, parse_qs | ||
|
|
||
| from version import __version__ | ||
|
|
@@ -377,6 +378,8 @@ def _print(message: str): | |
| from ayon_common.distribution import ( # noqa E402 | ||
| AYONDistribution, | ||
| BundleNotFoundError, | ||
| get_addons_dir, | ||
| get_dependencies_dir, | ||
| show_missing_bundle_information, | ||
| show_blocked_auto_update, | ||
| show_missing_permissions, | ||
|
|
@@ -799,16 +802,37 @@ def _start_distribution(): | |
| os.environ["AYON_STUDIO_BUNDLE_NAME"] = studio_bundle_name | ||
|
|
||
| # TODO probably remove paths to other addons? | ||
| python_paths = [ | ||
| path | ||
| for path in os.getenv("PYTHONPATH", "").split(os.pathsep) | ||
| if path | ||
| ] | ||
| 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) | ||
|
Comment on lines
812
to
837
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate a bit on the what and how? :) Why does it need to be before the dependencies dir path?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It removes all paths in PYTHONPATH that leads to addons dir or dependecy packages dir. If there was dependency package, then remember it's position, and replace it with current dependency package dir path. |
||
|
|
||
| for path in distribution.get_python_paths(): | ||
| for path in addon_paths: | ||
| sys.path.insert(0, path) | ||
| if path not in python_paths: | ||
| python_paths.append(path) | ||
| python_paths.insert(0, path) | ||
|
|
||
| for path in distribution.get_sys_paths(): | ||
| sys.path.insert(0, path) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced the
idxhere 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 inpython_pathslist would not match it? Because we're essentially 'inserting it' at the newpython_pathsthat we're generating 🤔Why do need the whole
idxtracking, 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.appenddirectly here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my perspective we should always prepend it... But you said it is not good idea 🙂