Repository: saltstack/salt
Pull request: [3006.x] `salt-pip` related fixes
### What does this PR do?
* Add a `.pth` to the onedir env to ensure packages in extras are importable.
* Upgrade relenv

### What issues does this PR fix or reference?
Fixes: #64192

Problem statement:
[BUG] Salt Extensions Installed to Extras Location Are Not Accessible
**Description**
Beginning in 3006.0, any Salt extensions that are installed via PIP that are installed to the onedir extras directory are not accessible by Salt

**Steps to Reproduce the behavior**
Install a Salt extension via PIP that will be installed to the onedir extras directory

**Expected behavior**
Salt extension should be accessible.

Hints from issue discussion:
This is also impacting any libraries installed to the extras directory. For example if using `salt-pip` to install heist-salt:

```
(heist-3.8)  ch3ll@megan-precision5550  ~/onedir  ./salt/extras-3.10/bin/heist --version
./salt/extras-3.10/bin/heist: line 3: /home/ch3ll/onedir/salt/extras-3.10/bin/python3: No such file or directory
```

If I update the heist file to directly point to ./salt/bin/python3:

```
#!/bin/sh
"true" ''''
"exec" "/home/ch3ll/onedir/salt/bin/python3" "$0" "$@"
'''
# -*- coding: utf-8 -*-
import re
import sys
from distro.distro import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())
```

I get this error:

```
(heist-3.8)  ch3ll@megan-precision5550  ~/onedir/salt  ./extras-3.10/bin/heist --version
Traceback (most recent call last):
  File "/home/ch3ll/onedir/salt/./extras-3.10/bin/heist", line 8, in <module>
    from heist.scripts import start
ModuleNotFoundError: No module named 'heist'
```

This is because the extras directory is not in the sys.path:

```
(heist-3.8)  ch3ll@megan-precision5550  ~/onedir/salt  ./bin/python3
Python 3.10.11 (main, Apr 14 2023, 05:57:16) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path
['', '/home/ch3ll/onedir/salt/lib/python310.zip', '/home/ch3ll/onedir/salt/lib/python3.10', '/home/ch3ll/onedir/salt/lib/python3.10/lib-dynload', '/home/ch3ll/onedir/salt/lib/python3.10/site-packages']
>>> 
```

```
(heist-3.8)  ch3ll@megan-precision5550  ~/onedir/salt  ./bin/python3
Python 3.10.11 (main, Apr 14 2023, 05:57:16) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> import heist
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'heist'
>>> sys.path.append('/home/ch3ll/onedir/salt/extras-3.10')
>>> import heist  
```

I also tried the same thing with the distro package in extras

Commit message: Be sure to run the installed script to confirm it works

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

Patch to review:
diff --git a/changelog/64192.fixed.md b/changelog/64192.fixed.md
new file mode 100644
index 000000000000..00f9209e1c3c
--- /dev/null
+++ b/changelog/64192.fixed.md
@@ -0,0 +1 @@
+Add a `.pth` to the onedir env to ensure packages in extras are importable.
diff --git a/pkg/tests/conftest.py b/pkg/tests/conftest.py
index 9e6ea6fad51c..89f54fe2f72a 100644
--- a/pkg/tests/conftest.py
+++ b/pkg/tests/conftest.py
@@ -474,12 +474,17 @@ def extras_pypath():
     extras_dir = "extras-{}.{}".format(*sys.version_info)
     if platform.is_windows():
         return pathlib.Path(
-            os.getenv("ProgramFiles"), "Salt Project", "Salt", extras_dir, "bin"
+            os.getenv("ProgramFiles"), "Salt Project", "Salt", extras_dir
         )
     elif platform.is_darwin():
-        return pathlib.Path(f"/opt", "salt", extras_dir, "bin")
+        return pathlib.Path("/opt", "salt", extras_dir)
     else:
-        return pathlib.Path(f"/opt", "saltstack", "salt", extras_dir, "bin")
+        return pathlib.Path("/opt", "saltstack", "salt", extras_dir)
+
+
+@pytest.fixture(scope="module")
+def extras_pypath_bin(extras_pypath):
+    return extras_pypath / "bin"
 
 
 @pytest.fixture(scope="module")
@@ -487,7 +492,7 @@ def salt_api(salt_master, install_salt, extras_pypath):
     """
     start up and configure salt_api
     """
-    shutil.rmtree(str(extras_pypath.parent), ignore_errors=True)
+    shutil.rmtree(str(extras_pypath), ignore_errors=True)
     start_timeout = None
     if platform.is_windows() and install_salt.singlebin:
         start_timeout = 240
diff --git a/pkg/tests/integration/test_pip.py b/pkg/tests/integration/test_pip.py
index ea9e6a81b84b..7b4ccc0d04f3 100644
--- a/pkg/tests/integration/test_pip.py
+++ b/pkg/tests/integration/test_pip.py
@@ -13,25 +13,21 @@ def pypath():
     if platform.is_windows():
         return pathlib.Path(os.getenv("ProgramFiles"), "Salt Project", "Salt")
     elif platform.is_darwin():
-        return pathlib.Path(f"{os.sep}opt", "salt", "bin")
+        return pathlib.Path("/opt", "salt", "bin")
     else:
-        return pathlib.Path(f"{os.sep}opt", "saltstack", "salt", "bin")
+        return pathlib.Path("/opt", "saltstack", "salt", "bin")
 
 
 @pytest.fixture(autouse=True)
-def wipe_pydeps(install_salt, extras_pypath):
+def wipe_pydeps(shell, install_salt, extras_pypath):
     try:
         yield
     finally:
         # Note, uninstalling anything with an associated script will leave the script.
         # This is due to a bug in pip.
         for dep in ["pep8", "PyGithub"]:
-            subprocess.run(
-                install_salt.binary_paths["pip"] + ["uninstall", "-y", dep],
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                check=False,
-                universal_newlines=True,
+            shell.run(
+                *(install_salt.binary_paths["pip"] + ["uninstall", "-y", dep]),
             )
         shutil.rmtree(extras_pypath, ignore_errors=True)
 
@@ -56,32 +52,24 @@ def test_pip_install(salt_call_cli):
         assert "The github execution module cannot be loaded" in use_lib.stderr
 
 
-def test_pip_install_extras(install_salt, extras_pypath):
+def test_pip_install_extras(shell, install_salt, extras_pypath_bin):
     """
     Test salt-pip installs into the correct directory
     """
     dep = "pep8"
-    extras_keyword = "extras"
+    extras_keyword = "extras-3"
     if platform.is_windows():
-        check_path = extras_pypath / f"{dep}.exe"
+        check_path = extras_pypath_bin / f"{dep}.exe"
     else:
-        check_path = extras_pypath / dep
+        check_path = extras_pypath_bin / dep
 
-    install_ret = subprocess.run(
-        install_salt.binary_paths["pip"] + ["install", dep],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-    )
+    install_ret = shell.run(*(install_salt.binary_paths["pip"] + ["install", dep]))
     assert install_ret.returncode == 0
 
-    ret = subprocess.run(
-        install_salt.binary_paths["pip"] + ["list", "--format=json"],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-    )
+    ret = shell.run(*(install_salt.binary_paths["pip"] + ["list", "--format=json"]))
     assert ret.returncode == 0
-    pkgs_installed = json.loads(ret.stdout.strip().decode())
-    for pkg in pkgs_installed:
+    assert ret.data  # We can parse the JSON output
+    for pkg in ret.data:
         if pkg["name"] == dep:
             break
     else:
@@ -89,15 +77,14 @@ def test_pip_install_extras(install_salt, extras_pypath):
             f"The {dep!r} package was not found installed. Packages Installed: {pkgs_installed}"
         )
 
-    show_ret = subprocess.run(
-        install_salt.binary_paths["pip"] + ["show", dep],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-    )
+    show_ret = shell.run(*(install_salt.binary_paths["pip"] + ["show", dep]))
     assert show_ret.returncode == 0
-    assert extras_keyword in show_ret.stdout.decode()
+    assert extras_keyword in show_ret.stdout
     assert check_path.exists()
 
+    ret = shell.run(str(check_path), "--version")
+    assert ret.returncode == 0
+
 
 def demote(user_uid, user_gid):
     def result():
@@ -108,8 +95,8 @@ def result():
 
 
 @pytest.mark.skip_on_windows(reason="We can't easily demote users on Windows")
-def test_pip_non_root(install_salt, test_account, extras_pypath):
-    check_path = extras_pypath / "pep8"
+def test_pip_non_root(shell, install_salt, test_account, extras_pypath_bin):
+    check_path = extras_pypath_bin / "pep8"
     # We should be able to issue a --help without being root
     ret = subprocess.run(
         install_salt.binary_paths["salt"] + ["--help"],
diff --git a/tools/pkg/build.py b/tools/pkg/build.py
index b3f92ef615c1..119df65241ea 100644
--- a/tools/pkg/build.py
+++ b/tools/pkg/build.py
@@ -538,6 +538,14 @@ def errfn(fn, path, err):
             shutil.rmtree(onedir_env / "etc", onerror=errfn)
             shutil.rmtree(onedir_env / "Library", onerror=errfn)
 
+    # TODO: Fix hardcoded 3.10
+    dest_path = onedir_env / "lib" / "python3.10" / "site-packages" / "extras.pth"
+    ctx.info(f"Writing '{dest_path}' ...")
+    dest_path.write_text(
+        'import sys, pathlib; extras = str(pathlib.Path(__file__).parent.parent.parent / "extras-{}.{}".format(*sys.version_info)); '
+        "extras not in sys.path and sys.path.insert(0, extras)\n"
+    )
+
 
 def _check_pkg_build_files_exist(ctx: Context, **kwargs):
     for name, path in kwargs.items():


Code review comments:
diff_hunk: @@ -22,7 +22,7 @@ inputs:
     required: false
     type: string
     description: The version of relenv to use
-    default: 0.10.2
+    default: 0.12.0
path: .github/actions/setup-relenv/action.yml
text: @user1:
Why are these first 3 files being kept at 0.12.0 instead of 0.12.1 like the others?

@author:
Just because they weren't updated. FYI, those are the default values if no value is passed in(we always pass values)

Now that's no longer a real problem since I added checks to make sure that the relenv being used is the one we actually target.
Previously, not updating those files could mean that we would be using the wrong(older) version of relenv.

But again, those are the default value. It's a good hygiene to also update them, but it's not going to break anything if we don't.

@author:
Fixed