Perfexionists / perun

Lightweight Performance Control System
GNU General Public License v3.0
16 stars 14 forks source link

Jinja2 - PackageLoader unable to find the installation of perun #204

Closed PeterMocary closed 3 months ago

PeterMocary commented 4 months ago

When loading a Jinja2 template thePackageLoader is used to locate the templates within the package. When is the perun package installed using make dev the PackageLoader fails and resutlts in a crash.

The PackageLoader is used in view_diff, ktrace, and in the PIN engine, therefore this should be investigated further to ensure stable development enviroment.

Triggered on PIN engine when generating a pintool from templates:

[ERROR] while collecting by trace: error while collect phase: The 'perun' package was not installed in a way that PackageLoader understands.
  File "[...]/perun/perun/logic/runner.py", line 217, in run_phase_function
    phase_result = phase_function(**report.kwargs)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[...]/perun/perun/collect/trace/run.py", line 99, in collect
    config.engine.assemble_collect_program(**kwargs)
  File "[...]/perun/perun/collect/trace/pin/engine.py", line 139, in assemble_collect_program
    self._assemble_pintool(**configuration)
  File "[...]/perun/perun/collect/trace/pin/engine.py", line 241, in _assemble_pintool
    loader=PackageLoader('perun', 'collect/trace/pin/templates'),
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[...]/perun/.perun-venv-python3.11/lib64/python3.11/site-packages/jinja2/loaders.py", line 326, in __init__
    raise ValueError(

Note: The [...] is truncated path (e.g. /home/user/projects).

The ValueError is triggered from the jinja2/loaders.py where one can see that import spec is used to extract submodule search locations. If we compare the value of submodule_search_locations in environment initialized with make install and another with make dev we see that the editable version from make dev environment contains path that is not existing.

Environment initialized with make dev reporting invalid path in submodule_search_locations:

Python 3.11.9 (main, Apr 17 2024, 00:00:00) [GCC 13.2.1 20240316 (Red Hat 13.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import importlib.util as u
>>> spec = u.find_spec("perun")
>>> spec.submodule_search_locations
['[...]/perun/.perun-venv-python3.11/lib64/python3.11/site-packages/_perun_toolsuite_editable_loader.py/perun']

Environment initialized with make install:

Python 3.11.9 (main, Apr 17 2024, 00:00:00) [GCC 13.2.1 20240316 (Red Hat 13.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import importlib.util as u
>>> spec = u.find_spec("perun")
>>> spec.submodule_search_locations
['[...]/perun/perun']

This could be a problem with the build system since @JiriPavela pointed out that the behaviour with editable environment is not deterministic (however for me it happened every time so far).

PeterMocary commented 4 months ago

This seems to be a problem with Meson build system. The editable version creates a MetaPathFinder in the file _editable.py. This finder implements the previously mentioned find_spec function and creates invalid path (see above in the make dev example) to the submodule_search_locations field. From the submodule_search_locations definition there should be a path present where the submodules of a package are found and thus, this seems to be a bug in the Meson build system. However, I might be missing something due to my limited understanding of Meson.

Proposed fix:

tfiedor commented 3 months ago

Yeah, this is currently p***ing me off as well, as suddenly it manifested in my environment. I'm wondering how we could approach it. What is the best or least resistance.

Is there some alternative of using PackageLoader?

tfiedor commented 3 months ago

Seems that we could use "FileLoader" with absolute/relative paths that could solve our problem, I will try to look into this soon, as it annoys me now. @JiriPavela what do you think? Should we somehow try to fix the PackageLoader or go for FileLoader (this might introduce some other issues though).

PeterMocary commented 3 months ago

Reading the documentation for the FileSystemLoader again, I might have dissregarded it previously due to the following: "Load templates from a directory in the file system. The path can be relative or absolute. Relative paths are relative to the current working directory."

This means that the developer cannot simply pass a relative path to the templates from the directory where the loader is called. Instead, the path will be interpreted relative to the current working directory from which the tool is executed. Therefore, it requires finding the absolute path to the template before passing it to the FileSystemLoader.

This is why I suggested the "adjustment" of the PackageLoader instead.

tfiedor commented 3 months ago

This should not pose a problem, we can easily extract the path wrt install directory. I am not 100% confident, but we are already using some of this magic elsewhere (to run scripts). So I could try to make it work. EDIT: If it is more complex, then we could use the custom PackageLoader, still good analysis of the problem from your part.