Merge pull request #3466 from ActiveState/DX-3006

Update shell detection

internal/constants/constants.go

@@ -490,3 +490,6 @@ const OverrideSandbox = "ACTIVESTATE_TEST_OVERRIDE_SANDBOX"
 
 // PlatformPrivateNamespace is the namespace for private packages.
 const PlatformPrivateNamespace = "private"
+
+// OverrideShellEnvVarName is the environment variable to set when overriding the shell for shell detection.
+const OverrideShellEnvVarName = "ACTIVESTATE_CLI_SHELL_OVERRIDE"

internal/subshell/subshell.go

@@ -4,10 +4,12 @@ import (
 	"errors"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
 
+	"github.com/ActiveState/cli/internal/constants"
 	"github.com/ActiveState/cli/internal/rtutils/ptr"
 	"github.com/shirou/gopsutil/v3/process"
 
@@ -182,20 +184,24 @@ func DetectShell(cfg sscommon.Configurable) (string, string) {
 		}
 	}()
 
-	binary = os.Getenv("SHELL")
-	if binary == "" && runtime.GOOS == "windows" {
-		binary = detectShellWindows()
+	if os.Getenv(constants.OverrideShellEnvVarName) != "" {
+		binary = os.Getenv(constants.OverrideShellEnvVarName)
+	}
+
+	if binary == "" {
+		binary = detectShellParent()
 	}
 
 	if binary == "" {
 		binary = configured
 	}
+
 	if binary == "" {
-		if runtime.GOOS == "windows" {
-			binary = "cmd.exe"
-		} else {
-			binary = "bash"
-		}
+		binary = os.Getenv(SHELL_ENV_VAR)
+	}
+
+	if binary == "" {
+		binary = OS_DEFAULT
 	}
 
 	path := resolveBinaryPath(binary)
@@ -239,24 +245,24 @@ func DetectShell(cfg sscommon.Configurable) (string, string) {
 	return name, path
 }
 
-func detectShellWindows() string {
-	// Windows does not provide a way of identifying which shell we are running in, so we have to look at the parent
-	// process.
-
+func detectShellParent() string {
 	p, err := process.NewProcess(int32(os.Getppid()))
 	if err != nil && !errors.As(err, ptr.To(&os.PathError{})) {
-		panic(err)
+		multilog.Error("Failed to get parent process: %s", errs.JoinMessage(err))
 	}
 
-	for p != nil {
+	for p != nil && p.Pid != 0 {
 		name, err := p.Name()
 		if err == nil {
-			if strings.Contains(name, "cmd.exe") || strings.Contains(name, "powershell.exe") {
+			if strings.Contains(name, string(filepath.Separator)) {
+				name = path.Base(name)
+			}
+			if supportedShellName(name) {
 				return name
 			}
 		}
 		p, _ = p.Parent()
 	}
 
-	return os.Getenv("ComSpec")
+	return ""
 }

internal/subshell/subshell_lin_mac.go

@@ -4,6 +4,8 @@
 package subshell
 
 import (
+	"strings"
+
 	"github.com/ActiveState/cli/internal/subshell/bash"
 	"github.com/ActiveState/cli/internal/subshell/cmd"
 	"github.com/ActiveState/cli/internal/subshell/fish"
@@ -18,3 +20,17 @@ var supportedShells = []SubShell{
 	&fish.SubShell{},
 	&cmd.SubShell{},
 }
+
+const (
+	SHELL_ENV_VAR = "SHELL"
+	OS_DEFAULT    = "bash"
+)
+
+func supportedShellName(filename string) bool {
+	for _, subshell := range supportedShells {
+		if strings.EqualFold(filename, subshell.Shell()) {
+			return true
+		}
+	}
+	return false
+}

internal/subshell/subshell_win.go

@@ -3,8 +3,31 @@
 
 package subshell
 
-import "github.com/ActiveState/cli/internal/subshell/cmd"
+import (
+	"fmt"
+	"strings"
+
+	"github.com/ActiveState/cli/internal/subshell/bash"
+	"github.com/ActiveState/cli/internal/subshell/cmd"
+	"github.com/ActiveState/cli/internal/subshell/pwsh"
+)
 
 var supportedShells = []SubShell{
 	&cmd.SubShell{},
+	&pwsh.SubShell{},
+	&bash.SubShell{},
+}
+
+const (
+	SHELL_ENV_VAR = "COMSPEC"
+	OS_DEFAULT    = "cmd.exe"
+)
+
+func supportedShellName(filename string) bool {
+	for _, subshell := range supportedShells {
+		if strings.EqualFold(filename, fmt.Sprintf("%s.exe", subshell.Shell())) {
+			return true
+		}
+	}
+	return false
 }

internal/testhelpers/e2e/env_windows.go

@@ -17,7 +17,7 @@ const (
 )
 
 func platformSpecificEnv(dirs *Dirs) []string {
-	return []string{
+	env := []string{
 		"SystemDrive=C:",
 		"SystemRoot=C:\\Windows",
 		"PROGRAMFILES=C:\\Program Files",
@@ -39,6 +39,12 @@ func platformSpecificEnv(dirs *Dirs) []string {
 		fmt.Sprintf("LOCALAPPDATA=%s", dirs.TempDir),
 		fmt.Sprintf("%s=true", constants.DisableActivateEventsEnvVarName),
 	}
+
+	if condition.OnCI() {
+		env = append(env, fmt.Sprintf("%s=cmd.exe", constants.OverrideShellEnvVarName))
+	}
+
+	return env
 }
 
 func platformPath() string {

internal/testhelpers/e2e/spawn.go

@@ -74,7 +74,7 @@ func (s *SpawnedCmd) ExpectInput(opts ...termtest.SetExpectOpt) error {
 	expect := `expect'input from posix shell`
 	if cmdName != "bash" && shellName != "bash" && runtime.GOOS == "windows" {
 		if strings.Contains(cmdName, "powershell") || strings.Contains(shellName, "powershell") {
-			send = "echo \"`<expect input from powershell`>\""
+			send = "echo \"<expect input from powershell>\""
 			expect = `<expect input from powershell>`
 		} else {
 			send = `echo ^<expect input from cmd prompt^>`

test/integration/install_scripts_int_test.go

@@ -7,6 +7,9 @@ import (
 	"runtime"
 	"testing"
 
+	"github.com/ActiveState/cli/internal/config"
+	"github.com/ActiveState/cli/internal/rtutils/singlethread"
+	"github.com/ActiveState/cli/internal/subshell"
 	"github.com/ActiveState/cli/internal/testhelpers/suite"
 	"github.com/stretchr/testify/require"
 	"github.com/thoas/go-funk"
@@ -104,7 +107,10 @@ func (suite *InstallScriptsIntegrationTestSuite) TestInstall() {
 			}
 			if runtime.GOOS == "windows" {
 				cmd = "powershell.exe"
-				opts = append(opts, e2e.OptAppendEnv("SHELL="))
+				opts = append(opts,
+					e2e.OptAppendEnv("SHELL="),
+					e2e.OptAppendEnv(constants.OverrideShellEnvVarName+"="),
+				)
 			}
 			cp := ts.SpawnCmdWithOpts(cmd, opts...)
 			cp.Expect("Preparing Installer for State Tool Package Manager")
@@ -137,12 +143,19 @@ func (suite *InstallScriptsIntegrationTestSuite) TestInstall() {
 			suite.assertAnalytics(ts)
 			suite.DirExists(ts.Dirs.Config)
 
+			// Clear configured shell.
+			cfg, err := config.NewCustom(ts.Dirs.Config, singlethread.New(), true)
+			suite.Require().NoError(err)
+			err = cfg.Set(subshell.ConfigKeyShell, "")
+			suite.Require().NoError(err)
+
 			// Verify that can install overtop
 			if runtime.GOOS != "windows" {
 				cp = ts.SpawnCmdWithOpts("bash", e2e.OptArgs(argsPlain...))
 			} else {
 				cp = ts.SpawnCmdWithOpts("powershell.exe", e2e.OptArgs(argsPlain...),
 					e2e.OptAppendEnv("SHELL="),
+					e2e.OptAppendEnv(constants.OverrideShellEnvVarName+"="),
 				)
 			}
 			cp.Expect("successfully installed")

test/integration/shell_int_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/ActiveState/cli/internal/config"
 	"github.com/ActiveState/cli/internal/constants"
 	"github.com/ActiveState/cli/internal/fileutils"
+	"github.com/ActiveState/cli/internal/rtutils/singlethread"
 	"github.com/ActiveState/cli/internal/subshell"
 	"github.com/ActiveState/cli/internal/subshell/bash"
 	"github.com/ActiveState/cli/internal/subshell/sscommon"
@@ -489,14 +490,28 @@ func (suite *ShellIntegrationTestSuite) TestWindowsShells() {
 
 	hostname, err := os.Hostname()
 	suite.Require().NoError(err)
-	cp := ts.SpawnCmd("cmd", "/C", "state", "shell")
+	cp := ts.SpawnCmdWithOpts(
+		"cmd",
+		e2e.OptArgs("/C", "state", "shell"),
+		e2e.OptAppendEnv(constants.OverrideShellEnvVarName+"="),
+	)
 	cp.ExpectInput()
 	cp.SendLine("hostname")
 	cp.Expect(hostname) // cmd.exe shows the actual hostname
 	cp.SendLine("exit")
 	cp.ExpectExitCode(0)
 
-	cp = ts.SpawnCmd("powershell", "-Command", "state", "shell")
+	// Clear configured shell.
+	cfg, err := config.NewCustom(ts.Dirs.Config, singlethread.New(), true)
+	suite.Require().NoError(err)
+	err = cfg.Set(subshell.ConfigKeyShell, "")
+	suite.Require().NoError(err)
+
+	cp = ts.SpawnCmdWithOpts(
+		"powershell",
+		e2e.OptArgs("-Command", "state", "shell"),
+		e2e.OptAppendEnv(constants.OverrideShellEnvVarName+"="),
+	)
 	cp.ExpectInput()
 	cp.SendLine("$host.name")
 	cp.Expect("ConsoleHost") // powershell always shows ConsoleHost, go figure

test/integration/shells_int_test.go

@@ -6,6 +6,10 @@ import (
 	"runtime"
 	"testing"
 
+	"github.com/ActiveState/cli/internal/config"
+	"github.com/ActiveState/cli/internal/constants"
+	"github.com/ActiveState/cli/internal/rtutils/singlethread"
+	"github.com/ActiveState/cli/internal/subshell"
 	"github.com/ActiveState/cli/internal/testhelpers/suite"
 
 	"github.com/ActiveState/cli/internal/fileutils"
@@ -47,8 +51,17 @@ func (suite *ShellsIntegrationTestSuite) TestShells() {
 				suite.Require().NoError(err)
 			}
 
+			// Clear configured shell.
+			cfg, err := config.NewCustom(ts.Dirs.Config, singlethread.New(), true)
+			suite.Require().NoError(err)
+			err = cfg.Set(subshell.ConfigKeyShell, "")
+			suite.Require().NoError(err)
+
 			// Run the checkout in a particular shell.
-			cp = ts.SpawnShellWithOpts(shell)
+			cp = ts.SpawnShellWithOpts(
+				shell,
+				e2e.OptAppendEnv(constants.OverrideShellEnvVarName+"="),
+			)
 			cp.SendLine(e2e.QuoteCommand(shell, ts.ExecutablePath(), "checkout", "ActiveState-CLI/small-python", string(shell)))
 			cp.Expect("Checked out project")
 			cp.SendLine("exit")
@@ -58,7 +71,10 @@ func (suite *ShellsIntegrationTestSuite) TestShells() {
 
 			// There are 2 or more instances checked out, so we should get a prompt in whichever shell we
 			// use.
-			cp = ts.SpawnShellWithOpts(shell)
+			cp = ts.SpawnShellWithOpts(
+				shell,
+				e2e.OptAppendEnv(constants.OverrideShellEnvVarName+"="),
+			)
 			cp.SendLine(e2e.QuoteCommand(shell, ts.ExecutablePath(), "shell", "small-python"))
 			cp.Expect("Multiple project paths")