-
Notifications
You must be signed in to change notification settings - Fork 211
Prefer $EBPYTHONPREFIXES
over $PYTHONPATH
#4496
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
Changes from 12 commits
0ec2df5
74a3cb3
03e304e
b07ea3f
8c66e1a
ccd3d7e
f76f320
8b5389f
cb6adb0
4a11f2f
c471e03
2676e4e
f4fbe76
92f0e62
e91186e
236c67f
ca304fb
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 |
---|---|---|
|
@@ -42,6 +42,7 @@ | |
from contextlib import contextmanager | ||
from easybuild.tools import LooseVersion | ||
from textwrap import wrap | ||
from typing import Union, Sequence | ||
|
||
from easybuild.base import fancylogger | ||
from easybuild.tools.build_log import EasyBuildError, print_warning | ||
|
@@ -205,63 +206,79 @@ def get_modules_path(self, fake=False, mod_path_suffix=None): | |
|
||
return os.path.join(mod_path, mod_path_suffix) | ||
|
||
def _filter_paths(self, key, paths): | ||
def _filter_paths(self, key: str, paths: list) -> list: | ||
Micket marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Filter out paths already added to key and return the remaining ones""" | ||
assert isinstance(paths, list) | ||
|
||
if self.added_paths_per_key is None: | ||
# For compatibility this is only a warning for now and we don't filter any paths | ||
print_warning('Module creation has not been started. Call start_module_creation first!') | ||
return paths | ||
|
||
added_paths = self.added_paths_per_key.setdefault(key, set()) | ||
# paths can be a string | ||
if isinstance(paths, str): | ||
if paths in added_paths: | ||
filtered_paths = None | ||
else: | ||
added_paths.add(paths) | ||
filtered_paths = paths | ||
else: | ||
# Coerce any iterable/generator into a list | ||
if not isinstance(paths, list): | ||
paths = list(paths) | ||
filtered_paths = [x for x in paths if x not in added_paths and not added_paths.add(x)] | ||
filtered_paths = [x for x in paths if x not in added_paths and not added_paths.add(x)] | ||
if filtered_paths != paths: | ||
removed_paths = paths if filtered_paths is None else [x for x in paths if x not in filtered_paths] | ||
print_warning("Suppressed adding the following path(s) to $%s of the module as they were already added: %s", | ||
Micket marked this conversation as resolved.
Show resolved
Hide resolved
|
||
key, removed_paths, | ||
key, ', '.join(removed_paths), | ||
log=self.log) | ||
if not filtered_paths: | ||
filtered_paths = None | ||
return filtered_paths | ||
|
||
def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True): | ||
def append_paths(self, key: str, paths: Union[str, Sequence[str]], allow_abs=False, expand_relpaths=True) -> str: | ||
""" | ||
Generate append-path statements for the given list of paths. | ||
|
||
:param key: environment variable to append paths to | ||
:param paths: list of paths to append | ||
:param paths: single or list of paths to append | ||
:param allow_abs: allow providing of absolute paths | ||
:param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) | ||
""" | ||
paths = self._filter_paths(key, paths) | ||
if paths is None: | ||
return '' | ||
return self.update_paths(key, paths, prepend=False, allow_abs=allow_abs, expand_relpaths=expand_relpaths) | ||
|
||
def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True): | ||
def prepend_paths(self, key: str, paths: Union[str, Sequence[str]], allow_abs=False, expand_relpaths=True) -> str: | ||
""" | ||
Generate prepend-path statements for the given list of paths. | ||
|
||
:param key: environment variable to append paths to | ||
:param paths: list of paths to append | ||
:param paths: single or list of paths to prepend | ||
:param allow_abs: allow providing of absolute paths | ||
:param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) | ||
""" | ||
paths = self._filter_paths(key, paths) | ||
if paths is None: | ||
return '' | ||
return self.update_paths(key, paths, prepend=True, allow_abs=allow_abs, expand_relpaths=expand_relpaths) | ||
|
||
def update_paths(self, key: str, paths: Union[str, Sequence[str]], prepend=True, allow_abs=False, | ||
expand_relpaths=True) -> str: | ||
""" | ||
Generate append/prepend-path statements for the given list of paths. | ||
|
||
:param key: environment variable to append paths to | ||
:param paths: single or list of paths to add | ||
:param allow_abs: allow providing of absolute paths | ||
:param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) | ||
""" | ||
if isinstance(paths, str): | ||
self.log.debug("Wrapping %s into a list before using it for %s", paths, key) | ||
paths = [paths] | ||
elif not isinstance(paths, list): | ||
paths = list(paths) # coerce any iterator into a list | ||
|
||
if key == 'PYTHONPATH': | ||
python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] | ||
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. Checking the logic in This means it this point Similar to my previous reasoning about breaking change in favor of cleaner code: I would simply disallow passing anything but a a string or a list to Quick example:
Micket marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if len(python_paths) > 1: | ||
raise EasyBuildError('Found multiple paths for PYTHONPATH: ' + ', '.join(python_paths)) | ||
|
||
# Special condition for PYTHONPATHs that match the standard pattern, | ||
# replace with EBPYTHONPREFIXES which is added to python sys path at runtime via sitecustomize.py | ||
if python_paths and build_option('replace_pythonpath'): | ||
python_path = python_paths[0] | ||
self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIXES", python_path) | ||
ret = self._update_paths('EBPYTHONPREFIXES', [''], prepend, allow_abs, expand_relpaths) | ||
paths = [path for path in paths if path != python_path] | ||
if paths: | ||
ret += self._update_paths(key, paths, prepend, allow_abs, expand_relpaths) | ||
return ret | ||
return self._update_paths(key, paths, prepend, allow_abs, expand_relpaths) | ||
|
||
def _modulerc_check_module_version(self, module_version): | ||
""" | ||
Check value type & contents of specified module-version spec. | ||
|
@@ -551,12 +568,12 @@ def unload_module(self, mod_name): | |
""" | ||
raise NotImplementedError | ||
|
||
def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): | ||
def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: | ||
Micket marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
Generate prepend-path or append-path statements for the given list of paths. | ||
|
||
:param key: environment variable to prepend/append paths to | ||
:param paths: list of paths to prepend | ||
:param paths: list of paths to add | ||
:param prepend: whether to prepend (True) or append (False) paths | ||
:param allow_abs: allow providing of absolute paths | ||
:param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) | ||
|
@@ -955,16 +972,18 @@ def msg_on_unload(self, msg): | |
print_cmd = "puts stderr %s" % quote_str(msg, tcl=True) | ||
return '\n'.join(['', self.conditional_statement("module-info mode unload", print_cmd, indent=False)]) | ||
|
||
def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): | ||
def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: | ||
""" | ||
Generate prepend-path or append-path statements for the given list of paths. | ||
|
||
:param key: environment variable to prepend/append paths to | ||
:param paths: list of paths to prepend | ||
:param paths: list of paths to add | ||
:param prepend: whether to prepend (True) or append (False) paths | ||
:param allow_abs: allow providing of absolute paths | ||
:param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) | ||
""" | ||
paths = self._filter_paths(key, paths) | ||
|
||
if prepend: | ||
update_type = 'prepend' | ||
else: | ||
|
@@ -974,10 +993,6 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath | |
self.log.info("Not including statement to %s environment variable $%s, as specified", update_type, key) | ||
return '' | ||
|
||
if isinstance(paths, str): | ||
self.log.debug("Wrapping %s into a list before using it to %s path %s", paths, update_type, key) | ||
paths = [paths] | ||
|
||
abspaths = [] | ||
for path in paths: | ||
if os.path.isabs(path) and not allow_abs: | ||
|
@@ -1426,16 +1441,18 @@ def modulerc(self, module_version=None, filepath=None, modulerc_txt=None): | |
return super(ModuleGeneratorLua, self).modulerc(module_version=module_version, filepath=filepath, | ||
modulerc_txt=modulerc_txt) | ||
|
||
def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): | ||
def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: | ||
Micket marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
Generate prepend_path or append_path statements for the given list of paths | ||
|
||
:param key: environment variable to prepend/append paths to | ||
:param paths: list of paths to prepend/append | ||
:param paths: list of paths to add | ||
:param prepend: whether to prepend (True) or append (False) paths | ||
:param allow_abs: allow providing of absolute paths | ||
:param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) | ||
""" | ||
paths = self._filter_paths(key, paths) | ||
|
||
if prepend: | ||
update_type = 'prepend' | ||
else: | ||
|
@@ -1445,10 +1462,6 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath | |
self.log.info("Not including statement to %s environment variable $%s, as specified", update_type, key) | ||
return '' | ||
|
||
if isinstance(paths, str): | ||
self.log.debug("Wrapping %s into a list before using it to %s path %s", update_type, paths, key) | ||
paths = [paths] | ||
|
||
abspaths = [] | ||
for path in paths: | ||
if os.path.isabs(path): | ||
|
Uh oh!
There was an error while loading. Please reload this page.