Switch to proving the proper hooks to PyInstaller

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>

changelog/61864.added

@@ -0,0 +1 @@
+Provide PyInstaller hooks that provide some runtime adjustments when Salt is running from a Tiamat(PyInstaller) bundled package.

salt/__init__.py

@@ -134,7 +134,3 @@ del __define_global_system_encoding_variable__
 
 # Import Salt's logging machinery
 import salt._logging.impl  # isort:skip  pylint: disable=unused-import
-
-# Do any nessessary patching when salt is running from a tiamat package
-# This code is temporary and will exist until we can handle this on salt-pkg
-import salt.utils.tiamatpkg  # isort:skip  pylint: disable=unused-import

salt/utils/pyinstaller/__init__.py

@@ -0,0 +1,21 @@
+"""
+This module exists to help PyInstaller bundle Salt
+"""
+import pathlib
+
+PYINSTALLER_UTILS_DIR_PATH = pathlib.Path(__file__).resolve().parent
+
+
+def get_hook_dirs():
+    """
+    Return a list of paths that PyInstaller can search for hooks.
+    """
+    hook_dirs = {PYINSTALLER_UTILS_DIR_PATH}
+    for path in PYINSTALLER_UTILS_DIR_PATH.iterdir():
+        if not path.is_dir():
+            continue
+        if "__pycache__" in path.parts:
+            continue
+        hook_dirs.add(path)
+
+    return sorted(str(p) for p in hook_dirs)

salt/utils/pyinstaller/rthooks.dat

@@ -0,0 +1,4 @@
+{
+    "subprocess": ["pyi_rth_subprocess.py"],
+    "salt.utils.vt": ["pyi_rth_salt.utils.vt.py"],
+}

salt/utils/pyinstaller/rthooks/__init__.py

@@ -0,0 +1,3 @@
+"""
+This package contains support code to package Salt with PyInstaller.
+"""

salt/utils/pyinstaller/rthooks/_overrides.py

@@ -0,0 +1,70 @@
+"""
+This package contains the runtime hooks support code for when Salt is pacakged with PyInstaller.
+"""
+import logging
+import os
+import subprocess
+import sys
+
+import salt.utils.vt
+
+log = logging.getLogger(__name__)
+
+
+def clean_pyinstaller_vars(environ):
+    """
+    Restore or cleanup PyInstaller specific environent variable behavior.
+    """
+    if environ is None:
+        environ = {}
+    # When Salt is bundled with tiamat, it MUST NOT contain LD_LIBRARY_PATH
+    # when shelling out, or, at least the value of LD_LIBRARY_PATH set by
+    # pyinstaller.
+    # See:
+    #  https://pyinstaller.readthedocs.io/en/stable/runtime-information.html#ld-library-path-libpath-considerations
+    for varname in ("LD_LIBRARY_PATH", "LIBPATH"):
+        original_varname = "{}_ORIG".format(varname)
+        if varname in environ and environ[varname] == sys._MEIPASS:
+            # If we find the varname on the user provided environment we need to at least
+            # check if it's not the value set by PyInstaller, if it is, remove it.
+            log.trace(
+                "User provided environment variable %r with value %r which is "
+                "the value that PyInstaller set's. Removing it",
+                varname,
+                environ[varname],
+            )
+            environ.pop(varname)
+
+        if original_varname in environ and varname not in environ:
+            # We found the original variable set by PyInstaller, and we didn't find
+            # any user provided variable, let's rename it.
+            log.trace(
+                "The %r variable was found in the passed environment, renaming it to %r",
+                original_varname,
+                varname,
+            )
+            environ[varname] = environ.pop(original_varname)
+
+        if varname not in environ:
+            if original_varname in os.environ:
+                log.trace(
+                    "Renaming environment variable %r to %r", original_varname, varname
+                )
+                environ[varname] = os.environ[original_varname]
+            elif varname in os.environ:
+                # Override the system environ variable with an empty one
+                log.trace("Setting environment variable %r to an empty string", varname)
+                environ[varname] = ""
+    return environ
+
+
+class PyinstallerPopen(subprocess.Popen):
+    def __init__(self, *args, **kwargs):
+        kwargs["env"] = clean_pyinstaller_vars(kwargs.pop("env", None))
+        super().__init__(*args, **kwargs)
+
+
+class PyinstallerTerminal(salt.utils.vt.Terminal):  # pylint: disable=abstract-method
+    def __init__(self, *args, **kwargs):
+        kwargs["env"] = clean_pyinstaller_vars(kwargs.pop("env", None))
+        super().__init__(*args, **kwargs)

salt/utils/pyinstaller/rthooks/pyi_rth_salt.utils.vt.py

@@ -0,0 +1,13 @@
+"""
+PyInstaller runtime hook to patch salt.utils.vt.Terminal
+"""
+import logging
+
+import salt.utils.vt
+from salt.utils.pyinstaller.rthooks._overrides import PyinstallerTerminal
+
+log = logging.getLogger(__name__)
+# Patch salt.utils.vt.Terminal when running within a pyinstalled bundled package
+salt.utils.vt.Terminal = PyinstallerTerminal
+
+log.debug("Replaced 'salt.utils.vt.Terminal' with 'PyinstallerTerminal'")

salt/utils/pyinstaller/rthooks/pyi_rth_subprocess.py

@@ -0,0 +1,13 @@
+"""
+PyInstaller runtime hook to patch subprocess.Popen
+"""
+import logging
+import subprocess
+
+from salt.utils.pyinstaller.rthooks._overrides import PyinstallerPopen
+
+log = logging.getLogger(__name__)
+# Patch subprocess.Popen when running within a pyinstalled bundled package
+subprocess.Popen = PyinstallerPopen
+
+log.debug("Replaced 'subprocess.Popen' with 'PyinstallerTerminal'")

salt/utils/tiamatpkg.py

@@ -1,51 +0,0 @@
-import os
-import subprocess
-import sys
-
-import salt.utils.vt
-from salt.utils.decorators import memoize
-
-# This code is temporary and will exist until we can handle this on salt-pkg
-
-
-@memoize
-def is_tiamat_packaged():
-    """
-    Returns True if salt is running from a tiamat pacakge, False otherwise
-    """
-    return hasattr(sys, "_MEIPASS")
-
-
-def _cleanup_environ(environ):
-    if environ is None:
-        environ = os.environ.copy()
-
-    # When Salt is bundled with tiamat, it MUST NOT contain LD_LIBRARY_PATH
-    # when shelling out, or, at least the value of LD_LIBRARY_PATH set by
-    # pyinstaller.
-    # See:
-    #  https://pyinstaller.readthedocs.io/en/stable/runtime-information.html#ld-library-path-libpath-considerations
-    for varname in ("LD_LIBRARY_PATH", "LIBPATH"):
-        original_varname = "{}_ORIG".format(varname)
-        if original_varname in environ:
-            environ[varname] = environ.pop(original_varname)
-        elif varname in environ:
-            environ.pop(varname)
-    return environ
-
-
-class TiamatPopen(subprocess.Popen):
-    def __init__(self, *args, **kwargs):
-        kwargs["env"] = _cleanup_environ(kwargs.pop("env", None))
-        super().__init__(*args, **kwargs)
-
-
-class TiamatTerminal(salt.utils.vt.Terminal):  # pylint: disable=abstract-method
-    def __init__(self, *args, **kwargs):
-        kwargs["env"] = _cleanup_environ(kwargs.pop("env", None))
-        super().__init__(*args, **kwargs)
-
-
-if is_tiamat_packaged():
-    subprocess.Popen = TiamatPopen
-    salt.utils.vt.Terminal = TiamatTerminal

setup.py

@@ -1258,6 +1258,11 @@ class SaltDistribution(distutils.dist.Distribution):
 
     @property
     def _property_entry_points(self):
+        entrypoints = {
+            "pyinstaller40": [
+                "hook-dirs = salt.utils.pyinstaller:get_hook_dirs",
+            ],
+        }
         # console scripts common to all scenarios
         scripts = [
             "salt-call = salt.scripts:salt_call",
@@ -1268,7 +1273,8 @@ class SaltDistribution(distutils.dist.Distribution):
             if IS_WINDOWS_PLATFORM:
                 return {"console_scripts": scripts}
             scripts.append("salt-cloud = salt.scripts:salt_cloud")
-            return {"console_scripts": scripts}
+            entrypoints["console_scripts"] = scripts
+            return entrypoints
 
         if IS_WINDOWS_PLATFORM:
             scripts.extend(
@@ -1281,7 +1287,8 @@ class SaltDistribution(distutils.dist.Distribution):
                     "spm = salt.scripts:salt_spm",
                 ]
             )
-            return {"console_scripts": scripts}
+            entrypoints["console_scripts"] = scripts
+            return entrypoints
 
         # *nix, so, we need all scripts
         scripts.extend(
@@ -1298,7 +1305,8 @@ class SaltDistribution(distutils.dist.Distribution):
                 "spm = salt.scripts:salt_spm",
             ]
         )
-        return {"console_scripts": scripts}
+        entrypoints["console_scripts"] = scripts
+        return entrypoints
 
     # <---- Dynamic Data ---------------------------------------------------------------------------------------------
 

tests/pytests/functional/utils/pyinstaller/rthooks/test_salt_utils_vt_terminal.py

@@ -0,0 +1,141 @@
+import json
+import os
+import sys
+
+import pytest
+import salt.utils.pyinstaller.rthooks._overrides as overrides
+from tests.support import mock
+from tests.support.helpers import PatchedEnviron
+
+
+@pytest.fixture(params=("LD_LIBRARY_PATH", "LIBPATH"))
+def envvar(request):
+    return request.param
+
+
+@pytest.fixture
+def meipass(envvar):
+    with mock.patch("salt.utils.pyinstaller.rthooks._overrides.sys") as patched_sys:
+        patched_sys._MEIPASS = envvar
+        assert overrides.sys._MEIPASS == envvar
+        yield "{}_VALUE".format(envvar)
+    assert not hasattr(sys, "_MEIPASS")
+    assert not hasattr(overrides.sys, "_MEIPASS")
+
+
+def test_vt_terminal_environ_cleanup_original(envvar, meipass):
+    orig_envvar = "{}_ORIG".format(envvar)
+    with PatchedEnviron(**{orig_envvar: meipass}):
+        original_env = dict(os.environ)
+        assert orig_envvar in original_env
+        instance = overrides.PyinstallerTerminal(
+            [
+                sys.executable,
+                "-c",
+                "import os, json; print(json.dumps(dict(os.environ)))",
+            ],
+            stream_stdout=False,
+            stream_stderr=False,
+        )
+        buffer_o = buffer_e = ""
+        while instance.has_unread_data:
+            stdout, stderr = instance.recv()
+            if stdout:
+                buffer_o += stdout
+            if stderr:
+                buffer_e += stderr
+        instance.terminate()
+
+        assert instance.exitstatus == 0
+        returned_env = json.loads(buffer_o)
+        assert returned_env != original_env
+        assert envvar in returned_env
+        assert orig_envvar not in returned_env
+        assert returned_env[envvar] == meipass
+
+
+def test_vt_terminal_environ_cleanup_original_passed_directly(envvar, meipass):
+    orig_envvar = "{}_ORIG".format(envvar)
+    env = {
+        orig_envvar: meipass,
+    }
+    original_env = dict(os.environ)
+
+    instance = overrides.PyinstallerTerminal(
+        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
+        env=env.copy(),
+        stream_stdout=False,
+        stream_stderr=False,
+    )
+    buffer_o = buffer_e = ""
+    while instance.has_unread_data:
+        stdout, stderr = instance.recv()
+        if stdout:
+            buffer_o += stdout
+        if stderr:
+            buffer_e += stderr
+    instance.terminate()
+
+    assert instance.exitstatus == 0
+    returned_env = json.loads(buffer_o)
+    assert returned_env != original_env
+    assert envvar in returned_env
+    assert orig_envvar not in returned_env
+    assert returned_env[envvar] == meipass
+
+
+def test_vt_terminal_environ_cleanup(envvar, meipass):
+    with PatchedEnviron(**{envvar: meipass}):
+        original_env = dict(os.environ)
+        assert envvar in original_env
+        instance = overrides.PyinstallerTerminal(
+            [
+                sys.executable,
+                "-c",
+                "import os, json; print(json.dumps(dict(os.environ)))",
+            ],
+            stream_stdout=False,
+            stream_stderr=False,
+        )
+        buffer_o = buffer_e = ""
+        while instance.has_unread_data:
+            stdout, stderr = instance.recv()
+            if stdout:
+                buffer_o += stdout
+            if stderr:
+                buffer_e += stderr
+        instance.terminate()
+
+        assert instance.exitstatus == 0
+        returned_env = json.loads(buffer_o)
+        assert returned_env != original_env
+        assert envvar in returned_env
+        assert returned_env[envvar] == ""
+
+
+def test_vt_terminal_environ_cleanup_passed_directly_not_removed(envvar, meipass):
+    env = {
+        envvar: meipass,
+    }
+    original_env = dict(os.environ)
+
+    instance = overrides.PyinstallerTerminal(
+        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
+        env=env.copy(),
+        stream_stdout=False,
+        stream_stderr=False,
+    )
+    buffer_o = buffer_e = ""
+    while instance.has_unread_data:
+        stdout, stderr = instance.recv()
+        if stdout:
+            buffer_o += stdout
+        if stderr:
+            buffer_e += stderr
+    instance.terminate()
+
+    assert instance.exitstatus == 0
+    returned_env = json.loads(buffer_o)
+    assert returned_env != original_env
+    assert envvar in returned_env
+    assert returned_env[envvar] == meipass

tests/pytests/functional/utils/pyinstaller/rthooks/test_subprocess.py

@@ -0,0 +1,110 @@
+import json
+import os
+import subprocess
+import sys
+
+import pytest
+import salt.utils.pyinstaller.rthooks._overrides as overrides
+from tests.support import mock
+from tests.support.helpers import PatchedEnviron
+
+
+@pytest.fixture(params=("LD_LIBRARY_PATH", "LIBPATH"))
+def envvar(request):
+    return request.param
+
+
+@pytest.fixture
+def meipass(envvar):
+    with mock.patch("salt.utils.pyinstaller.rthooks._overrides.sys") as patched_sys:
+        patched_sys._MEIPASS = envvar
+        assert overrides.sys._MEIPASS == envvar
+        yield "{}_VALUE".format(envvar)
+    assert not hasattr(sys, "_MEIPASS")
+    assert not hasattr(overrides.sys, "_MEIPASS")
+
+
+def test_subprocess_popen_environ_cleanup_original(envvar, meipass):
+    orig_envvar = "{}_ORIG".format(envvar)
+    with PatchedEnviron(**{orig_envvar: meipass}):
+        original_env = dict(os.environ)
+        assert orig_envvar in original_env
+        instance = overrides.PyinstallerPopen(
+            [
+                sys.executable,
+                "-c",
+                "import os, json; print(json.dumps(dict(os.environ)))",
+            ],
+            stdout=subprocess.PIPE,
+            universal_newlines=True,
+        )
+        stdout, _ = instance.communicate()
+        assert instance.returncode == 0
+        returned_env = json.loads(stdout)
+        assert returned_env != original_env
+        assert envvar in returned_env
+        assert orig_envvar not in returned_env
+        assert returned_env[envvar] == meipass
+
+
+def test_subprocess_popen_environ_cleanup_original_passed_directly(envvar, meipass):
+    orig_envvar = "{}_ORIG".format(envvar)
+    env = {
+        orig_envvar: meipass,
+    }
+    original_env = dict(os.environ)
+
+    instance = overrides.PyinstallerPopen(
+        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
+        env=env.copy(),
+        stdout=subprocess.PIPE,
+        universal_newlines=True,
+    )
+    stdout, _ = instance.communicate()
+    assert instance.returncode == 0
+    returned_env = json.loads(stdout)
+    assert returned_env != original_env
+    assert envvar in returned_env
+    assert orig_envvar not in returned_env
+    assert returned_env[envvar] == meipass
+
+
+def test_subprocess_popen_environ_cleanup(envvar, meipass):
+    with PatchedEnviron(**{envvar: meipass}):
+        original_env = dict(os.environ)
+        assert envvar in original_env
+        instance = overrides.PyinstallerPopen(
+            [
+                sys.executable,
+                "-c",
+                "import os, json; print(json.dumps(dict(os.environ)))",
+            ],
+            stdout=subprocess.PIPE,
+            universal_newlines=True,
+        )
+        stdout, _ = instance.communicate()
+        assert instance.returncode == 0
+        returned_env = json.loads(stdout)
+        assert returned_env != original_env
+        assert envvar in returned_env
+        assert returned_env[envvar] == ""
+
+
+def test_subprocess_popen_environ_cleanup_passed_directly_not_removed(envvar, meipass):
+    env = {
+        envvar: meipass,
+    }
+    original_env = dict(os.environ)
+
+    instance = overrides.PyinstallerPopen(
+        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
+        env=env.copy(),
+        stdout=subprocess.PIPE,
+        universal_newlines=True,
+    )
+    stdout, _ = instance.communicate()
+    assert instance.returncode == 0
+    returned_env = json.loads(stdout)
+    assert returned_env != original_env
+    assert envvar in returned_env
+    assert returned_env[envvar] == meipass

tests/pytests/functional/utils/test_tiamatpkg.py

@@ -1,103 +0,0 @@
-import json
-import subprocess
-import sys
-
-import pytest
-import salt.utils.tiamatpkg
-
-
-@pytest.fixture(params=("LD_LIBRARY_PATH", "LIBPATH"))
-def envvar(request):
-    return request.param
-
-
-def test_subprocess_popen_environ_cleanup_existing(envvar):
-    envvar_value = "foo"
-    orig_envvar = "{}_ORIG".format(envvar)
-    env = {
-        orig_envvar: envvar_value,
-    }
-    instance = salt.utils.tiamatpkg.TiamatPopen(
-        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
-        env=env.copy(),
-        stdout=subprocess.PIPE,
-    )
-    stdout, _ = instance.communicate()
-    assert instance.returncode == 0
-    returned_env = json.loads(stdout)
-    assert returned_env != env
-    assert envvar in returned_env
-    assert orig_envvar not in returned_env
-    assert returned_env[envvar] == envvar_value
-
-
-def test_subprocess_popen_environ_cleanup(envvar):
-    envvar_value = "foo"
-    env = {
-        envvar: envvar_value,
-    }
-    instance = salt.utils.tiamatpkg.TiamatPopen(
-        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
-        env=env.copy(),
-        stdout=subprocess.PIPE,
-    )
-    stdout, _ = instance.communicate()
-    assert instance.returncode == 0
-    returned_env = json.loads(stdout)
-    assert returned_env != env
-    assert envvar not in returned_env
-
-
-def test_vt_terminal_environ_cleanup_existing(envvar):
-    envvar_value = "foo"
-    orig_envvar = "{}_ORIG".format(envvar)
-    env = {
-        orig_envvar: envvar_value,
-    }
-    instance = salt.utils.tiamatpkg.TiamatTerminal(
-        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
-        env=env.copy(),
-        stream_stdout=False,
-        stream_stderr=False,
-    )
-    buffer_o = buffer_e = ""
-    while instance.has_unread_data:
-        stdout, stderr = instance.recv()
-        if stdout:
-            buffer_o += stdout
-        if stderr:
-            buffer_e += stderr
-    instance.terminate()
-
-    assert instance.exitstatus == 0
-    returned_env = json.loads(buffer_o)
-    assert returned_env != env
-    assert envvar in returned_env
-    assert orig_envvar not in returned_env
-    assert returned_env[envvar] == envvar_value
-
-
-def test_vt_terminal_environ_cleanup(envvar):
-    envvar_value = "foo"
-    env = {
-        envvar: envvar_value,
-    }
-    instance = salt.utils.tiamatpkg.TiamatTerminal(
-        [sys.executable, "-c", "import os, json; print(json.dumps(dict(os.environ)))"],
-        env=env.copy(),
-        stream_stdout=False,
-        stream_stderr=False,
-    )
-    buffer_o = buffer_e = ""
-    while instance.has_unread_data:
-        stdout, stderr = instance.recv()
-        if stdout:
-            buffer_o += stdout
-        if stderr:
-            buffer_e += stderr
-    instance.terminate()
-
-    assert instance.exitstatus == 0
-    returned_env = json.loads(buffer_o)
-    assert returned_env != env
-    assert envvar not in returned_env