pyinstaller support added to PipEnv #19030
Conversation
davidsanfal
force-pushed
the
feature/pyinstaller-venv
branch
from
237122f to
0501a01
Compare
davidsanfal
force-pushed
the
feature/pyinstaller-venv
branch
from
0501a01 to
57efa02
Compare
| env.prepend_path("PATH", self.bin_dir) | ||
| env.vars(self._conanfile).save_script(self.env_name) | ||
|
|
||
| def _create_venv(self): |
There was a problem hiding this comment.
What about relying on a user configuration pointing to the python interpreter when not found in path?
conan/internal/model/conf.py
Outdated
| "tools.system.package_manager:mode": "Mode for package_manager tools: 'check', 'report', 'report-installed' or 'install'", | ||
| "tools.system.package_manager:sudo": "Use 'sudo' when invoking the package manager tools in Linux (False by default)", | ||
| "tools.system.package_manager:sudo_askpass": "Use the '-A' argument if using sudo in Linux to invoke the system package manager (False by default)", | ||
| "tools.system.pip_manager:python_interpreter": "Path to the Python interpreter to be used to create the virtualenv", |
There was a problem hiding this comment.
| "tools.system.pip_manager:python_interpreter": "Path to the Python interpreter to be used to create the virtualenv", | |
| "tools.system.pipenv:python_interpreter": "Path to the Python interpreter to be used to create the virtualenv", |
what about tools.system.pipenv:python_interpreter ?
There was a problem hiding this comment.
I used the same name as the python module, thinking it was our "naming standard". If we don't follow that pattern, I'll change it.
There was a problem hiding this comment.
I would say we don't have have a naming standard, In my opinion the most important rule is that is easy to find for users and the name is a bit self-explanatory to the extent possible? Check other confs to see if you think it could work
conan/tools/system/pip_manager.py
Outdated
| def _create_venv(self): | ||
| python_interpreter = self._conanfile.conf.get( | ||
| "tools.system.pipenv:python_interpreter", | ||
| default=shutil.which('python') if platform.system() == "Windows" else shutil.which('python3')) |
There was a problem hiding this comment.
Does the shutil.which() default to find the current interpreter if we are running in a venv already? Or it finds it directly in the system?
There was a problem hiding this comment.
the one inside the virtualenv. But it's a python that's only used during recipe execution. Every time you run it, it's a new environment. In any case, I'm going to add that it gets the real path because in virtualenv python is a symlink.
| python_interpreter = self._conanfile.conf.get("tools.system.pipenv:python_interpreter", | ||
| default=os.path.realpath(default_python) if default_python else None) | ||
| if not python_interpreter: | ||
| raise ConanException("PipEnv could not find a Python executable path.") |
There was a problem hiding this comment.
I think the error should give the user some hints about the solution. Something like this?
Could not find a usable Python interpreter to create a virtualenv.
Install Python system-wide or set the "tools.system.pipenv:python_interpreter" conf to the full path of a Python executable.
| default_python = shutil.which('python') if platform.system() == "Windows" else shutil.which('python3') | ||
| python_interpreter = self._conanfile.conf.get("tools.system.pipenv:python_interpreter", | ||
| default=os.path.realpath(default_python) if default_python else None) | ||
| if not python_interpreter: | ||
| raise ConanException("PipEnv could not find a Python executable path.") |
There was a problem hiding this comment.
Why doing shutil.which before checking the conf? I think it should first check the conf and then, if not defined go for shutil.which
| """ | ||
|
|
||
| venv.EnvBuilder(clear=True, with_pip=True).create(self._env_dir) | ||
| self._create_venv() |
There was a problem hiding this comment.
I think we should have at least a basic test testing with mocks that:
- The conf has priority
- The fallback depending on the OS
- Error message when no interpreter is found
4 participants
Changelog: Fix: Improve the python virtualenv creation using the python installed in the machine.
Docs: TODO
developbranch, documenting this one.