easybuild-easyblocks icon indicating copy to clipboard operation
easybuild-easyblocks copied to clipboard

change custom easyblock for Python to set `pythonpath` class variable to a fixed location relative to build path

Open gkaf89 opened this issue 1 year ago • 2 comments
trafficstars

The pythonpath variable in the python.py EasyBlock is used to create temporary directories to store build artifacts. However, there are 2 problems with the current definition of the pythonpath instance field:

  • it uses the log directory to store build artifacts, and
  • the functions that use pythonpath do not accept a full path.

The build_path() function cannot be used instead of log_path() as the function provides a full path as well. Thus we opted to hard-code a relative path with respect to the build directory.

Issue: https://github.com/easybuilders/easybuild-easyconfigs/issues/21078

gkaf89 avatar Jul 31 '24 10:07 gkaf89

@gkaf89 Thanks a lot for your in-depth analysis in https://github.com/easybuilders/easybuild-easyconfigs/issues/21078, that's very helpful.

I'm not sure the proposed fix is correct though... I think you're seeing a secondary effect of another problem that is likely to cause trouble somewhere else too.

I'll need some time to deep dive into this though, and fully understand what's going on.

boegel avatar Jul 31 '24 19:07 boegel

With the proposed modification the following file directory structure is created in the Python installation directory:

easybuild/python/sitecustomize.py

The easybuild normally contains the log files. If sitecustomize.py script is part of the log files, then the commit indeed has the potential to break any system that depends using sitecustomize.py from the log files.

However, based on definition of SITECUSTOMIZE, the file seems to target the installed software, but I cannot exclude that it is also needed by the files in the reprod directory of the logs.

If the sitecustomize.py is required by both the log files and the installed python software, maybe we need to extract out any functionality from the log files that depends on the software installation?

gkaf89 avatar Aug 12 '24 10:08 gkaf89

@boegel Independent of #3493 this looks correct to me as it is merely a semantic change without any actual change as far as I can tell. I.e. the usage of log_path at that point isn't correct from a semantic POV

Flamefire avatar Nov 07 '24 11:11 Flamefire

BTW: Can you change the title and description to make it fit better:

  • The path is relative to the install directory not the build path
  • The variable is not "used to create temporary directories to store build artifacts". There is nothing temporary about pythonpath
  • "the functions that use pythonpath do not accept a full path." -> "... expect a relative path"
  • The build_path() function -> Again nothing about the build path here, it is always used (more or less) relative to the install path

This would help better understanding the change some time later

Flamefire avatar Nov 21 '24 08:11 Flamefire

The commit is reorganized according to the suggestions. All modification are made in a single commit so that they are easier to track. The underlying issue is that sitecustomize.py is an environment configuration script, and as such it should be stored under an etc directory in the installation directory of Python.

I was a bit confused about the role of sitecustomize.py as it was stored in the directory containing the EasyBuild installation logs.

gkaf89 avatar Nov 24 '24 09:11 gkaf89

This now got superseded by getting rid of the path: https://github.com/easybuilders/easybuild-easyblocks/pull/3514

This PR would break --module-only builds as the path changed. So I'd go with the other that takes care of that too

Flamefire avatar Nov 25 '24 08:11 Flamefire

Indeed, let's go forward with the approach proposed in #3514, so closing...

boegel avatar Dec 03 '24 20:12 boegel