easybuilders / easybuild-easyblocks

Collection of easyblocks that implement support for building and installing software with EasyBuild.
https://easybuild.io
GNU General Public License v2.0
106 stars 285 forks source link

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

Open gkaf89 opened 3 months ago

gkaf89 commented 3 months ago

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:

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

boegel commented 3 months ago

@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.

gkaf89 commented 3 months ago

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?

Flamefire commented 1 week ago

@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