Skip to content
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

enable download_dep_fail, use_pip, sanity_pip_check by default in PythonPackage easyblock #3022

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 1 addition & 22 deletions easybuild/easyblocks/e/easybuildmeta.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import sys
from collections import OrderedDict

from easybuild.easyblocks.generic.pythonpackage import PythonPackage, det_pip_version
from easybuild.easyblocks.generic.pythonpackage import PythonPackage
from easybuild.tools import LooseVersion
from easybuild.tools.build_log import EasyBuildError
from easybuild.tools.filetools import apply_regex_substitutions, change_dir, read_file
Expand Down Expand Up @@ -66,27 +66,6 @@ def __init__(self, *args, **kwargs):
# consider setuptools first, in case it is listed as a sources
self.easybuild_pkgs.insert(0, 'setuptools')

# opt-in to using pip for recent version of EasyBuild, if:
# - EasyBuild is being installed for Python >= 3.6;
# - pip is available, and recent enough (>= 21.0);
# - use_pip is not specified;
pyver = sys.version.split(' ')[0]
self.log.info("Python version: %s", pyver)
if sys.version_info >= (3, 6) and self.cfg['use_pip'] is None:
# try to determine pip version, ignore any failures that occur while doing so;
# problems may occur due changes in environment ($PYTHONPATH, etc.)
pip_version = None
try:
pip_version = det_pip_version(python_cmd=sys.executable)
self.log.info("Found Python v%s + pip: %s", pyver, pip_version)
except Exception as err:
self.log.warning("Failed to determine pip version: %s", err)

if pip_version and LooseVersion(pip_version) >= LooseVersion('21.0'):
self.log.info("Auto-enabling use of pip to install EasyBuild!")
self.cfg['use_pip'] = True
self.determine_install_command()

# Override this function since we want to respect the user choice for the python installation to use
# (which can be influenced by EB_PYTHON and EB_INSTALLPYTHON)
def prepare_python(self):
Expand Down
3 changes: 0 additions & 3 deletions easybuild/easyblocks/generic/pythonbundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ def __init__(self, *args, **kwargs):
if key not in self.cfg['exts_default_options']:
self.cfg['exts_default_options'][key] = self.cfg[key]

self.cfg['exts_default_options']['download_dep_fail'] = True
self.log.info("Detection of downloaded extension dependencies is enabled")

self.log.info("exts_default_options: %s", self.cfg['exts_default_options'])

self.pylibdir = None
Expand Down
16 changes: 8 additions & 8 deletions easybuild/easyblocks/generic/pythonpackage.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def extra_options(extra_vars=None):
"Otherwise it will be used as-is. A value of None then skips the build step. "
"The template %(python)s will be replace by the currently used Python binary.", CUSTOM],
'check_ldshared': [None, 'Check Python value of $LDSHARED, correct if needed to "$CC -shared"', CUSTOM],
'download_dep_fail': [None, "Fail if downloaded dependencies are detected", CUSTOM],
'download_dep_fail': [True, "Fail if downloaded dependencies are detected", CUSTOM],
'install_src': [None, "Source path to pass to the install command (e.g. a whl file)."
"Defaults to '.' for unpacked sources or the first source file specified", CUSTOM],
'install_target': ['install', "Option to pass to setup.py", CUSTOM],
Expand All @@ -348,8 +348,8 @@ def extra_options(extra_vars=None):
"Enabled by default if the EB option --debug is used.", CUSTOM],
'req_py_majver': [None, "Required major Python version (only relevant when using system Python)", CUSTOM],
'req_py_minver': [None, "Required minor Python version (only relevant when using system Python)", CUSTOM],
'sanity_pip_check': [False, "Run 'python -m pip check' to ensure all required Python packages are "
"installed and check for any package with an invalid (0.0.0) version.", CUSTOM],
'sanity_pip_check': [True, "Run 'python -m pip check' to ensure all required Python packages are "
"installed and check for any package with an invalid (0.0.0) version.", CUSTOM],
'runtest': [True, "Run unit tests.", CUSTOM], # overrides default
'testinstall': [False, "Install into temporary directory prior to running the tests.", CUSTOM],
'unpack_sources': [None, "Unpack sources prior to build/install. Defaults to 'True' except for whl files",
Expand All @@ -358,7 +358,7 @@ def extra_options(extra_vars=None):
# version. Those would fail the (extended) sanity_pip_check. So as a last resort they can be added here
# and will be excluded from that check. Note that the display name is required, i.e. from `pip list`.
'unversioned_packages': [[], "List of packages that don't have a version at all, i.e. show 0.0.0", CUSTOM],
'use_pip': [None, "Install using '%s'" % PIP_INSTALL_CMD, CUSTOM],
'use_pip': [True, "Install using '%s'" % PIP_INSTALL_CMD, CUSTOM],
'use_pip_editable': [False, "Install using 'pip install --editable'", CUSTOM],
# see https://packaging.python.org/tutorials/installing-packages/#installing-setuptools-extras
'use_pip_extras': [None, "String with comma-separated list of 'extras' to install via pip", CUSTOM],
Expand Down Expand Up @@ -427,7 +427,7 @@ def determine_install_command(self):
"""
Determine install command to use.
"""
if self.cfg.get('use_pip', False) or self.cfg.get('use_pip_editable', False):
if self.cfg.get('use_pip', True) or self.cfg.get('use_pip_editable', False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this duplication of the default params in EB 5.x or could we just switch to self.cfg['use_pip'] and fix EasyBlocks not "inheriting" the options this block requires?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's wise to keep using self.cfg.get, to avoid hard crashes in easyblock to derive from PythonPackage but do not inherit the custom easyconfig parameters from PythonPackage.
We can make sure the easyblocks we have in the central repository do so, but we don't control all easyblocks out there, and it's a small effort to avoid fallout...

The self.cfg.get is a pattern that we should use in all generic easyblocks (and even some custom ones) since they may be used as a base for other easyblocks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that motivation for EB 4.x but I wanted to suggest that such a breakage could be OK for EB 5.x. Not inheriting the parameters has several other implications such as not being able to list available parameters correctly. This introduces some kind of "hidden" parameters and puts burden on upstream EB to be careful to always use the self.cfg.get pattern and deal with potential different default values. Technically each instance of those self.cfg.get calls would need a check against the extra_parameters section that the default matches and that possibly even across easyblocks (some descendant of e.g. PythonPackage also using self.cfg.get('use_pip' would possibly need to be updated too)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Flamefire It's a good point, and I'm happy to follow up on it, but we should do that consistently then, so let's open an issue on this.

The window of opportunity w.r.t. (potentially) breaking changes to include in EasyBuild v5.0 is slowly closing though...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree that it is a good point. Let's follow up on this topic in a separate issue or PR. I will merge this PR as is so we can continue testing the 5.0.x branch for EasyBuild v5.0.

self.install_cmd = PIP_INSTALL_CMD

pip_verbose = self.cfg.get('pip_verbose', None)
Expand All @@ -450,7 +450,7 @@ def determine_install_command(self):
self.py_installopts.append('--egg')

pip_no_index = self.cfg.get('pip_no_index', None)
if pip_no_index or (pip_no_index is None and self.cfg.get('download_dep_fail')):
if pip_no_index or (pip_no_index is None and self.cfg.get('download_dep_fail', True)):
self.py_installopts.append('--no-index')

# avoid that pip (ab)uses $HOME/.cache/pip
Expand Down Expand Up @@ -970,7 +970,7 @@ def sanity_check_step(self, *args, **kwargs):
# see also https://github.com/easybuilders/easybuild-easyblocks/issues/1877
env.setvar('PYTHONNOUSERSITE', '1', verbose=False)

if self.cfg.get('download_dep_fail', False):
if self.cfg.get('download_dep_fail', True):
self.log.info("Detection of downloaded depenencies enabled, checking output of installation command...")
patterns = [
'Downloading .*/packages/.*', # setuptools
Expand Down Expand Up @@ -1014,7 +1014,7 @@ def sanity_check_step(self, *args, **kwargs):
exts_filter = (orig_exts_filter[0].replace('python', self.python_cmd), orig_exts_filter[1])
kwargs.update({'exts_filter': exts_filter})

if self.cfg.get('sanity_pip_check', False):
if self.cfg.get('sanity_pip_check', True):
pip_version = det_pip_version(python_cmd=python_cmd)

if pip_version:
Expand Down
1 change: 0 additions & 1 deletion easybuild/easyblocks/j/jaxlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ def extra_options():
"""Custom easyconfig parameters specific to jaxlib."""
extra_vars = PythonPackage.extra_options()

extra_vars['use_pip'][0] = True
# Run custom build script and install the generated whl file
extra_vars['buildcmd'][0] = '%(python)s build/build.py'
extra_vars['install_src'][0] = 'dist/*.whl'
Expand Down
10 changes: 0 additions & 10 deletions easybuild/easyblocks/n/numexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@
class EB_numexpr(PythonPackage):
"""Support for building/installing numexpr."""

@staticmethod
def extra_options():
"""Override some custom easyconfig parameters specifically for numexpr."""
extra_vars = PythonPackage.extra_options()

extra_vars['download_dep_fail'][0] = True
extra_vars['use_pip'][0] = True

return extra_vars

def __init__(self, *args, **kwargs):
"""Initialisation of custom class variables for numexpr."""
super(EB_numexpr, self).__init__(*args, **kwargs)
Expand Down
9 changes: 0 additions & 9 deletions easybuild/easyblocks/p/pybind11.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,6 @@ class EB_pybind11(CMakePythonPackage):
Python packages using `import pybind11`
Hence we need to install PyBind11 twice: Once with CMake and once with pip
"""
@staticmethod
def extra_options(extra_vars=None):
"""Easyconfig parameters specific to PyBind11: Set defaults"""
extra_vars = PythonPackage.extra_options(extra_vars=extra_vars)
extra_vars = CMakeMake.extra_options(extra_vars=extra_vars)
extra_vars['use_pip'][0] = True
extra_vars['sanity_pip_check'][0] = True
extra_vars['download_dep_fail'][0] = True
return extra_vars

def configure_step(self):
"""Avoid that a system Python is picked up when a Python module is loaded"""
Expand Down
3 changes: 0 additions & 3 deletions easybuild/easyblocks/p/pytorch.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ def extra_options():
# see also https://github.com/easybuilders/easybuild-easyblocks/pull/3022
extra_vars['use_pip'][0] = None

extra_vars['download_dep_fail'][0] = True
extra_vars['sanity_pip_check'][0] = True

# Make pip show output of build process as that may often contain errors or important warnings
extra_vars['pip_verbose'][0] = True

Expand Down
9 changes: 0 additions & 9 deletions easybuild/easyblocks/s/sympy.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,6 @@
class EB_sympy(PythonPackage):
"""Custom easyblock for installing the sympy Python package."""

@staticmethod
def extra_options(extra_vars=None):
"""Customize default value for easyconfig parameters for sympy"""
extra_vars = PythonPackage.extra_options(extra_vars=extra_vars)
extra_vars['use_pip'][0] = True
extra_vars['sanity_pip_check'][0] = True
extra_vars['download_dep_fail'][0] = True
return extra_vars

def test_step(self):
"""Custom test step for sympy"""

Expand Down
2 changes: 0 additions & 2 deletions easybuild/easyblocks/t/tensorflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,6 @@ def __init__(self, *args, **kwargs):
with self.cfg.disable_templating():
self.cfg['exts_defaultclass'] = 'PythonPackage'

self.cfg['exts_default_options']['download_dep_fail'] = True
self.cfg['exts_default_options']['use_pip'] = True
self.cfg['exts_filter'] = EXTS_FILTER_PYTHON_PACKAGES

self.system_libs_info = None
Expand Down
5 changes: 0 additions & 5 deletions easybuild/easyblocks/t/tensorrt.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ def __init__(self, *args, **kwargs):
# Setup for the extensions step
self.cfg['exts_defaultclass'] = 'PythonPackage'

self.cfg['exts_default_options'] = {
'download_dep_fail': True,
'use_pip': True,
}

def configure_step(self):
"""Custom configuration procedure for TensorRT."""
pass
Expand Down
9 changes: 0 additions & 9 deletions easybuild/easyblocks/t/torchvision.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,6 @@
class EB_torchvision(PythonPackage):
"""Support for building/installing TorchVison."""

@staticmethod
def extra_options():
"""Change some defaults for easyconfig parameters."""
extra_vars = PythonPackage.extra_options()
extra_vars['use_pip'][0] = True
extra_vars['download_dep_fail'][0] = True
extra_vars['sanity_pip_check'][0] = True
return extra_vars

def __init__(self, *args, **kwargs):
"""Initialize torchvision easyblock."""
super(EB_torchvision, self).__init__(*args, **kwargs)
Expand Down