PowerShell: Fix assumption that $LASTEXITCODE is always defined (#1962)

Signed-off-by: Nathan Rusch <[email protected]>

Author: nrusch

src/rez/bind/hello_world.py

@@ -47,25 +47,31 @@ def bind(path, version_range=None, opts=None, parser=None):
     def make_root(variant, root):
         binpath = make_dirs(root, "bin")
         binpathtmp = make_dirs(root, "bintmp")
-        filepath = os.path.join(binpathtmp, "hello_world")
 
         create_executable_script(
-            filepath,
+            os.path.join(binpathtmp, "hello_world"),
             hello_world_source,
             py_script_mode=ExecutableScriptMode.single,
         )
+        create_executable_script(
+            os.path.join(binpathtmp, "hello_world_gui"),
+            hello_world_source,
+            program="pythonw",
+            py_script_mode=ExecutableScriptMode.single,
+        )
 
         # We want to use ScriptMaker on all platofrms. This allows us to
         # correctly setup the script to work everywhere, even on Windows.
         # create_executable_script should be fixed to use ScriptMaker
         # instead.
         maker = ScriptMaker(binpathtmp, make_dirs(binpath))
         maker.make("hello_world")
+        maker.make("hello_world_gui")
         shutil.rmtree(binpathtmp)
 
     with make_package("hello_world", path, make_root=make_root) as pkg:
         pkg.version = version
-        pkg.tools = ["hello_world"]
+        pkg.tools = ["hello_world", "hello_world_gui"]
         pkg.commands = commands
 
     return pkg.installed_variants

src/rez/data/tests/builds/packages/whack/package.py

@@ -4,4 +4,6 @@
 uuid = "4e9f63cbc4794453b0031f0c5ff50759"
 description = "a deliberately broken package"
 
+private_build_requires = ["python"]
+
 build_command = "python {root}/build.py {install}"

src/rez/tests/test_build.py

@@ -141,6 +141,7 @@ def test_build_whack(self, shell):
         """Test that a broken build fails correctly.
         """
         config.override("default_shell", shell)
+        self.inject_python_repo()
 
         working_dir = os.path.join(self.src_root, "whack")
         builder = self._create_builder(working_dir)

src/rez/tests/test_shells.py

@@ -11,8 +11,9 @@
 from rez.rex import literal, expandable
 from rez.plugin_managers import plugin_manager
 from rez.utils.execution import ExecutableScriptMode, _get_python_script_files
-from rez.tests.util import TestBase, TempdirMixin, per_available_shell, \
-    install_dependent
+from rez.utils.platform_ import platform_
+from rez.tests.util import TestBase, TempdirMixin, get_available_shells, \
+    per_available_shell, install_dependent
 from rez.bind import hello_world
 from rez.config import config
 import unittest
@@ -252,6 +253,39 @@ def test_command_returncode(self, shell):
                     p.wait()
                 self.assertEqual(p.returncode, 66)
 
+    @unittest.skipIf(platform_.name != "windows", "GUI entrypoint test is only relevant on Windows")
+    @unittest.skipIf("pwsh" not in get_available_shells(), "PowerShell unavailable or disabled")
+    def test_pwsh_lastexitcode_gui(self):
+        """This validates some semi-unintuitive behavior on Windows, where GUI applications
+        will "return" immediately without any exit status when launched from a shell.
+        """
+        sh = create_shell("pwsh")
+        _, _, _, command = sh.startup_capabilities(command=True)
+
+        if command:
+            def actions_callback(ex):
+                """Action callback to enable PowerShell's "strict" mode."""
+                ex.command("Set-StrictMode -version Latest")
+
+            r = self._create_context(["hello_world"])
+            command = "hello_world -q -r 66"
+            commands = (command, command.split())
+            for cmd in commands:
+                with r.execute_shell(shell="pwsh", command=cmd, actions_callback=actions_callback,
+                                     stdout=subprocess.PIPE) as p:
+                    p.wait()
+                self.assertEqual(p.returncode, 66)
+
+            command = "hello_world_gui -q -r 49"
+            commands = (command, command.split())
+            for cmd in commands:
+                with r.execute_shell(shell="pwsh", command=cmd, actions_callback=actions_callback,
+                                     stdout=subprocess.PIPE) as p:
+                    p.wait()
+                # The GUI application should return control to the shell immediately, and that
+                # should bubble up through the rez shell as a 0 exit status.
+                self.assertEqual(p.returncode, 0)
+
     @per_available_shell()
     def test_norc(self, shell):
         sh = create_shell(shell)

src/rez/tests/util.py

@@ -203,6 +203,21 @@ def wrapper(self, *args, **kwargs):
     return decorator
 
 
+def get_available_shells():
+    """Helper to get all available shells in a testing context."""
+    shells = get_shell_types()
+
+    only_shell = os.getenv("__REZ_SELFTEST_SHELL")
+    if only_shell:
+        shells = [only_shell]
+
+    # filter to only those shells available
+    return [
+        x for x in shells
+        if get_shell_class(x).is_available()
+    ]
+
+
 def per_available_shell(exclude=None):
     """Function decorator that runs the function over all available shell types.
     """

src/rezplugins/shell/_utils/powershell_base.py

@@ -146,17 +146,25 @@ def _record_shell(ex, files, bind_rez=True, print_msg=False):
         if shell_command:
             executor.command(shell_command)
 
-        # Forward exit call to parent PowerShell process
+        # Translate the status of the most recent command into an exit code.
         #
-        # Note that in powershell, $LASTEXITCODE is only set after running an
-        # executable - in other cases (such as when a command is not found),
-        # only the bool $? var is set.
+        # Note that in PowerShell, `$LASTEXITCODE` is only set after calling a
+        # native command (i.e. an executable), or another script that uses the
+        # `exit` keyword. Otherwise, only the boolean `$?` variable is set (to
+        # True if the last command succeeded and False if it failed).
+        # See https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_automatic_variables  # noqa
+        #
+        # Additionally, if PowerShell is running in strict mode, references to
+        # uninitialized variables will error instead of simply returning 0 or
+        # `$null`, so we use `Test-Path` here to verify that `$LASTEXITCODE` has
+        # been set before using it.
+        # See https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/set-strictmode?view=powershell-7.5#description  # noqa
         #
         executor.command(
-            "if(! $? -or $LASTEXITCODE) {\n"
-            "  if ($LASTEXITCODE) {\n"
-            "    exit $LASTEXITCODE\n"
-            "  }\n"
+            "if ((Test-Path variable:LASTEXITCODE) -and $LASTEXITCODE) {\n"
+            "  exit $LASTEXITCODE\n"
+            "}\n"
+            "if (! $?) {\n"
             "  exit 1\n"
             "}"
         )