Skip to content
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

Hardcoded Python executable path depends upon system Python being installed #114

Closed
vps-eric opened this issue Apr 26, 2023 · 9 comments
Closed

Comments

@vps-eric
Copy link

vps-eric commented Apr 26, 2023

get_config_vars_wrapper(...) runs Python as a subprocess (more explanation on why this is necessary would be great). The path to Python to run in the subprocess is hard coded to /usr/bin/python3, which is not always installed and would require either a symlink to Salt's salt-pip or a system Python installation.

To reproduce, spin up Almalinux 8.7 (I do so in Vagrant on QEMU) and manually install the Salt minion, confirming there is no Python installed on the system. Then, log in and execute:

/opt/saltstack/salt/salt-pip install pycryptodome
Exception
Traceback (most recent call last):
  File "/opt/saltstack/salt/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/opt/saltstack/salt/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/__main__.py", line 29, in <module>
    from pip._internal.cli.main import main as _main
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/cli/main.py", line 9, in <module>
    from pip._internal.cli.autocompletion import autocomplete
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/cli/autocompletion.py", line 10, in <module>
    from pip._internal.cli.main_parser import create_main_parser
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/cli/main_parser.py", line 9, in <module>
    from pip._internal.build_env import get_runnable_pip
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/build_env.py", line 20, in <module>
    from pip._internal.cli.spinners import open_spinner
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/cli/spinners.py", line 9, in <module>
    from pip._internal.utils.logging import get_indentation
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/utils/logging.py", line 29, in <module>
    from pip._internal.utils.misc import ensure_dir
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/utils/misc.py", line 42, in <module>
    from pip._internal.locations import get_major_minor_version
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/locations/__init__.py", line 14, in <module>
    from . import _sysconfig
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/locations/_sysconfig.py", line 11, in <module>
    from .base import change_root, get_major_minor_version, is_osx_framework
  File "/opt/saltstack/salt/lib/python3.10/site-packages/pip/_internal/locations/base.py", line 16, in <module>
    site_packages: typing.Optional[str] = sysconfig.get_path("purelib")
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 572, in get_path
    return get_paths(scheme, vars, expand)[name]
  File "/opt/saltstack/salt/lib/python3.10/site-packages/relenv/runtime.py", line 167, in wrapped
    paths = func(scheme=scheme, vars=vars, expand=expand)
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 562, in get_paths
    return _expand_vars(scheme, vars)
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 218, in _expand_vars
    _extend_dict(vars, get_config_vars())
  File "/opt/saltstack/salt/lib/python3.10/site-packages/relenv/runtime.py", line 131, in wrapped
    _CONFIG_VARS = func()
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 640, in get_config_vars
    srcdir = os.path.dirname(get_makefile_filename())
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 399, in get_makefile_filename
    return os.path.join(get_path('stdlib'), config_dir_name, 'Makefile')
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 572, in get_path
    return get_paths(scheme, vars, expand)[name]
  File "/opt/saltstack/salt/lib/python3.10/site-packages/relenv/runtime.py", line 167, in wrapped
    paths = func(scheme=scheme, vars=vars, expand=expand)
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 562, in get_paths
    return _expand_vars(scheme, vars)
  File "/opt/saltstack/salt/lib/python3.10/sysconfig.py", line 218, in _expand_vars
    _extend_dict(vars, get_config_vars())
  File "/opt/saltstack/salt/lib/python3.10/site-packages/relenv/runtime.py", line 132, in wrapped
    p = subprocess.run(
  File "/opt/saltstack/salt/lib/python3.10/subprocess.py", line 503, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/opt/saltstack/salt/lib/python3.10/subprocess.py", line 971, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/opt/saltstack/salt/lib/python3.10/subprocess.py", line 1863, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '/usr/bin/python3'

This issue can be bypassed by:

  • installing Python on the system
  • creating a symlink:
    ln -s /opt/saltstack/salt/bin/python3 /usr/bin/python3
@vps-eric
Copy link
Author

As found in #115, the current best solution is not to symlink Salt's bundled Python. Instead, install Python on the system (so you will have two Python installations).

@dwoz
Copy link
Contributor

dwoz commented Apr 27, 2023

@vps-eric This is done for salt-pip by design. There are two ways you can install python packages that have C extensions. One is to compile the extension against the relenv's libraries. The other way is to compile c extensions against the system's libraries. We decided make the default install against the system's libraries because it tends to be easier for users. In this case you need a system python installed so that relenv can figure out the compiler options to use to compile against your system's libraries. If you want to install using relenv's libraries you need should set the RELENV_BUILDENV environment variable. This will cause relenv to build pycryptodome against relenv's openssl.

@lorengordon
Copy link

@dwoz Do you have some docs on RELENV_BUILDENV? I can't seem to turn anything up with a simple google search....

@vps-eric
Copy link
Author

@dwoz I appreciate the clarification, thank you. As someone who's brand new to this relenv idea, what is simplified for users by installing against system libraries? My impression is that I'd need to install dependencies on the system (finding system pip and then defining it in pip.installed states for example) and keep track of which dependencies are where (on the system or within relenv).

@dwoz
Copy link
Contributor

dwoz commented Apr 28, 2023

@dwoz I appreciate the clarification, thank you. As someone who's brand new to this relenv idea, what is simplified for users by installing against system libraries? My impression is that I'd need to install dependencies on the system (finding system pip and then defining it in pip.installed states for example) and keep track of which dependencies are where (on the system or within relenv).

If we were to default to using relenv's dependencies it would mean you would have to build any non-python dependencies from source and install them into relenv's lib directory. That means you'd be compiling libssh2 and libgit2 to be able to install pygit2, for example. Another case would be having to build openldap to install python-ldap. Versus, by defaulting to the system packages you are able to install those dependencies from your OS with yum, apt, ect.

@vps-eric
Copy link
Author

vps-eric commented May 1, 2023

Given that relying on system Python is required, I propose that the hardcoded path is still removed and that it uses PATH instead, or gives a useful error message for the user.

@dwoz
Copy link
Contributor

dwoz commented May 2, 2023

Given that relying on system Python is required, I propose that the hardcoded path is still removed and that it uses PATH instead, or gives a useful error message for the user.

I agree that pulling the 'system' python from the path is a better way forward. I'm also going to provide pip some sane defaults for when no system python is present.

@dwoz
Copy link
Contributor

dwoz commented May 3, 2023

Fixed in 0.12.0

@dwoz dwoz closed this as completed May 3, 2023
@OrangeDog
Copy link

Certainly some of the libraries you compile against have to be relenv's. Because those are the libraries it's going to run with and may not be ABI-compatible with the system ones. The version of Python, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants