-
Couldn't load subscription status.
- Fork 24
fix: expose python libraries #184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
We chose to use a direct path to the python executable. This way the user-configured workflow/env will not conflict with the venv that cpp-linter-action uses.
If you need a python setup where the env is updated/altered, then you can still use actions/setup-python in your own workflow.
I don't need it, the problem is when creating the venv. It's not able to find libpython when doing python -m venv.
Maybe we could export the variables right there
Activating the venv does export modifications to env variables, but I'm more concerned with the fact that you are using a python venv that is supposed to be specific to cpp-linter-action.
Is there a reason you can't use actions/setup-python in your user workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are undesired, unless you can tell me why you want to use the same venv this action uses instead of setting up your own venv.
I think I got misunderstood. The problem I face is in the venv creation that cpp-linter-action does here.
With the following minimal example it can be reproduced:
name: Test on: push: jobs: cpp-linter: runs-on: ubuntu-latest container: ubuntu:22.04 steps: - name: Install missing deps env: DEBIAN_FRONTEND: noninteractive run: apt-get update -y && apt-get install -y sudo - uses: cpp-linter/cpp-linter-action@latest
Here you have a run with it. Note the error that some python library is missing there.
Note that in the minimal example I'm using a container (ubuntu:22.04 in that case) that does not have python3.11 installed.
We could export the necessary variables to avoid polluting the env, but the internals of the action shouldn't rely on the particular system being used.
I now realized that I wasn't clear at all in my explanations, sorry for that 😉
And you can't use actions/setup-python to installed a cached version of python 3.11?
Honestly, we can't be expected to support every docker image that people throw at us. If your docker image doesn't have python 3.11 installed, then your workflow needs to ensure it is installed.
I see, but I don't need python at all. It's just used in the internals of cpp-linter-action. For example, only by changing the version of python used in the action would basically break any container that does not have that installed.
And it's not any container, I'd say is the exact requirement specified in the README.
My concern is when users build with a python venv of their own (common practice for meson and bazel build system generators). We don't want cpp-linter-action deps to cause a conflict with build env's dependencies. By updating the environment, then we risk causing conflicts.
Reading the docs for update-environment, I don't thing this flag will update the LD_LIBRARY_PATH env var.
I think the problem is from trying to install one of cpp-linter's dependencies, pygit2, when using a docker image that isn't fully prepared to build python c-extensions from source. If pygit2 is installing from source (instead of from a compiled binary wheel), then cffi is used and would require the python3-dev and libffi-dev apt packages.
This is what update-environment does.
I mean, it gets solved by setting setup-python right before cpp-linter-action, but I'd say it should be included. At this point not executing setup-python inside cpp-linter-action has the same effect as doing it, since the libraries are not properly set I'd say.
At this point not executing setup-python inside cpp-linter-action has the same effect as doing it
This is not true. You are getting an error because you are using a docker image that is not well enough equipped for cpp-linter-action.
Tip
Using ubuntu-latest as the job's base image does not help when another docker image is specified.
since the libraries are not properly set I'd say.
I'm not exactly sure what you mean here. Installing our cpp-linter python package requires a few other third-party dependencies be installed. How those dependencies are distributed (binary wheels for pygit2 may not be available for all OS and architecture types) are not within our control. The dependencies and our python cpp-linter package are all installed by pip, which can have different behavior depending on the environment used.
If you don't know the difference between a binary wheel versus a source distribution of a python package (written in C or pure python), then I'd say that you have misdiagnosed the problem.
Looking at the minimal example I posted, here you have the line with the error. The problem appears when executing python (in the venv creation inside cpp-linter-action). Since CPython is being used, libpython must be correctly installed and set in the system.
libpython3.11.so.1.0: cannot open shared object file: No such file or directory
libpython3.11.so is the python library for building python libraries/packages from C sources. The only dependency written in C that we rely on is pygit2. Please add python3-dev and libffi-dev packages to you workflow's "Install missing deps" step. There may be other libraries missing (maybe libssl-dev) from the Ubuntu:22.04 docker image that you are using.
That's the first thing I tried, installing python3-dev and avoid opening a PR but the version in latest ubuntu LTS is python 3.10, so that does not help.
I as well tried if adding setup python was making any difference in the action, and tested against your test suite. It seems that is not making any difference.
My whole point is that python usage in the internals of cpp-linter-action relies on the system python (at least in Linux systems, of course), so having setup-python is not needed, or is not properly set up.
I'm sorry if I'm not being clear, or if you are fine with the current situation of relying on the system Python installation, that would be a design decision that is completely fine.
Thanks once again for the work in the project.
We are using actions/setup-python to copy a cached build of python at a specified version. We do this to avoid using any system python installed as we cannot garauntee the installed python version is compatible with cpp-linter pkg (v3.8+ should work fine).
Honestly, to solve your problem would require intimate understanding of exactly what is shipped with that docker image you are using. I'm not open to these changes because... #184 (comment)
Ultimately, there seems to be a problem with installing python C-extension package (likely pygit2). I would start there.
I would be open to an input option that specifies what version of python to use. But that seems like it would be specific to edge cases like this.
I understand your point. Even if it's just for cpp-linter, I'll set up python 3.11.
One thing that would be nice to have in the README is that Python 3.11 needs to be installed in the base image; that would extend the requirements and make it easier to understand.
Thanks for your time, have a nice day!
When
update-environmentis false, setup-python does not export LD_LIBRARY_PATH and PKG_CONFIG_PATH variables, which are required to find properlylibpython.soand this kind of libs. Without this change, it's required to have thelibpython311.soalready installed in the system, that kinda breaks the purpose of using setup-python.Thanks for the great work you do!