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.

Sign up for GitHub

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

reduce the number of command line options for 'cmake' command in CMakeMake generic easyblock #2514

Merged
merged 5 commits into from
Jul 27, 2022
Merged

reduce the number of command line options for 'cmake' command in CMakeMake generic easyblock #2514

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5f8af17
Reduce the number of command line args for CMake configure
Flamefire Jul 14, 2021
507511e
Deduplicate options
Flamefire Jul 14, 2021
d05c006
Check for cache before omitting args
Flamefire Jul 14, 2021
0b78b02
Introduce prepend_config_opts for easier option handling
Flamefire Jul 16, 2021
cd45d1c
Remove extra space in configopts
Flamefire May 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 54 additions & 29 deletions easybuild/easyblocks/generic/cmakemake.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@
"""
import glob
import os
from distutils.version import LooseVersion

from easybuild.easyblocks.generic.configuremake import ConfigureMake
from easybuild.framework.easyconfig import CUSTOM
from easybuild.tools.build_log import print_warning
from easybuild.tools.config import build_option
from easybuild.tools.filetools import change_dir, create_unused_dir, mkdir, which
from easybuild.tools.environment import setvar
from easybuild.tools.modules import get_software_root
from easybuild.tools.modules import get_software_root, get_software_version
from easybuild.tools.run import run_cmd
from easybuild.tools.systemtools import get_shared_lib_ext
from easybuild.tools.utilities import nub
Expand Down Expand Up @@ -116,6 +117,14 @@ def build_type(self):
build_type = 'Debug' if self.toolchain.options.get('debug', None) else 'Release'
return build_type

def prepend_config_opts(self, config_opts):
"""Prepends configure options (-Dkey=value) to configopts ignoring those already set"""
cfg_configopts = self.cfg['configopts']
# All options are of the form '-D<key>=<value>'
new_opts = ' '.join('-D%s=%s' % (key, value) for key, value in config_opts.items()
if '-D%s=' % key not in cfg_configopts)
self.cfg['configopts'] = ' '.join([new_opts, cfg_configopts])

def configure_step(self, srcdir=None, builddir=None):
"""Configure build using cmake"""

Expand All @@ -142,36 +151,38 @@ def configure_step(self, srcdir=None, builddir=None):
install_target_subdir = self.cfg.get('install_target_subdir')
if install_target_subdir:
install_target = os.path.join(install_target, install_target_subdir)
options = ['-DCMAKE_INSTALL_PREFIX=%s' % install_target]
options = {'CMAKE_INSTALL_PREFIX': install_target}

if self.installdir.startswith('/opt') or self.installdir.startswith('/usr'):
# https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html
localstatedir = os.path.join(self.installdir, 'var')
runstatedir = os.path.join(localstatedir, 'run')
sysconfdir = os.path.join(self.installdir, 'etc')
options.append("-DCMAKE_INSTALL_LOCALSTATEDIR=%s" % localstatedir)
options.append("-DCMAKE_INSTALL_RUNSTATEDIR=%s" % runstatedir)
options.append("-DCMAKE_INSTALL_SYSCONFDIR=%s" % sysconfdir)
options['CMAKE_INSTALL_LOCALSTATEDIR'] = localstatedir
options['CMAKE_INSTALL_RUNSTATEDIR'] = runstatedir
options['CMAKE_INSTALL_SYSCONFDIR'] = sysconfdir

if '-DCMAKE_BUILD_TYPE=' in self.cfg['configopts']:
if self.cfg.get('build_type') is not None:
self.log.warning('CMAKE_BUILD_TYPE is set in configopts. Ignoring build_type')
else:
options.append('-DCMAKE_BUILD_TYPE=%s' % self.build_type)
options['CMAKE_BUILD_TYPE'] = self.build_type

# Add -fPIC flag if necessary
if self.toolchain.options['pic']:
options.append('-DCMAKE_POSITION_INDEPENDENT_CODE=ON')
options['CMAKE_POSITION_INDEPENDENT_CODE'] = 'ON'

if self.cfg['generator']:
options.append('-G "%s"' % self.cfg['generator'])
generator = '-G "%s"' % self.cfg['generator']
else:
generator = ''

# pass --sysroot value down to CMake,
# and enable using absolute paths to compiler commands to avoid
# that CMake picks up compiler from sysroot rather than toolchain compiler...
sysroot = build_option('sysroot')
if sysroot:
options.append('-DCMAKE_SYSROOT=%s' % sysroot)
options['CMAKE_SYSROOT'] = sysroot
self.log.info("Using absolute path to compiler commands because of alterate sysroot %s", sysroot)
self.cfg['abs_path_compilers'] = True

Expand All @@ -187,32 +198,49 @@ def configure_step(self, srcdir=None, builddir=None):
print_warning('Ignoring BUILD_SHARED_LIBS is set in configopts because build_shared_libs is set')
self.cfg.update('configopts', '-DBUILD_SHARED_LIBS=%s' % ('ON' if build_shared_libs else 'OFF'))

env_to_options = {
'CC': 'CMAKE_C_COMPILER',
'CFLAGS': 'CMAKE_C_FLAGS',
'CXX': 'CMAKE_CXX_COMPILER',
'CXXFLAGS': 'CMAKE_CXX_FLAGS',
'F90': 'CMAKE_Fortran_COMPILER',
'FFLAGS': 'CMAKE_Fortran_FLAGS',
}
# If the cache does not exist CMake reads the environment variables
cache_exists = os.path.exists('CMakeCache.txt')
branfosj marked this conversation as resolved.
Show resolved Hide resolved
env_to_options = dict()

# Setting compilers is not required unless we want absolute paths
if self.cfg.get('abs_path_compilers', False) or cache_exists:
env_to_options.update({
'CC': 'CMAKE_C_COMPILER',
'CXX': 'CMAKE_CXX_COMPILER',
'F90': 'CMAKE_Fortran_COMPILER',
})
else:
# Set the variable which CMake uses to init the compiler using F90 for backward compatibility
fc = os.getenv('F90')
if fc:
setvar('FC', fc)

# Flags are read from environment variables already since at least CMake 2.8.0
if LooseVersion(get_software_version('CMake')) < LooseVersion('2.8.0') or cache_exists:
Copy link
Member

Choose a reason for hiding this comment

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

We're making the assumption here that CMake is indeed listed as a (build) dependency, which was not the case before.

Prior to these changes, the configure step was failing with "cmake: command not found" (unless cmake was also available in the OS), but now we get a nasty crash:

ERROR: Traceback (most recent call last):
  File "/data/gent/400/vsc40023/easybuild_easy_installed/lib/python3.6/site-packages/easybuild_framework-4.6.1.dev0-py3.6.egg/easybuild/main.py", line 128, in build_and_install_software
    (ec_res['success'], app_log, err) = build_and_install_one(ec, init_env)
  File "/data/gent/400/vsc40023/easybuild_easy_installed/lib/python3.6/site-packages/easybuild_framework-4.6.1.dev0-py3.6.egg/easybuild/framework/easyblock.py", line 4077, in build_and_install_one
    result = app.run_all_steps(run_test_cases=run_test_cases)
  File "/data/gent/400/vsc40023/easybuild_easy_installed/lib/python3.6/site-packages/easybuild_framework-4.6.1.dev0-py3.6.egg/easybuild/framework/easyblock.py", line 3960, in run_all_steps
    self.run_step(step_name, step_methods)
  File "/data/gent/400/vsc40023/easybuild_easy_installed/lib/python3.6/site-packages/easybuild_framework-4.6.1.dev0-py3.6.egg/easybuild/framework/easyblock.py", line 3795, in run_step
    step_method(self)()
  File "/data/gent/400/vsc40023/easybuild_easy_installed/lib/python3.6/site-packages/easybuild_easyblocks-4.6.1.dev0-py3.6.egg/easybuild/easyblocks/generic/cmakemake.py", line 221, in configure_step
    if LooseVersion(get_software_version('CMake')) < LooseVersion('2.8.0') or cache_exists:
  File "/usr/lib64/python3.6/distutils/version.py", line 52, in __lt__
    c = self._cmp(other)
  File "/usr/lib64/python3.6/distutils/version.py", line 335, in _cmp
    if self.version == other.version:
AttributeError: 'LooseVersion' object has no attribute 'version'

We're also not taking into account that CMake may be filtered out as a dependency via --filter-deps (some people may prefer using the CMake that is provided via the OS), so we should have a fallback to determining the CMake version via cmake --version if get_software_version returns None.

@Flamefire Are you up for looking into this? If not, we should open an issue to avoid forgetting about this (we should handle this prior to the next EasyBuild release that includes the changes in this PR)

Copy link
Member

Choose a reason for hiding this comment

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

I looked into this myself, see #2772

env_to_options.update({
'CFLAGS': 'CMAKE_C_FLAGS',
'CXXFLAGS': 'CMAKE_CXX_FLAGS',
'FFLAGS': 'CMAKE_Fortran_FLAGS',
})

for env_name, option in env_to_options.items():
value = os.getenv(env_name)
if value is not None:
if option.endswith('_COMPILER') and self.cfg.get('abs_path_compilers', False):
value = which(value)
self.log.info("Using absolute path to compiler command: %s", value)
options.append("-D%s='%s'" % (option, value))
options[option] = value

if build_option('rpath'):
# instruct CMake not to fiddle with RPATH when --rpath is used, since it will undo stuff on install...
# https://github.com/LLNL/spack/blob/0f6a5cd38538e8969d11bd2167f11060b1f53b43/lib/spack/spack/build_environment.py#L416
options.append('-DCMAKE_SKIP_RPATH=ON')
options['CMAKE_SKIP_RPATH'] = 'ON'

# show what CMake is doing by default
options.append('-DCMAKE_VERBOSE_MAKEFILE=ON')
options['CMAKE_VERBOSE_MAKEFILE'] = 'ON'

# disable CMake user package repository
options.append('-DCMAKE_FIND_USE_PACKAGE_REGISTRY=FALSE')
options['CMAKE_FIND_USE_PACKAGE_REGISTRY'] = 'OFF'

if not self.cfg.get('allow_system_boost', False):
boost_root = get_software_root('Boost')
Expand All @@ -224,23 +252,20 @@ def configure_step(self, srcdir=None, builddir=None):
if len(cmake_files) > 1 and 'libboost_system-variant-shared.cmake' in cmake_files:
# disable search for Boost CMake package configuration files when conflicting variant configs
# are present (builds using the old EasyBlock)
options.append('-DBoost_NO_BOOST_CMAKE=ON')
options['Boost_NO_BOOST_CMAKE'] = 'ON'

# Don't pick up on system Boost if Boost is included as dependency
# - specify Boost location via -DBOOST_ROOT
# - instruct CMake to not search for Boost headers/libraries in other places
options.extend([
'-DBOOST_ROOT=%s' % boost_root,
'-DBoost_NO_SYSTEM_PATHS=ON',
])

options_string = ' '.join(options)
options['BOOST_ROOT'] = boost_root
options['Boost_NO_SYSTEM_PATHS'] = 'ON'

if self.cfg.get('configure_cmd') == DEFAULT_CONFIGURE_CMD:
self.prepend_config_opts(options)
command = ' '.join([
self.cfg['preconfigopts'],
DEFAULT_CONFIGURE_CMD,
options_string,
generator,
self.cfg['configopts'],
srcdir])
else:
Expand Down