python: enter virtual environments before starting modules#314
python: enter virtual environments before starting modules#314andistorm merged 8 commits intoEVerest:mainfrom
Conversation
Signed-off-by: Ivar Scholten <ivar.scholten@protonmail.com>
Signed-off-by: Ivar Scholten <ivar.scholten@protonmail.com>
When spawning Python modules the manager previously only set PYTHONPATH if it isn't already present in the environment, which can result in modules not being able to locate the `everestpy` package. This commit fixes that by prepending the `everestpy` path to PYTHONPATH if already set. Signed-off-by: Ivar Scholten <ivar.scholten@protonmail.com>
We noticed the lack of Python dependency isolation with `pydantic`,
`PyEvJosev` requires a rather old version which conflicts with our own
modules. While we could upgrade Josev to temporarily fix the problem,
this doesn't scale well: having modules be tightly coupled to one other's
internals makes it hard to develop them in isolation.
This commit makes the first step towards fixing that by having the
manager process activate each module's virtual environment before
starting it (if present).
It does not automatically generate or install the module's dependencies
in the virtual environment. Ideally CMake would generate it for us at
build time, but this turns out to be a bit trickier than expected:
* Virtual environments are not portable, they contain absolute paths to
themselves in various generated scripts.
* While `everest-cmake` is already able to generate a wheel for each
module (containing the dependency manifest and the module itself),
it isn't able to cross-compile them.
This matters because a lot of Python libraries link to native code that
is fetched/compiled at install time, so forcibly installing wheels
generated by CMake would break EVerest's cross-compilation support.
There sadly doesn't appear to be a standardized way to indicate a
cross-compilation context, though we might be able to utilize projects
like crossenv[1] or cibuildwheel[2]. This needs more investigation.
* If cross-compilation doesn't turn out to be viable we could instead
install dependencies at runtime, from the manager process. This
completely resolves any cross-compilation concerns, but in turn has the
downside of slowing down module startup.
Assuming we're fine with requiring `pyproject.toml` (PEP 621) for
dependency management, the installation procedure could look something
like this:
- Install the `pyproject.toml` to a standardized location from CMake.
- At runtime, use the build[3] module to generate a virtual environment
from the `pyproject.toml`.
It looks like we can install only the `build-frontend` and
dependencies with a bit of scripting, so that we don't redundantly
include the module itself like the CLI does by default. This is
useful for incrementality, directly copying the module source when
making a change is a lot faster than going through the build
frontend.
In any case, after creating a virtual environment we would need to enter
it from the manager process, which this commit does. I'm not sure what
the best way to proceed forward is, I'd love some feedback.
[1]: https://pypi.org/project/crossenv
[2]: https://github.com/pypa/cibuildwheel
[3]: https://pypi.org/project/build
Signed-off-by: Ivar Scholten <ivar.scholten@protonmail.com>
…H lookup is needed Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
There was a problem hiding this comment.
@IvarWithoutBones thanks for your contribution. 🐧 Looks good to me, just two things:
- I added a fix that makes use of
execvp()instead ofexecv()this is necessary for the case js modules are started and python modules without dedicated venv are started. - I reverted the clang-tidy fixes commit, since it is not related. We should open up a separate PR for this.
This reverts commit 27abada. Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
b99b280 to
fef9a35
Compare
|
I opened #315 for the clang tidy spellchecker commit |
Oh right, that makes sense. Thanks for fixing it.
I'll keep future PRs more to the point, I appreciate you separating it out this time :) |
1 similar comment
Oh right, that makes sense. Thanks for fixing it.
I'll keep future PRs more to the point, I appreciate you separating it out this time :) |
We noticed that Python dependencies are currently not isolated between modules, specifically with
pydantic:PyEvJosevrequires a rather old version which conflicts with our own modules. While we could upgrade Josev to temporarily fix the problem, this doesn't scale very well in the long run.This PR makes a step towards improving the situation, it makes the manager process activate each module's virtual environment before starting it (if present). This PR does not automatically generate or install the module's dependencies in the virtual environment, I tested it by generating one by hand:
Ideally CMake would generate everything for us at build time, but this turns out to be a bit trickier than expected:
Virtual environments are not portable, they contain absolute paths to themselves in various generated scripts.
While
everest-cmakeis already able to generate a wheel for each module (which we could reasonably quickly install at runtime), it isn't able to cross-compile them. This matters because a lot of Python libraries link to native code that is fetched/compiled at install time, so forcibly using wheels generated by CMake would break EVerest's cross-compilation support.There sadly doesn't appear to be a standardized way to indicate a cross-compilation context when building Python packages, though we might be able to utilize projects like crossenv or cibuildwheel. This needs more investigation.
If cross-compilation doesn't turn out to be viable we could instead install/download dependencies at runtime, from the manager process. This completely resolves any cross-compilation concerns, but in turn has the downside of slowing down module startup.
In any case, after creating a virtual environment we would need to enter it from the manager process, which this PR does (in addition to some small cleanup). I'm not sure what the best way to proceed forward is, I'd love some feedback. What would be the appropriate repository to open an issue about this?