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

enhance CMakeMake easyblock to check whether correct Python installation was picked up by cmake #3233

Closed
wants to merge 15 commits into from
Closed
68 changes: 68 additions & 0 deletions easybuild/easyblocks/generic/cmakemake.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ def prepend_config_opts(self, config_opts):
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 Down Expand Up @@ -318,6 +319,73 @@ def configure_step(self, srcdir=None, builddir=None):

(out, _) = run_cmd(command, log_all=True, simple=False)

def sanitycheck_for_configuration(self, out):
self.log.info("Checking Python paths")

if LooseVersion(self.cmake_version) >= '3.16':
try:
with open('CMakeCache.txt', 'r') as file:
lines = file.readlines()
except FileNotFoundError:
self.log.warning("CMakeCache.txt not found. Python paths checks skipped.")
return

if not os.getenv('EBROOTPYTHON'):
self.log.warning("EBROOTPYTHON is not set.")
return
else:
self.log.info("Skipping Python path checks as CMake version might be lower than 3.16.")
return

python_exec_path = []
python_include_path = []
python_library_path = []

python_prefixes = r"(Python|Python2|Python3)_"

for line in lines:
if line.startswith(python_prefixes + r"EXECUTABLE:FILEPATH="):
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work. python_prefixes is all lowercase and you likely intended to use something like re.match. Got to look up the docs but from the top of my head: python_prefixes = "PYTHON[23]?_" above and re.match(python_prefixes + "EXECUTABLE(:FILEPATH)?=", line) here would be needed

python_exec_path.append(line.split('=')[1].strip())
self.log.info("Python executable path: %s", python_exec_path[-1])
elif line.startswith(python_prefixes + r"INCLUDE_DIR:FILEPATH="):
python_include_path.append(line.split('=')[1].strip())
self.log.info("Python include path: %s", python_include_path[-1])
elif line.startswith(python_prefixes + r"LIBRARY:FILEPATH="):
python_library_path.append(line.split('=')[1].strip())
self.log.info("Python library path: %s", python_library_path[-1])

# Check if paths are found and validate paths for symlinks
Copy link
Contributor

Choose a reason for hiding this comment

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

Everything from here should be outside the loop shouldn't it?

for path_list in [python_exec_path, python_include_path, python_library_path]:
for path in path_list:
if path and not os.path.exists(path):
raise EasyBuildError(f"Path does not exist: {path}")

# Check if paths are found and handle EBROOTPYTHON cases
if any(path for path in [python_exec_path, python_include_path, python_library_path]):
if not os.getenv('EBROOTPYTHON'):
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a duplication to the below (ebrootpython_path etc). Maybe at least put them together? Are both even required? I.e. isn't ebrootpython_path enough/better than EBROOTPYTHON?

self.log.error(
"EBROOTPYTHON is not set and Python related paths are found."
)
elif not all(
(path and os.getenv('EBROOTPYTHON') in path)
for path in [python_exec_path, python_include_path, python_library_path]
Copy link
Contributor

Choose a reason for hiding this comment

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

You are checking only for $EBROOTPYTHON being in a list of paths, which likely is always false. Maybe rename the variables to be able to spot that: python_exec_path -> python_exec_paths and so on.

It might even make sense to show which path is "wrong". I.e. which of the 3 lists and which exact path is outside EB.

):
self.log.error("Python related paths do not include EBROOTPYTHON.")
else:
self.log.info("Python related paths configured correctly.")
else:
self.log.error("No Python related paths found in CMakeCache.txt.")
raise EasyBuildError("Python related paths not configured correctly.")

ebrootpython_path = get_software_root("Python")

# Validate that Python paths are consistent with EBROOTPYTHON
if python_exec_path and not ebrootpython_path:
raise EasyBuildError(
f"Python executable path found ({python_exec_path})"
f" but no Python dependency included. Check log."
)

return out

def test_step(self):
Expand Down