Rework python environment installation

[ldowen]

Summary

• This PR is a refactoring of how the virtual python environment is installed.
• Details in RELEASE_NOTES.md

---

ToDo :

• Annotate RELEASE_NOTES.md with notable changes.
• Create LLNLSpheral PR pointing at this branch. (PR#)
• LLNLSpheral PR has passed all tests.

[mdavis36]

execute_process doesn't run through a shell IIRC ... I would verify if this is doing what you think.

The old code used add_custom_command(COMMAND ...) which is executed by the build system passing through a shell, so && chaining and . activate worked. execute_process(COMMAND ...) which calls with the assumption this is a singe process and not a shell command.

The entire execute_process block may need to be wrapped in bash -c "..." or rewritten as separate execute_process calls.

[ldowen]

You are correct. It might make more sense to eliminate this file altogether and move all this logic back into some sort of configured bash file? One that configures all variables known at configure time and passes install variables in through the command line.

[mdavis36]

SpheralPRT.cmake uses ENV_REQS is this variable name changed at some point between this call and the diff I see above?

[mdavis36]

A failed installation will always touch a stamp file here, false positive install state. Need to evaluate the result of the above process with a RESULT_VARIABLE before creating.

[mdavis36]

This very likely isn't a problem, but it could be a massive headache if it becomes one. Consider forwarding compiler args here too: Probably not a huge deal since spack deals with building things like mpi4py but I know mpi4py's python install often checks compiler versions to configure itself for the system and cmake subprocesses don't forward state.

[ldowen]

Good catch. I intended to forward the install and download commands instead but forgot to change that.

[mdavis36]

Suggested change

	-P${VENV_SCRIPT}
	-P ${VENV_SCRIPT}

[mdavis36]

Suggested change

	set(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_RPATH}:${CMAKE_INSTALL_PREFIX}/lib")
	list(APPEND CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib")

If CMAKE_INSTALL_RPATH is empty this will evaluate to :/path/to/lib leading colon is interpreted by the linker as the CWD

[ldowen]

@mdavis36 The more I think about it, the less this PR makes sense. This whole rework is not worth it just to have such a small benefit. I think I will make a new PR that does some of the things here but leaves the python environment creation alone.

[mdavis36]

@ldowen I cant see the pipelines for this anymore so not sure what the final issue is, but my guess is this is either due to broken r-paths for the installed library files, attempting to move a virtual env after creation (maybe both) if so here are some thoughts:

venv

Spheral is unconventional in that it builds up a python environment for you as a part of CMake. Projects that are c++/python will often have CMake handle building and compiling libraries only. Then use a setup.py (old method) or project.toml (modern) tools to wrap the cmake process and then handle the "python install" into a given python environment (system or venv) - Spheral's CMake takes ownership of this extra python install step and goes even further out of conventional norms by creating a virtual environment to install.

Python virtual environments are not portable. They need to be re-constructed whenever they need to be moved. If all of your requirements for a python env are already installed and available on your system this should be fairly quick so you need to treat the install of the c++ compiled libs and bindings separately from the construction of a virtual environment. Spheral blurs the lines of this a little bit since the install target handles setting up the virtual environment too.

Possible approaches:

1. When needing to create a spheral instance in a different path : reconfigure cmake with a new CMAKE_INSTALL_PREFIX and run the current install target, assuming you already built spheral this shouldn't be too expensive.
2. Decouple CMake's install target. cmake install no longer builds a virtual environment. You use a separate process that builds a venv where requested, there are many ways of doing this (install-venv specific target, conventional python tools setup, direct helper script) but you are essentially treating the install location as the final path for where the system expects the libraries to be held.

r-path

Spheral's libraries and install structure needs to make reasonable assumptions to make it portable in the way devs/users expect, the install directory structure should be well defined so that when you move libs from $CMAKE_INSTALL_PREFIX to a python packages directory their relative paths don't break.

You can assume: TPL libraries never move: system level dependencies and third party libraries built by spack never move. Pathing to these should never break because you link to their shared libs directly or they are compiled static into your target so their path doesn't matter.

Now something to consider is Spheral has two levels of libraries C++ and Python shared objects, there are two approach you can take in how you might want to define this relationship:

1. (All in One): Bundle all (Spheral) C++ and Python libraries required for runtime execution into one directory (not including system / tpl dependencies), this single directory structure should be moveable from it's root since the relative paths between c++ and py bindings never change.
2. (Split): Spheral C++ libraries go into a $CMAKE_INSTALL_PREFIX directory to live out their life; Spheral python libs have their paths for Spheral C++ libs point to a static $CMAKE_INSTALL_PREFIX, allowing those python libraries to be movable to any python site-packages dir. (treating c++ libs as the static "engine" and python libs like a "portable runtime")

[ldowen]

@mdavis36 Thanks for more info on this. I initially tried the approach of reconfiguring cmake with a new CMAKE_INSTALL_PREFIX and running install. When I did that prior to this branch, it triggered a rebuild of seemingly the world. I am not sure why that happened but I can't recreate it with my current changes so perhaps I am mistaken. For my use case, I didn't want to overwrite the install directory, just set it to something different on the command line. I then tried cmake --install /path/to/build_dir --prefix new/install/prefix and that actually failed because so many things explicitly depended on CMAKE_INSTALL_PREFIX. Since reconfiguring with a new CMAKE_INSTALL_PREFIX seems to be working as I had wanted, I will switch to doing that in the particular part of the CI of interest to me.