From 7212d5545317bf00509b8ec3825b53d124d94b82 Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 26 Sep 2024 11:15:12 +0200 Subject: [PATCH 01/12] Added to CMakeMake --- easybuild/easyblocks/generic/cmakemake.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index eb0d582018..98cd44f258 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -284,6 +284,10 @@ def configure_step(self, srcdir=None, builddir=None): # disable CMake user package repository options['CMAKE_FIND_USE_PACKAGE_REGISTRY'] = 'OFF' + # ensure CMake uses PATH to determine python without prioritizing a virtualenv + # necessary to pick up on the correct python version when `eb` is run from a virtualenv + options['Python3_FIND_VIRTUALENV'] = 'STANDARD' + if not self.cfg.get('allow_system_boost', False): boost_root = get_software_root('Boost') if boost_root: From 47f21b974fda5c4b4404e09f3a677de0e0667b49 Mon Sep 17 00:00:00 2001 From: crivella Date: Mon, 30 Sep 2024 15:43:41 +0200 Subject: [PATCH 02/12] Generalized to `Python_EXECUTABLE` --- easybuild/easyblocks/generic/cmakemake.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 98cd44f258..906fa5ade2 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -284,9 +284,19 @@ def configure_step(self, srcdir=None, builddir=None): # disable CMake user package repository options['CMAKE_FIND_USE_PACKAGE_REGISTRY'] = 'OFF' - # ensure CMake uses PATH to determine python without prioritizing a virtualenv - # necessary to pick up on the correct python version when `eb` is run from a virtualenv - options['Python3_FIND_VIRTUALENV'] = 'STANDARD' + # ensure CMake uses EB python, not system or virtualenv python + python_root = get_software_root('Python') + if python_root: + python_version = LooseVersion(get_software_version('Python')) + python_exe = os.path.join(python_root, 'bin', 'python') + options['Python_EXECUTABLE'] = python_exe + # This is needed if someone is still using `find_package(PythonInterp ...)` in their CMakeLists.txt + options['PYTHON_EXECUTABLE'] = python_exe + # Ensure that both `find_package(Python) and find_package(Python2/3)` work as intended + if python_version >= "3": + options['Python3_EXECUTABLE'] = python_exe + else: + options['Python2_EXECUTABLE'] = python_exe if not self.cfg.get('allow_system_boost', False): boost_root = get_software_root('Boost') From 2878569fafe2043a63a2accc24463b8e632f4dc8 Mon Sep 17 00:00:00 2001 From: Davide Grassano <34096612+Crivella@users.noreply.github.com> Date: Tue, 1 Oct 2024 11:01:45 +0200 Subject: [PATCH 03/12] Update easybuild/easyblocks/generic/cmakemake.py Co-authored-by: Alexander Grund --- easybuild/easyblocks/generic/cmakemake.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 906fa5ade2..cfdb73f02d 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -289,10 +289,10 @@ def configure_step(self, srcdir=None, builddir=None): if python_root: python_version = LooseVersion(get_software_version('Python')) python_exe = os.path.join(python_root, 'bin', 'python') - options['Python_EXECUTABLE'] = python_exe - # This is needed if someone is still using `find_package(PythonInterp ...)` in their CMakeLists.txt + # This is required for (deprecated) `find_package(PythonInterp ...)` options['PYTHON_EXECUTABLE'] = python_exe # Ensure that both `find_package(Python) and find_package(Python2/3)` work as intended + options['Python_EXECUTABLE'] = python_exe if python_version >= "3": options['Python3_EXECUTABLE'] = python_exe else: From 57b03e4d5e062bebd7e63e112e2ce00bc8750530 Mon Sep 17 00:00:00 2001 From: crivella Date: Fri, 4 Oct 2024 13:27:24 +0200 Subject: [PATCH 04/12] Store generate options to allow usage by children classes --- easybuild/easyblocks/generic/cmakemake.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index cfdb73f02d..5b7ec4d702 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -316,6 +316,8 @@ def configure_step(self, srcdir=None, builddir=None): options['BOOST_ROOT'] = boost_root options['Boost_NO_SYSTEM_PATHS'] = 'ON' + self.cmake_options = options + if self.cfg.get('configure_cmd') == DEFAULT_CONFIGURE_CMD: self.prepend_config_opts(options) command = ' '.join([ From 606533b25a0557f8c93240c35e87ac448394683e Mon Sep 17 00:00:00 2001 From: crivella Date: Fri, 4 Oct 2024 13:28:51 +0200 Subject: [PATCH 05/12] Convenience function to set find_python HINTS --- easybuild/easyblocks/generic/cmakemake.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 5b7ec4d702..296c8ab039 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -336,6 +336,22 @@ def configure_step(self, srcdir=None, builddir=None): return out + def set_cmake_env_vars_python(self): + """Convenience function to set CMake hints for FindPython[_2/3] as environment variables. + Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a + build and no option is provided to set custom CMake variables. + """ + if LooseVersion(self.cmake_version) < '3.12': + raise EasyBuildError("Setting Python hints for CMake requires CMake version 3.12 or newer") + python_root = get_software_root('Python') + if python_root: + python_version = LooseVersion(get_software_version('Python')) + setvar('Python_ROOT_DIR', python_root) + if python_version >= "3": + setvar('Python3_ROOT_DIR', python_root) + else: + setvar('Python2_ROOT_DIR', python_root) + def test_step(self): """CMake specific test setup""" # When using ctest for tests (default) then show verbose output if a test fails From 9a31f79bf8c3c4d9bb5e18ecf6d6f6f21e57d486 Mon Sep 17 00:00:00 2001 From: crivella Date: Wed, 9 Oct 2024 11:45:17 +0200 Subject: [PATCH 06/12] Modularized and more convenience functions --- easybuild/easyblocks/generic/cmakemake.py | 41 ++++++++++++++++------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 296c8ab039..8400d32217 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -285,18 +285,7 @@ def configure_step(self, srcdir=None, builddir=None): options['CMAKE_FIND_USE_PACKAGE_REGISTRY'] = 'OFF' # ensure CMake uses EB python, not system or virtualenv python - python_root = get_software_root('Python') - if python_root: - python_version = LooseVersion(get_software_version('Python')) - python_exe = os.path.join(python_root, 'bin', 'python') - # This is required for (deprecated) `find_package(PythonInterp ...)` - options['PYTHON_EXECUTABLE'] = python_exe - # Ensure that both `find_package(Python) and find_package(Python2/3)` work as intended - options['Python_EXECUTABLE'] = python_exe - if python_version >= "3": - options['Python3_EXECUTABLE'] = python_exe - else: - options['Python2_EXECUTABLE'] = python_exe + options.update(self._get_cmake_python_config()) if not self.cfg.get('allow_system_boost', False): boost_root = get_software_root('Boost') @@ -336,7 +325,33 @@ def configure_step(self, srcdir=None, builddir=None): return out - def set_cmake_env_vars_python(self): + def _get_cmake_python_config(self): + """Get the CMake configuration options for Python hints.""" + options = {} + python_root = get_software_root('Python') + if python_root: + python_version = LooseVersion(get_software_version('Python')) + python_exe = os.path.join(python_root, 'bin', 'python') + # This is required for (deprecated) `find_package(PythonInterp ...)` + options['PYTHON_EXECUTABLE'] = python_exe + # Ensure that both `find_package(Python) and find_package(Python2/3)` work as intended + options['Python_EXECUTABLE'] = python_exe + if python_version >= "3": + options['Python3_EXECUTABLE'] = python_exe + else: + options['Python2_EXECUTABLE'] = python_exe + return options + + def get_cmake_python_config_dict(self): + """Get a dictionary with the CMake configuration options for Python hints.""" + return self._get_cmake_python_config() + + def get_cmake_python_config_str(self): + """Get a string with the CMake configuration options for Python hints.""" + options = self._get_cmake_python_config() + return ' '.join(['-D%s=%s' % (key, value) for key, value in options.items()]) + + def set_cmake_python_env_hints(self): """Convenience function to set CMake hints for FindPython[_2/3] as environment variables. Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a build and no option is provided to set custom CMake variables. From fa30b9cd6c40ec1a02db44b802c6cb39525ec338 Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 10 Oct 2024 10:45:36 +0200 Subject: [PATCH 07/12] Generalized functions and removed redundant policy --- easybuild/easyblocks/generic/cmakemake.py | 90 +++++++++++------------ 1 file changed, 43 insertions(+), 47 deletions(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 8400d32217..d3bd6c4487 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -86,6 +86,49 @@ def setup_cmake_env(tc): setvar("CMAKE_INCLUDE_PATH", include_paths) setvar("CMAKE_LIBRARY_PATH", library_paths) +def setup_cmake_env_python_hints(cmake_version): + """Convenience function to set CMake hints for FindPython[_2/3] as environment variables. + Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a + build and no option is provided to set custom CMake variables. + """ + cmake_version = det_cmake_version() + if LooseVersion(cmake_version) < '3.12': + raise EasyBuildError("Setting Python hints for CMake requires CMake version 3.12 or newer") + python_root = get_software_root('Python') + if python_root: + python_version = LooseVersion(get_software_version('Python')) + setvar('Python_ROOT_DIR', python_root) + if python_version >= "3": + setvar('Python3_ROOT_DIR', python_root) + else: + setvar('Python2_ROOT_DIR', python_root) + +def _get_cmake_python_config(): + """Get the CMake configuration options for Python hints.""" + options = {} + python_root = get_software_root('Python') + if python_root: + python_version = LooseVersion(get_software_version('Python')) + python_exe = os.path.join(python_root, 'bin', 'python') + # This is required for (deprecated) `find_package(PythonInterp ...)` + options['PYTHON_EXECUTABLE'] = python_exe + # Ensure that both `find_package(Python) and find_package(Python2/3)` work as intended + options['Python_EXECUTABLE'] = python_exe + if python_version >= "3": + options['Python3_EXECUTABLE'] = python_exe + else: + options['Python2_EXECUTABLE'] = python_exe + return options + +def get_cmake_python_config_dict(): + """Get a dictionary with the CMake configuration options for Python hints.""" + return _get_cmake_python_config() + +def get_cmake_python_config_str(): + """Get a string with the CMake configuration options for Python hints.""" + options = _get_cmake_python_config() + return ' '.join(['-D%s=%s' % (key, value) for key, value in options.items()]) + class CMakeMake(ConfigureMake): """Support for configuring build with CMake instead of traditional configure script""" @@ -273,11 +316,6 @@ def configure_step(self, srcdir=None, builddir=None): # see https://github.com/Kitware/CMake/commit/3ec9226779776811240bde88a3f173c29aa935b5 options['CMAKE_SKIP_RPATH'] = 'ON' - # make sure that newer CMAKE picks python based on location, not just the newest python - # Avoids issues like e.g. https://github.com/EESSI/software-layer/pull/370#issuecomment-1785594932 - if LooseVersion(self.cmake_version) >= '3.15': - options['CMAKE_POLICY_DEFAULT_CMP0094'] = 'NEW' - # show what CMake is doing by default options['CMAKE_VERBOSE_MAKEFILE'] = 'ON' @@ -325,48 +363,6 @@ def configure_step(self, srcdir=None, builddir=None): return out - def _get_cmake_python_config(self): - """Get the CMake configuration options for Python hints.""" - options = {} - python_root = get_software_root('Python') - if python_root: - python_version = LooseVersion(get_software_version('Python')) - python_exe = os.path.join(python_root, 'bin', 'python') - # This is required for (deprecated) `find_package(PythonInterp ...)` - options['PYTHON_EXECUTABLE'] = python_exe - # Ensure that both `find_package(Python) and find_package(Python2/3)` work as intended - options['Python_EXECUTABLE'] = python_exe - if python_version >= "3": - options['Python3_EXECUTABLE'] = python_exe - else: - options['Python2_EXECUTABLE'] = python_exe - return options - - def get_cmake_python_config_dict(self): - """Get a dictionary with the CMake configuration options for Python hints.""" - return self._get_cmake_python_config() - - def get_cmake_python_config_str(self): - """Get a string with the CMake configuration options for Python hints.""" - options = self._get_cmake_python_config() - return ' '.join(['-D%s=%s' % (key, value) for key, value in options.items()]) - - def set_cmake_python_env_hints(self): - """Convenience function to set CMake hints for FindPython[_2/3] as environment variables. - Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a - build and no option is provided to set custom CMake variables. - """ - if LooseVersion(self.cmake_version) < '3.12': - raise EasyBuildError("Setting Python hints for CMake requires CMake version 3.12 or newer") - python_root = get_software_root('Python') - if python_root: - python_version = LooseVersion(get_software_version('Python')) - setvar('Python_ROOT_DIR', python_root) - if python_version >= "3": - setvar('Python3_ROOT_DIR', python_root) - else: - setvar('Python2_ROOT_DIR', python_root) - def test_step(self): """CMake specific test setup""" # When using ctest for tests (default) then show verbose output if a test fails From bdcf7dada1ef9841366b801576bce31337d40656 Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 10 Oct 2024 10:46:21 +0200 Subject: [PATCH 08/12] lint --- easybuild/easyblocks/generic/cmakemake.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index d3bd6c4487..19400d64fd 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -86,6 +86,7 @@ def setup_cmake_env(tc): setvar("CMAKE_INCLUDE_PATH", include_paths) setvar("CMAKE_LIBRARY_PATH", library_paths) + def setup_cmake_env_python_hints(cmake_version): """Convenience function to set CMake hints for FindPython[_2/3] as environment variables. Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a @@ -103,6 +104,7 @@ def setup_cmake_env_python_hints(cmake_version): else: setvar('Python2_ROOT_DIR', python_root) + def _get_cmake_python_config(): """Get the CMake configuration options for Python hints.""" options = {} @@ -120,10 +122,12 @@ def _get_cmake_python_config(): options['Python2_EXECUTABLE'] = python_exe return options + def get_cmake_python_config_dict(): """Get a dictionary with the CMake configuration options for Python hints.""" return _get_cmake_python_config() + def get_cmake_python_config_str(): """Get a string with the CMake configuration options for Python hints.""" options = _get_cmake_python_config() From c88446f9a364ef6bb4fce0607e0736881c2732bb Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 10 Oct 2024 11:19:32 +0200 Subject: [PATCH 09/12] Removed helper function --- easybuild/easyblocks/generic/cmakemake.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 19400d64fd..46d3470181 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -105,8 +105,8 @@ def setup_cmake_env_python_hints(cmake_version): setvar('Python2_ROOT_DIR', python_root) -def _get_cmake_python_config(): - """Get the CMake configuration options for Python hints.""" +def get_cmake_python_config_dict(): + """Get a dictionary with the CMake configuration options for Python hints.""" options = {} python_root = get_software_root('Python') if python_root: @@ -123,14 +123,9 @@ def _get_cmake_python_config(): return options -def get_cmake_python_config_dict(): - """Get a dictionary with the CMake configuration options for Python hints.""" - return _get_cmake_python_config() - - def get_cmake_python_config_str(): """Get a string with the CMake configuration options for Python hints.""" - options = _get_cmake_python_config() + options = get_cmake_python_config_dict() return ' '.join(['-D%s=%s' % (key, value) for key, value in options.items()]) @@ -327,7 +322,7 @@ def configure_step(self, srcdir=None, builddir=None): options['CMAKE_FIND_USE_PACKAGE_REGISTRY'] = 'OFF' # ensure CMake uses EB python, not system or virtualenv python - options.update(self._get_cmake_python_config()) + options.update(get_cmake_python_config_dict()) if not self.cfg.get('allow_system_boost', False): boost_root = get_software_root('Boost') From b265338dc3fa4a6ba65c40dc5026e60a16f9ec8f Mon Sep 17 00:00:00 2001 From: Davide Grassano <34096612+Crivella@users.noreply.github.com> Date: Thu, 10 Oct 2024 12:34:23 +0200 Subject: [PATCH 10/12] Update easybuild/easyblocks/generic/cmakemake.py Co-authored-by: Alexander Grund --- easybuild/easyblocks/generic/cmakemake.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 46d3470181..40a7df4015 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -92,7 +92,8 @@ def setup_cmake_env_python_hints(cmake_version): Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a build and no option is provided to set custom CMake variables. """ - cmake_version = det_cmake_version() + if cmake_version is None: + cmake_version = det_cmake_version() if LooseVersion(cmake_version) < '3.12': raise EasyBuildError("Setting Python hints for CMake requires CMake version 3.12 or newer") python_root = get_software_root('Python') From 7aff4c0026a6c5e4b2b0a605cef4e847b698886b Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 10 Oct 2024 12:35:17 +0200 Subject: [PATCH 11/12] Add default --- easybuild/easyblocks/generic/cmakemake.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index 40a7df4015..b8e02897e0 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -87,7 +87,7 @@ def setup_cmake_env(tc): setvar("CMAKE_LIBRARY_PATH", library_paths) -def setup_cmake_env_python_hints(cmake_version): +def setup_cmake_env_python_hints(cmake_version=None): """Convenience function to set CMake hints for FindPython[_2/3] as environment variables. Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a build and no option is provided to set custom CMake variables. From cd7cad3aff580cf0e0b1fd96102f33969e143d1c Mon Sep 17 00:00:00 2001 From: Davide Grassano <34096612+Crivella@users.noreply.github.com> Date: Thu, 10 Oct 2024 12:41:47 +0200 Subject: [PATCH 12/12] Apply suggestions from code review Co-authored-by: Alexander Grund --- easybuild/easyblocks/generic/cmakemake.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/easybuild/easyblocks/generic/cmakemake.py b/easybuild/easyblocks/generic/cmakemake.py index b8e02897e0..797a504fb2 100644 --- a/easybuild/easyblocks/generic/cmakemake.py +++ b/easybuild/easyblocks/generic/cmakemake.py @@ -88,9 +88,10 @@ def setup_cmake_env(tc): def setup_cmake_env_python_hints(cmake_version=None): - """Convenience function to set CMake hints for FindPython[_2/3] as environment variables. - Needed to avoid wrong Python being picked up by CMake when not called directly by EasyBuild but as step in a - build and no option is provided to set custom CMake variables. + """Set environment variables as hints for CMake to prefer the Python module, if loaded. + Useful when there is no way to specify arguments for CMake directly, + e.g. when CMake is called from within another build system. + Otherwise get_cmake_python_config_[str/dict] should be used instead. """ if cmake_version is None: cmake_version = det_cmake_version() @@ -107,7 +108,7 @@ def setup_cmake_env_python_hints(cmake_version=None): def get_cmake_python_config_dict(): - """Get a dictionary with the CMake configuration options for Python hints.""" + """Get a dictionary with CMake configuration options for finding Python if loaded as a module.""" options = {} python_root = get_software_root('Python') if python_root: @@ -125,9 +126,11 @@ def get_cmake_python_config_dict(): def get_cmake_python_config_str(): - """Get a string with the CMake configuration options for Python hints.""" + """Get CMake configuration arguments for finding Python if loaded as a module. + This string is intended to be passed to the invocation of `cmake`. + """ options = get_cmake_python_config_dict() - return ' '.join(['-D%s=%s' % (key, value) for key, value in options.items()]) + return ' '.join('-D%s=%s' % (key, value) for key, value in options.items()) class CMakeMake(ConfigureMake):