From a541ffaae3044aa1adfffff981ea3a7248b03c81 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 27 Aug 2024 14:24:55 -0700 Subject: [PATCH 01/38] Update shell detection --- internal/subshell/subshell.go | 37 +++++++++++++++------------ internal/subshell/subshell_lin_mac.go | 5 ++++ internal/subshell/subshell_win.go | 5 ++++ ssTest/main.go | 30 ++++++++++++++++++++++ 4 files changed, 60 insertions(+), 17 deletions(-) create mode 100644 ssTest/main.go diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index ac590f43b4..7992f1cde8 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -182,20 +182,17 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } }() - binary = os.Getenv("SHELL") - if binary == "" && runtime.GOOS == "windows" { - binary = detectShellWindows() + binary = configured + if binary == "" { + binary = detectShellParent() } if binary == "" { - binary = configured + binary = os.Getenv(SHELL_ENV_VAR) } + if binary == "" { - if runtime.GOOS == "windows" { - binary = "cmd.exe" - } else { - binary = "bash" - } + binary = OS_DEFULAT } path := resolveBinaryPath(binary) @@ -239,24 +236,30 @@ 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) + logging.Error("Failed to get parent process: %v", 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 supportedShellName(name) { return name } } p, _ = p.Parent() } - return os.Getenv("ComSpec") + return "" +} + +func supportedShellName(name string) bool { + for _, subshell := range supportedShells { + if name == subshell.Shell() { + return true + } + } + return false } diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index bcf296d137..02e6178e6b 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/internal/subshell/subshell_lin_mac.go @@ -18,3 +18,8 @@ var supportedShells = []SubShell{ &fish.SubShell{}, &cmd.SubShell{}, } + +const ( + SHELL_ENV_VAR = "SHELL" + OS_DEFULAT = "bash" +) diff --git a/internal/subshell/subshell_win.go b/internal/subshell/subshell_win.go index 12df8f51e2..992ed7f5ce 100644 --- a/internal/subshell/subshell_win.go +++ b/internal/subshell/subshell_win.go @@ -8,3 +8,8 @@ import "github.com/ActiveState/cli/internal/subshell/cmd" var supportedShells = []SubShell{ &cmd.SubShell{}, } + +const ( + SHELL_ENV_VAR = "COMSPEC" + OS_DEFULAT = "cmd.exe" +) diff --git a/ssTest/main.go b/ssTest/main.go new file mode 100644 index 0000000000..ba3d364242 --- /dev/null +++ b/ssTest/main.go @@ -0,0 +1,30 @@ +package main + +import ( + "fmt" + + "github.com/ActiveState/cli/internal/config" + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/internal/subshell" +) + +func main() { + if err := run(); err != nil { + panic(err) + } +} + +func run() error { + fmt.Println("Setting up configuration") + cfg, err := config.New() + if err != nil { + return errs.Wrap(err, "Could not create configuration") + } + + fmt.Println("Detecting shell") + shell, path := subshell.DetectShell(cfg) + fmt.Println("Found shell:", shell) + fmt.Println("Path:", path) + + return nil +} From 1807ceeec55cbf5153ef2a9e90cd0f6570562ec6 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Tue, 27 Aug 2024 14:41:00 -0700 Subject: [PATCH 02/38] Fix Windows detection --- internal/subshell/subshell.go | 9 --------- internal/subshell/subshell_lin_mac.go | 9 +++++++++ internal/subshell/subshell_win.go | 17 ++++++++++++++++- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 7992f1cde8..27cbeb2a94 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -254,12 +254,3 @@ func detectShellParent() string { return "" } - -func supportedShellName(name string) bool { - for _, subshell := range supportedShells { - if name == subshell.Shell() { - return true - } - } - return false -} diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index 02e6178e6b..b8bc036e12 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/internal/subshell/subshell_lin_mac.go @@ -23,3 +23,12 @@ const ( SHELL_ENV_VAR = "SHELL" OS_DEFULAT = "bash" ) + +func supportedShellName(name string) bool { + for _, subshell := range supportedShells { + if name == subshell.Shell() { + return true + } + } + return false +} \ No newline at end of file diff --git a/internal/subshell/subshell_win.go b/internal/subshell/subshell_win.go index 992ed7f5ce..7e976841b7 100644 --- a/internal/subshell/subshell_win.go +++ b/internal/subshell/subshell_win.go @@ -3,13 +3,28 @@ package subshell -import "github.com/ActiveState/cli/internal/subshell/cmd" +import ( + "fmt" + + "github.com/ActiveState/cli/internal/subshell/cmd" + "github.com/ActiveState/cli/internal/subshell/pwsh" +) var supportedShells = []SubShell{ &cmd.SubShell{}, + &pwsh.SubShell{}, } const ( SHELL_ENV_VAR = "COMSPEC" OS_DEFULAT = "cmd.exe" ) + +func supportedShellName(name string) bool { + for _, subshell := range supportedShells { + if name == fmt.Sprintf("%s.exe", subshell.Shell()) { + return true + } + } + return false +} From 958ba7fc65d22e90ca7182e685a4dcec1abef2d4 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 27 Aug 2024 14:55:23 -0700 Subject: [PATCH 03/38] Remove test code --- ssTest/main.go | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 ssTest/main.go diff --git a/ssTest/main.go b/ssTest/main.go deleted file mode 100644 index ba3d364242..0000000000 --- a/ssTest/main.go +++ /dev/null @@ -1,30 +0,0 @@ -package main - -import ( - "fmt" - - "github.com/ActiveState/cli/internal/config" - "github.com/ActiveState/cli/internal/errs" - "github.com/ActiveState/cli/internal/subshell" -) - -func main() { - if err := run(); err != nil { - panic(err) - } -} - -func run() error { - fmt.Println("Setting up configuration") - cfg, err := config.New() - if err != nil { - return errs.Wrap(err, "Could not create configuration") - } - - fmt.Println("Detecting shell") - shell, path := subshell.DetectShell(cfg) - fmt.Println("Found shell:", shell) - fmt.Println("Path:", path) - - return nil -} From 2a9fbc2571da4a429894d99c11bade6b720d72c1 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 27 Aug 2024 15:16:30 -0700 Subject: [PATCH 04/38] Add newline --- internal/subshell/subshell_lin_mac.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index b8bc036e12..fe66d31c41 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/internal/subshell/subshell_lin_mac.go @@ -31,4 +31,4 @@ func supportedShellName(name string) bool { } } return false -} \ No newline at end of file +} From 14e274f868774d76842caa90232327bd94e1badd Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 27 Aug 2024 15:49:13 -0700 Subject: [PATCH 05/38] Debug test failures --- internal/subshell/subshell.go | 1 + internal/subshell/subshell_lin_mac.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 27cbeb2a94..f2815b3df5 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -245,6 +245,7 @@ func detectShellParent() string { for p != nil && p.Pid != 0 { name, err := p.Name() if err == nil { + logging.Debug("Searching for supported shell in parent process: %s", name) if supportedShellName(name) { return name } diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index fe66d31c41..b271543206 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/internal/subshell/subshell_lin_mac.go @@ -4,6 +4,7 @@ package subshell import ( + "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/subshell/bash" "github.com/ActiveState/cli/internal/subshell/cmd" "github.com/ActiveState/cli/internal/subshell/fish" @@ -26,6 +27,7 @@ const ( func supportedShellName(name string) bool { for _, subshell := range supportedShells { + logging.Debug("Shell name: %s", subshell.Shell()) if name == subshell.Shell() { return true } From 27932e13978acdb2f0766f6ba056e55e836f6257 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 27 Aug 2024 16:00:41 -0700 Subject: [PATCH 06/38] Debug --- internal/subshell/subshell.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index f2815b3df5..848a3d924d 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -183,6 +183,7 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { }() binary = configured + logging.Debug("Configured shell: %s", binary) if binary == "" { binary = detectShellParent() } @@ -237,6 +238,7 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } func detectShellParent() string { + logging.Debug("Detecting shell from parent process") p, err := process.NewProcess(int32(os.Getppid())) if err != nil && !errors.As(err, ptr.To(&os.PathError{})) { logging.Error("Failed to get parent process: %v", err) From e4f6e33f518bf51f68b28c226a58f8b78191010f Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 27 Aug 2024 16:16:30 -0700 Subject: [PATCH 07/38] Change order in shell detection --- internal/subshell/subshell.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 848a3d924d..b95e8713e8 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -182,10 +182,10 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } }() - binary = configured + binary = detectShellParent() logging.Debug("Configured shell: %s", binary) if binary == "" { - binary = detectShellParent() + binary = configured } if binary == "" { From 3f56d391388b64c0a4cf449c6c8f378e037a0737 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 28 Aug 2024 09:15:09 -0700 Subject: [PATCH 08/38] Add bash support on Windows --- internal/subshell/subshell_win.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/subshell/subshell_win.go b/internal/subshell/subshell_win.go index 7e976841b7..6ad787901f 100644 --- a/internal/subshell/subshell_win.go +++ b/internal/subshell/subshell_win.go @@ -6,6 +6,7 @@ package subshell import ( "fmt" + "github.com/ActiveState/cli/internal/subshell/bash" "github.com/ActiveState/cli/internal/subshell/cmd" "github.com/ActiveState/cli/internal/subshell/pwsh" ) @@ -13,6 +14,7 @@ import ( var supportedShells = []SubShell{ &cmd.SubShell{}, &pwsh.SubShell{}, + &bash.SubShell{}, } const ( From 84ba928c98eb7f0f4bede47c4a23b33e2bdb1aa7 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 28 Aug 2024 09:58:03 -0700 Subject: [PATCH 09/38] Remove debug logs --- internal/subshell/subshell.go | 2 -- internal/subshell/subshell_lin_mac.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index b95e8713e8..6c491bce9e 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -238,7 +238,6 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } func detectShellParent() string { - logging.Debug("Detecting shell from parent process") p, err := process.NewProcess(int32(os.Getppid())) if err != nil && !errors.As(err, ptr.To(&os.PathError{})) { logging.Error("Failed to get parent process: %v", err) @@ -247,7 +246,6 @@ func detectShellParent() string { for p != nil && p.Pid != 0 { name, err := p.Name() if err == nil { - logging.Debug("Searching for supported shell in parent process: %s", name) if supportedShellName(name) { return name } diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index b271543206..fe66d31c41 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/internal/subshell/subshell_lin_mac.go @@ -4,7 +4,6 @@ package subshell import ( - "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/subshell/bash" "github.com/ActiveState/cli/internal/subshell/cmd" "github.com/ActiveState/cli/internal/subshell/fish" @@ -27,7 +26,6 @@ const ( func supportedShellName(name string) bool { for _, subshell := range supportedShells { - logging.Debug("Shell name: %s", subshell.Shell()) if name == subshell.Shell() { return true } From efe42e92bc88afd348012d52c1fd2f80ed5f3de4 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Wed, 28 Aug 2024 11:23:53 -0700 Subject: [PATCH 10/38] Add back debug logging --- internal/subshell/subshell.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 6c491bce9e..b95e8713e8 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -238,6 +238,7 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } func detectShellParent() string { + logging.Debug("Detecting shell from parent process") p, err := process.NewProcess(int32(os.Getppid())) if err != nil && !errors.As(err, ptr.To(&os.PathError{})) { logging.Error("Failed to get parent process: %v", err) @@ -246,6 +247,7 @@ func detectShellParent() string { for p != nil && p.Pid != 0 { name, err := p.Name() if err == nil { + logging.Debug("Searching for supported shell in parent process: %s", name) if supportedShellName(name) { return name } From 46e9a0048da4778e3347632be42a0e6d3fe1219d Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 28 Aug 2024 12:53:31 -0700 Subject: [PATCH 11/38] Set shell on Windows --- internal/testhelpers/e2e/session.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index 030e279c23..f84b71007f 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -189,6 +189,12 @@ func new(t *testing.T, retainDirs, updatePath bool, extraEnv ...string) *Session require.NoError(session.T, err) } + if runtime.GOOS == "windows" { + if err := cfg.Set(subshell.ConfigKeyShell, "cmd.exe"); err != nil { + require.NoError(session.T, err) + } + } + return session } From 9882ea8dcec7c0ba5d2ae83bfacdc5797293df47 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 29 Aug 2024 13:50:23 -0700 Subject: [PATCH 12/38] Run Windows CI tests inside subshell --- internal/testhelpers/e2e/session.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index f84b71007f..032d4c911b 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -261,6 +261,14 @@ func (s *Session) SpawnCmdWithOpts(exe string, optSetters ...SpawnOptSetter) *Sp termtest.OptNormalizedLineEnds(true), ) + if runtime.GOOS == "windows" && condition.OnCI() { + // Our Windows integration tests are run on bash. Due to the way the PTY library runs a new + // command we need to run the command inside a shell in order to setup the correct process + // tree. Without this integration tests run in bash will incorrectly identify the partent shell + // as bash, rather than the actual shell that is running the command. + spawnOpts.RunInsideShell = true + } + for _, optSet := range optSetters { optSet(&spawnOpts) } From d1a9f8ae46386e9c36714e6427a646b82ee49627 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 29 Aug 2024 15:31:03 -0700 Subject: [PATCH 13/38] Attempt to fix Windows tests --- .../test/integration/installer_int_test.go | 2 ++ internal/testhelpers/e2e/session.go | 22 ++++++++++++------- internal/testhelpers/e2e/spawn.go | 11 ++++++++++ test/integration/activate_int_test.go | 17 ++++++++------ test/integration/deploy_int_test.go | 1 + test/integration/install_scripts_int_test.go | 2 +- test/integration/shell_int_test.go | 2 +- 7 files changed, 40 insertions(+), 17 deletions(-) diff --git a/cmd/state-installer/test/integration/installer_int_test.go b/cmd/state-installer/test/integration/installer_int_test.go index 1f83eaa242..8b6b99c21b 100644 --- a/cmd/state-installer/test/integration/installer_int_test.go +++ b/cmd/state-installer/test/integration/installer_int_test.go @@ -44,6 +44,7 @@ func (suite *InstallerIntegrationTestSuite) TestInstallFromLocalSource() { e2e.OptArgs(installationDir(ts), "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=false"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.OverwriteDefaultSystemPathEnvVarName, dir)), + e2e.OptMaybeRunInsideShell(), ) // Assert output @@ -176,6 +177,7 @@ func (suite *InstallerIntegrationTestSuite) TestInstallErrorTips() { e2e.OptArgs(installationDir(ts), "--activate", "ActiveState-CLI/Python3", "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=true"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.OverwriteDefaultSystemPathEnvVarName, dir)), + e2e.OptMaybeRunInsideShell(), ) cp.ExpectInput(e2e.RuntimeSourcingTimeoutOpt) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index 032d4c911b..7db033dec0 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -234,6 +234,20 @@ func (s *Session) SpawnShellWithOpts(shell Shell, opts ...SpawnOptSetter) *Spawn return s.SpawnCmdWithOpts(string(shell), opts...) } +// SpawnShell spawns the state tool executable to be tested with arguments. +// This function differs from Spawn in that it runs the command in a shell on Windows CI. +// Our Windows integration tests are run on bash. Due to the way the PTY library runs a new +// command we need to run the command inside a shell in order to setup the correct process +// tree. Without this integration tests run in bash will incorrectly identify the partent shell +// as bash, rather than the actual shell that is running the command +func (s *Session) SpawnShell(args ...string) *SpawnedCmd { + opts := []SpawnOptSetter{OptArgs(args...)} + if runtime.GOOS == "windows" && condition.OnCI() { + opts = append(opts, OptRunInsideShell(true)) + } + return s.SpawnCmdWithOpts(s.Exe, opts...) +} + // SpawnCmdWithOpts executes an executable in a pseudo-terminal for integration tests // Arguments and other parameters can be specified by specifying SpawnOptSetter func (s *Session) SpawnCmdWithOpts(exe string, optSetters ...SpawnOptSetter) *SpawnedCmd { @@ -261,14 +275,6 @@ func (s *Session) SpawnCmdWithOpts(exe string, optSetters ...SpawnOptSetter) *Sp termtest.OptNormalizedLineEnds(true), ) - if runtime.GOOS == "windows" && condition.OnCI() { - // Our Windows integration tests are run on bash. Due to the way the PTY library runs a new - // command we need to run the command inside a shell in order to setup the correct process - // tree. Without this integration tests run in bash will incorrectly identify the partent shell - // as bash, rather than the actual shell that is running the command. - spawnOpts.RunInsideShell = true - } - for _, optSet := range optSetters { optSet(&spawnOpts) } diff --git a/internal/testhelpers/e2e/spawn.go b/internal/testhelpers/e2e/spawn.go index bf73ffcd22..bcc466ea9c 100644 --- a/internal/testhelpers/e2e/spawn.go +++ b/internal/testhelpers/e2e/spawn.go @@ -10,6 +10,7 @@ import ( "github.com/ActiveState/termtest" + "github.com/ActiveState/cli/internal/condition" "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/osutils" ) @@ -180,3 +181,13 @@ func OptRunInsideShell(v bool) SpawnOptSetter { opts.RunInsideShell = v } } + +func OptMaybeRunInsideShell() SpawnOptSetter { + if runtime.GOOS == "windows" && condition.OnCI() { + return func(opts *SpawnOpts) { + opts.RunInsideShell = true + } + } + + return func(opts *SpawnOpts) {} +} diff --git a/test/integration/activate_int_test.go b/test/integration/activate_int_test.go index d91ed46918..9f34c6daa3 100644 --- a/test/integration/activate_int_test.go +++ b/test/integration/activate_int_test.go @@ -54,7 +54,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateWithoutRuntime() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.Spawn("activate", "ActiveState-CLI/Empty") + cp := ts.SpawnShell("activate", "ActiveState-CLI/Empty") cp.Expect("Activated") cp.ExpectInput() @@ -134,7 +134,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateUsingCommitID() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.Spawn("activate", "ActiveState-CLI/Empty#6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8", "--path", ts.Dirs.Work) + cp := ts.SpawnShell("activate", "ActiveState-CLI/Empty#6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8", "--path", ts.Dirs.Work) cp.Expect("Activated") cp.ExpectInput() @@ -149,7 +149,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateNotOnPath() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.Spawn("activate", "activestate-cli/empty", "--path", ts.Dirs.Work) + cp := ts.SpawnShell("activate", "activestate-cli/empty", "--path", ts.Dirs.Work) cp.Expect("Activated") cp.ExpectInput() @@ -177,7 +177,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivatePythonByHostOnly() { defer close() projectName := "Python-LinuxWorks" - cp := ts.Spawn("activate", "cli-integration-tests/"+projectName, "--path="+ts.Dirs.Work) + cp := ts.SpawnShell("activate", "cli-integration-tests/"+projectName, "--path="+ts.Dirs.Work) if runtime.GOOS == "linux" { cp.Expect("Creating a Virtual Environment") @@ -221,6 +221,7 @@ func (suite *ActivateIntegrationTestSuite) activatePython(version string, extraE cp := ts.SpawnWithOpts( e2e.OptArgs("activate", namespace), e2e.OptAppendEnv(extraEnv...), + e2e.OptMaybeRunInsideShell(), ) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) @@ -294,7 +295,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_PythonPath() { namespace := "ActiveState-CLI/Python3" - cp := ts.Spawn("activate", namespace) + cp := ts.SpawnShell("activate", namespace) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) // ensure that shell is functional @@ -337,6 +338,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_SpaceInCacheDir() { cp := ts.SpawnWithOpts( e2e.OptArgs("activate", "ActiveState-CLI/Python3"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.CacheEnvVarName, cacheDir)), + e2e.OptMaybeRunInsideShell(), ) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) @@ -358,7 +360,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivatePerlCamel() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.Spawn("activate", "ActiveState-CLI/Perl") + cp := ts.SpawnShell("activate", "ActiveState-CLI/Perl") cp.Expect("Downloading", termtest.OptExpectTimeout(40*time.Second)) cp.Expect("Installing", termtest.OptExpectTimeout(140*time.Second)) @@ -402,6 +404,7 @@ version: %s c2 := ts.SpawnWithOpts( e2e.OptArgs("activate"), e2e.OptWD(filepath.Join(ts.Dirs.Work, "foo", "bar", "baz")), + e2e.OptMaybeRunInsideShell(), ) c2.Expect("Activated") @@ -474,7 +477,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_FromCache() { // Note: cannot use Empty project since we need artifacts to download and install. // Pick the langless project, which just has some small, non-language artifacts. - cp := ts.Spawn("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) + cp := ts.SpawnShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) suite.assertCompletedStatusBarReport(cp.Output()) diff --git a/test/integration/deploy_int_test.go b/test/integration/deploy_int_test.go index f51e686baa..28b2621daf 100644 --- a/test/integration/deploy_int_test.go +++ b/test/integration/deploy_int_test.go @@ -168,6 +168,7 @@ func (suite *DeployIntegrationTestSuite) TestDeployPython() { "cmd.exe", e2e.OptArgs("/k", filepath.Join(targetPath, "bin", "shell.bat")), e2e.OptAppendEnv("PATHEXT=.COM;.EXE;.BAT;.LNK", "SHELL="), + e2e.OptRunInsideShell(true), ) } else { cp = ts.SpawnCmdWithOpts( diff --git a/test/integration/install_scripts_int_test.go b/test/integration/install_scripts_int_test.go index 8846c44353..4f0b866d9b 100644 --- a/test/integration/install_scripts_int_test.go +++ b/test/integration/install_scripts_int_test.go @@ -104,7 +104,7 @@ func (suite *InstallScriptsIntegrationTestSuite) TestInstall() { } if runtime.GOOS == "windows" { cmd = "powershell.exe" - opts = append(opts, e2e.OptAppendEnv("SHELL=")) + opts = append(opts, e2e.OptAppendEnv("SHELL="), e2e.OptRunInsideShell(true)) } cp := ts.SpawnCmdWithOpts(cmd, opts...) cp.Expect("Preparing Installer for State Tool Package Manager") diff --git a/test/integration/shell_int_test.go b/test/integration/shell_int_test.go index 77183146e1..8faf38257f 100644 --- a/test/integration/shell_int_test.go +++ b/test/integration/shell_int_test.go @@ -466,7 +466,7 @@ events:`, lang, splat), 1) suite.Require().NoError(fileutils.WriteFile(asyFilename, []byte(contents))) // Verify that running a script as a command with an argument containing special characters works. - cp = ts.Spawn("shell") + cp = ts.SpawnShell("shell") cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) cp.ExpectInput() cp.SendLine(`args "<3"`) From 976c6fc016f24c98ee613e118e3ca568badb6454 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 29 Aug 2024 16:04:25 -0700 Subject: [PATCH 14/38] Address more tests --- cmd/state-installer/test/integration/installer_int_test.go | 1 + test/integration/activate_int_test.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/state-installer/test/integration/installer_int_test.go b/cmd/state-installer/test/integration/installer_int_test.go index 8b6b99c21b..b3231d0165 100644 --- a/cmd/state-installer/test/integration/installer_int_test.go +++ b/cmd/state-installer/test/integration/installer_int_test.go @@ -62,6 +62,7 @@ func (suite *InstallerIntegrationTestSuite) TestInstallFromLocalSource() { e2e.OptArgs(installationDir(ts), "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=false"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.OverwriteDefaultSystemPathEnvVarName, dir)), + e2e.OptMaybeRunInsideShell(), ) cp.Expect("successfully installed") cp.ExpectInput() diff --git a/test/integration/activate_int_test.go b/test/integration/activate_int_test.go index 9f34c6daa3..e5a9843e04 100644 --- a/test/integration/activate_int_test.go +++ b/test/integration/activate_int_test.go @@ -485,7 +485,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_FromCache() { cp.ExpectExitCode(0) // next activation is cached - cp = ts.Spawn("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) + cp = ts.SpawnShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) cp.ExpectInput(e2e.RuntimeSourcingTimeoutOpt) cp.SendLine("exit") From 0f6e7d55ea3eb3c5a00bd9d698560cfdaea970f2 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 29 Aug 2024 16:27:31 -0700 Subject: [PATCH 15/38] Revert install scripts test change --- test/integration/install_scripts_int_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/install_scripts_int_test.go b/test/integration/install_scripts_int_test.go index 4f0b866d9b..8846c44353 100644 --- a/test/integration/install_scripts_int_test.go +++ b/test/integration/install_scripts_int_test.go @@ -104,7 +104,7 @@ func (suite *InstallScriptsIntegrationTestSuite) TestInstall() { } if runtime.GOOS == "windows" { cmd = "powershell.exe" - opts = append(opts, e2e.OptAppendEnv("SHELL="), e2e.OptRunInsideShell(true)) + opts = append(opts, e2e.OptAppendEnv("SHELL=")) } cp := ts.SpawnCmdWithOpts(cmd, opts...) cp.Expect("Preparing Installer for State Tool Package Manager") From 408e7ddae3247a425082b7c627d07572266c9db7 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Fri, 30 Aug 2024 09:46:37 -0700 Subject: [PATCH 16/38] Change powershell expect --- internal/testhelpers/e2e/spawn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testhelpers/e2e/spawn.go b/internal/testhelpers/e2e/spawn.go index bcc466ea9c..d36e97f369 100644 --- a/internal/testhelpers/e2e/spawn.go +++ b/internal/testhelpers/e2e/spawn.go @@ -75,7 +75,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 \"`\"" + send = "echo `" expect = `` } else { send = `echo ^` From 73fec747e75e77c117022bb4841e49d3ebc08f79 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Fri, 30 Aug 2024 10:46:07 -0700 Subject: [PATCH 17/38] Try another expect string --- internal/testhelpers/e2e/spawn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testhelpers/e2e/spawn.go b/internal/testhelpers/e2e/spawn.go index d36e97f369..1a04ae696b 100644 --- a/internal/testhelpers/e2e/spawn.go +++ b/internal/testhelpers/e2e/spawn.go @@ -75,7 +75,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 `" + send = "echo \"\"" expect = `` } else { send = `echo ^` From 471f5097ac2988eccab36b90b95e14e711ebf768 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Sep 2024 09:57:05 -0700 Subject: [PATCH 18/38] Remove maybe option --- .../test/integration/installer_int_test.go | 9 ++--- internal/testhelpers/e2e/session.go | 14 -------- internal/testhelpers/e2e/session_unix.go | 18 ++++++++++ internal/testhelpers/e2e/session_windows.go | 36 +++++++++++++++++++ internal/testhelpers/e2e/spawn.go | 11 ------ test/integration/activate_int_test.go | 25 ++++++------- test/integration/shell_int_test.go | 2 +- 7 files changed, 69 insertions(+), 46 deletions(-) diff --git a/cmd/state-installer/test/integration/installer_int_test.go b/cmd/state-installer/test/integration/installer_int_test.go index b3231d0165..82c9fb3a9a 100644 --- a/cmd/state-installer/test/integration/installer_int_test.go +++ b/cmd/state-installer/test/integration/installer_int_test.go @@ -39,12 +39,11 @@ func (suite *InstallerIntegrationTestSuite) TestInstallFromLocalSource() { suite.NoError(err) // Run installer with source-path flag (ie. install from this local path) - cp := ts.SpawnCmdWithOpts( + cp := ts.SpawnCmdInsideShellWithOpts( suite.installerExe, e2e.OptArgs(installationDir(ts), "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=false"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.OverwriteDefaultSystemPathEnvVarName, dir)), - e2e.OptMaybeRunInsideShell(), ) // Assert output @@ -57,12 +56,11 @@ func (suite *InstallerIntegrationTestSuite) TestInstallFromLocalSource() { cp.ExpectExitCode(0) // Ensure installing overtop doesn't result in errors - cp = ts.SpawnCmdWithOpts( + cp = ts.SpawnCmdInsideShellWithOpts( suite.installerExe, e2e.OptArgs(installationDir(ts), "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=false"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.OverwriteDefaultSystemPathEnvVarName, dir)), - e2e.OptMaybeRunInsideShell(), ) cp.Expect("successfully installed") cp.ExpectInput() @@ -173,12 +171,11 @@ func (suite *InstallerIntegrationTestSuite) TestInstallErrorTips() { dir, err := os.MkdirTemp("", "system*") suite.NoError(err) - cp := ts.SpawnCmdWithOpts( + cp := ts.SpawnCmdInsideShellWithOpts( suite.installerExe, e2e.OptArgs(installationDir(ts), "--activate", "ActiveState-CLI/Python3", "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=true"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.OverwriteDefaultSystemPathEnvVarName, dir)), - e2e.OptMaybeRunInsideShell(), ) cp.ExpectInput(e2e.RuntimeSourcingTimeoutOpt) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index 7db033dec0..f84b71007f 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -234,20 +234,6 @@ func (s *Session) SpawnShellWithOpts(shell Shell, opts ...SpawnOptSetter) *Spawn return s.SpawnCmdWithOpts(string(shell), opts...) } -// SpawnShell spawns the state tool executable to be tested with arguments. -// This function differs from Spawn in that it runs the command in a shell on Windows CI. -// Our Windows integration tests are run on bash. Due to the way the PTY library runs a new -// command we need to run the command inside a shell in order to setup the correct process -// tree. Without this integration tests run in bash will incorrectly identify the partent shell -// as bash, rather than the actual shell that is running the command -func (s *Session) SpawnShell(args ...string) *SpawnedCmd { - opts := []SpawnOptSetter{OptArgs(args...)} - if runtime.GOOS == "windows" && condition.OnCI() { - opts = append(opts, OptRunInsideShell(true)) - } - return s.SpawnCmdWithOpts(s.Exe, opts...) -} - // SpawnCmdWithOpts executes an executable in a pseudo-terminal for integration tests // Arguments and other parameters can be specified by specifying SpawnOptSetter func (s *Session) SpawnCmdWithOpts(exe string, optSetters ...SpawnOptSetter) *SpawnedCmd { diff --git a/internal/testhelpers/e2e/session_unix.go b/internal/testhelpers/e2e/session_unix.go index 970a8fbec0..4e5493e68c 100644 --- a/internal/testhelpers/e2e/session_unix.go +++ b/internal/testhelpers/e2e/session_unix.go @@ -13,3 +13,21 @@ var ( RuntimeSourcingTimeoutOpt = termtest.OptExpectTimeout(3 * time.Minute) RuntimeBuildSourcingTimeoutOpt = termtest.OptExpectTimeout(6 * time.Minute) ) + +// SpawnInsideShell spawns the state tool executable to be tested with arguments. +// On Unix systems, this function is equivalent to Spawn. +func (s *Session) SpawnInsideShell(args ...string) *SpawnedCmd { + return s.SpawnCmd(s.Exe, args...) +} + +// SpawnCmdInsideShellWithOpts spawns the state tool executable to be tested with arguments and options. +// On Unix systems, this function is equivalent to SpawnCmdWithOpts. +func (s *Session) SpawnCmdInsideShellWithOpts(exe string, opts ...SpawnOptSetter) *SpawnedCmd { + return s.SpawnCmdWithOpts(exe, opts...) +} + +// SpawnInsideShell spawns the state tool executable to be tested with arguments. +// On Unix systems, this function is equivalent to SpawnWithOpts. +func (s *Session) SpawnInsideShellWithOpts(opts ...SpawnOptSetter) *SpawnedCmd { + return s.SpawnCmdWithOpts(s.Exe, opts...) +} diff --git a/internal/testhelpers/e2e/session_windows.go b/internal/testhelpers/e2e/session_windows.go index 6c1830eadd..dd76f1e5d7 100644 --- a/internal/testhelpers/e2e/session_windows.go +++ b/internal/testhelpers/e2e/session_windows.go @@ -4,8 +4,10 @@ package e2e import ( + "runtime" "time" + "github.com/ActiveState/cli/internal/condition" "github.com/ActiveState/termtest" ) @@ -13,3 +15,37 @@ var ( RuntimeSourcingTimeoutOpt = termtest.OptExpectTimeout(3 * time.Minute) RuntimeBuildSourcingTimeoutOpt = termtest.OptExpectTimeout(6 * time.Minute) ) + +// SpawnInsideShell spawns the state tool executable to be tested with arguments. +// This function differs from Spawn in that it runs the command in a shell on Windows CI. +// Our Windows integration tests are run on bash. Due to the way the PTY library runs a new +// command we need to run the command inside a shell in order to setup the correct process +// tree. Without this integration tests run in bash will incorrectly identify the partent shell +// as bash, rather than the actual shell that is running the command +func (s *Session) SpawnInsideShell(args ...string) *SpawnedCmd { + opts := []SpawnOptSetter{OptArgs(args...)} + if runtime.GOOS == "windows" && condition.OnCI() { + opts = append(opts, OptRunInsideShell(true)) + } + return s.SpawnCmdWithOpts(s.Exe, opts...) +} + +// SpawnCmdInsideShellWithOpts spawns the executable to be tested with arguments and options. +// This function differs from SpawnCmdWithOpts in that it runs the command in a shell on Windows CI. +// See SpawnInsideShell for more information. +func (s *Session) SpawnCmdInsideShellWithOpts(exe string, opts ...SpawnOptSetter) *SpawnedCmd { + if runtime.GOOS == "windows" && condition.OnCI() { + opts = append(opts, OptRunInsideShell(true)) + } + return s.SpawnCmdWithOpts(exe, opts...) +} + +// SpawnInsideShell spawns the state tool executable to be tested with arguments. +// This function differs from SpawnWithOpts in that it runs the command in a shell on Windows CI. +// See SpawnInsideShell for more information. +func (s *Session) SpawnInsideShellWithOpts(opts ...SpawnOptSetter) *SpawnedCmd { + if runtime.GOOS == "windows" && condition.OnCI() { + opts = append(opts, OptRunInsideShell(true)) + } + return s.SpawnCmdWithOpts(s.Exe, opts...) +} diff --git a/internal/testhelpers/e2e/spawn.go b/internal/testhelpers/e2e/spawn.go index 1a04ae696b..fff1cd25b8 100644 --- a/internal/testhelpers/e2e/spawn.go +++ b/internal/testhelpers/e2e/spawn.go @@ -10,7 +10,6 @@ import ( "github.com/ActiveState/termtest" - "github.com/ActiveState/cli/internal/condition" "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/osutils" ) @@ -181,13 +180,3 @@ func OptRunInsideShell(v bool) SpawnOptSetter { opts.RunInsideShell = v } } - -func OptMaybeRunInsideShell() SpawnOptSetter { - if runtime.GOOS == "windows" && condition.OnCI() { - return func(opts *SpawnOpts) { - opts.RunInsideShell = true - } - } - - return func(opts *SpawnOpts) {} -} diff --git a/test/integration/activate_int_test.go b/test/integration/activate_int_test.go index e5a9843e04..24057ad6b0 100644 --- a/test/integration/activate_int_test.go +++ b/test/integration/activate_int_test.go @@ -54,7 +54,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateWithoutRuntime() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnShell("activate", "ActiveState-CLI/Empty") + cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/Empty") cp.Expect("Activated") cp.ExpectInput() @@ -134,7 +134,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateUsingCommitID() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnShell("activate", "ActiveState-CLI/Empty#6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8", "--path", ts.Dirs.Work) + cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/Empty#6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8", "--path", ts.Dirs.Work) cp.Expect("Activated") cp.ExpectInput() @@ -149,7 +149,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateNotOnPath() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnShell("activate", "activestate-cli/empty", "--path", ts.Dirs.Work) + cp := ts.SpawnInsideShell("activate", "activestate-cli/empty", "--path", ts.Dirs.Work) cp.Expect("Activated") cp.ExpectInput() @@ -177,7 +177,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivatePythonByHostOnly() { defer close() projectName := "Python-LinuxWorks" - cp := ts.SpawnShell("activate", "cli-integration-tests/"+projectName, "--path="+ts.Dirs.Work) + cp := ts.SpawnInsideShell("activate", "cli-integration-tests/"+projectName, "--path="+ts.Dirs.Work) if runtime.GOOS == "linux" { cp.Expect("Creating a Virtual Environment") @@ -218,10 +218,9 @@ func (suite *ActivateIntegrationTestSuite) activatePython(version string, extraE namespace := "ActiveState-CLI/Python" + version - cp := ts.SpawnWithOpts( + cp := ts.SpawnInsideShellWithOpts( e2e.OptArgs("activate", namespace), e2e.OptAppendEnv(extraEnv...), - e2e.OptMaybeRunInsideShell(), ) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) @@ -295,7 +294,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_PythonPath() { namespace := "ActiveState-CLI/Python3" - cp := ts.SpawnShell("activate", namespace) + cp := ts.SpawnInsideShell("activate", namespace) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) // ensure that shell is functional @@ -335,10 +334,9 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_SpaceInCacheDir() { err := fileutils.MkdirUnlessExists(cacheDir) suite.Require().NoError(err) - cp := ts.SpawnWithOpts( + cp := ts.SpawnInsideShellWithOpts( e2e.OptArgs("activate", "ActiveState-CLI/Python3"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.CacheEnvVarName, cacheDir)), - e2e.OptMaybeRunInsideShell(), ) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) @@ -360,7 +358,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivatePerlCamel() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnShell("activate", "ActiveState-CLI/Perl") + cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/Perl") cp.Expect("Downloading", termtest.OptExpectTimeout(40*time.Second)) cp.Expect("Installing", termtest.OptExpectTimeout(140*time.Second)) @@ -401,10 +399,9 @@ version: %s ts.PrepareCommitIdFile("6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8") // Activate in the subdirectory - c2 := ts.SpawnWithOpts( + c2 := ts.SpawnInsideShellWithOpts( e2e.OptArgs("activate"), e2e.OptWD(filepath.Join(ts.Dirs.Work, "foo", "bar", "baz")), - e2e.OptMaybeRunInsideShell(), ) c2.Expect("Activated") @@ -477,7 +474,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_FromCache() { // Note: cannot use Empty project since we need artifacts to download and install. // Pick the langless project, which just has some small, non-language artifacts. - cp := ts.SpawnShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) + cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) suite.assertCompletedStatusBarReport(cp.Output()) @@ -485,7 +482,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_FromCache() { cp.ExpectExitCode(0) // next activation is cached - cp = ts.SpawnShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) + cp = ts.SpawnInsideShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) cp.ExpectInput(e2e.RuntimeSourcingTimeoutOpt) cp.SendLine("exit") diff --git a/test/integration/shell_int_test.go b/test/integration/shell_int_test.go index 8faf38257f..9fbe4365ea 100644 --- a/test/integration/shell_int_test.go +++ b/test/integration/shell_int_test.go @@ -466,7 +466,7 @@ events:`, lang, splat), 1) suite.Require().NoError(fileutils.WriteFile(asyFilename, []byte(contents))) // Verify that running a script as a command with an argument containing special characters works. - cp = ts.SpawnShell("shell") + cp = ts.SpawnInsideShell("shell") cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) cp.ExpectInput() cp.SendLine(`args "<3"`) From 8d8234f919c18a4a5de98808f5f6ad574916dce1 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Sep 2024 11:52:03 -0700 Subject: [PATCH 19/38] Use env var --- .../test/integration/installer_int_test.go | 6 ++-- internal/constants/constants.go | 3 ++ internal/subshell/subshell.go | 11 ++++-- internal/testhelpers/e2e/env_windows.go | 8 ++++- internal/testhelpers/e2e/session_unix.go | 18 ---------- internal/testhelpers/e2e/session_windows.go | 36 ------------------- test/integration/activate_int_test.go | 22 ++++++------ test/integration/shell_int_test.go | 2 +- 8 files changed, 34 insertions(+), 72 deletions(-) diff --git a/cmd/state-installer/test/integration/installer_int_test.go b/cmd/state-installer/test/integration/installer_int_test.go index 82c9fb3a9a..1f83eaa242 100644 --- a/cmd/state-installer/test/integration/installer_int_test.go +++ b/cmd/state-installer/test/integration/installer_int_test.go @@ -39,7 +39,7 @@ func (suite *InstallerIntegrationTestSuite) TestInstallFromLocalSource() { suite.NoError(err) // Run installer with source-path flag (ie. install from this local path) - cp := ts.SpawnCmdInsideShellWithOpts( + cp := ts.SpawnCmdWithOpts( suite.installerExe, e2e.OptArgs(installationDir(ts), "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=false"), @@ -56,7 +56,7 @@ func (suite *InstallerIntegrationTestSuite) TestInstallFromLocalSource() { cp.ExpectExitCode(0) // Ensure installing overtop doesn't result in errors - cp = ts.SpawnCmdInsideShellWithOpts( + cp = ts.SpawnCmdWithOpts( suite.installerExe, e2e.OptArgs(installationDir(ts), "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=false"), @@ -171,7 +171,7 @@ func (suite *InstallerIntegrationTestSuite) TestInstallErrorTips() { dir, err := os.MkdirTemp("", "system*") suite.NoError(err) - cp := ts.SpawnCmdInsideShellWithOpts( + cp := ts.SpawnCmdWithOpts( suite.installerExe, e2e.OptArgs(installationDir(ts), "--activate", "ActiveState-CLI/Python3", "-n"), e2e.OptAppendEnv(constants.DisableUpdates+"=true"), diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 2433f9c5af..34718abd9c 100644 --- a/internal/constants/constants.go +++ b/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" diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index b95e8713e8..67fb4d9111 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -8,6 +8,7 @@ import ( "runtime" "strings" + "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/shirou/gopsutil/v3/process" @@ -182,8 +183,14 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } }() - binary = detectShellParent() - logging.Debug("Configured shell: %s", binary) + if os.Getenv(constants.OverrideShellEnvVarName) != "" { + binary = os.Getenv(constants.OverrideShellEnvVarName) + } + + if binary == "" { + binary = detectShellParent() + } + if binary == "" { binary = configured } diff --git a/internal/testhelpers/e2e/env_windows.go b/internal/testhelpers/e2e/env_windows.go index 0c7b72aeec..c91d1efe39 100644 --- a/internal/testhelpers/e2e/env_windows.go +++ b/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 { diff --git a/internal/testhelpers/e2e/session_unix.go b/internal/testhelpers/e2e/session_unix.go index 4e5493e68c..970a8fbec0 100644 --- a/internal/testhelpers/e2e/session_unix.go +++ b/internal/testhelpers/e2e/session_unix.go @@ -13,21 +13,3 @@ var ( RuntimeSourcingTimeoutOpt = termtest.OptExpectTimeout(3 * time.Minute) RuntimeBuildSourcingTimeoutOpt = termtest.OptExpectTimeout(6 * time.Minute) ) - -// SpawnInsideShell spawns the state tool executable to be tested with arguments. -// On Unix systems, this function is equivalent to Spawn. -func (s *Session) SpawnInsideShell(args ...string) *SpawnedCmd { - return s.SpawnCmd(s.Exe, args...) -} - -// SpawnCmdInsideShellWithOpts spawns the state tool executable to be tested with arguments and options. -// On Unix systems, this function is equivalent to SpawnCmdWithOpts. -func (s *Session) SpawnCmdInsideShellWithOpts(exe string, opts ...SpawnOptSetter) *SpawnedCmd { - return s.SpawnCmdWithOpts(exe, opts...) -} - -// SpawnInsideShell spawns the state tool executable to be tested with arguments. -// On Unix systems, this function is equivalent to SpawnWithOpts. -func (s *Session) SpawnInsideShellWithOpts(opts ...SpawnOptSetter) *SpawnedCmd { - return s.SpawnCmdWithOpts(s.Exe, opts...) -} diff --git a/internal/testhelpers/e2e/session_windows.go b/internal/testhelpers/e2e/session_windows.go index dd76f1e5d7..6c1830eadd 100644 --- a/internal/testhelpers/e2e/session_windows.go +++ b/internal/testhelpers/e2e/session_windows.go @@ -4,10 +4,8 @@ package e2e import ( - "runtime" "time" - "github.com/ActiveState/cli/internal/condition" "github.com/ActiveState/termtest" ) @@ -15,37 +13,3 @@ var ( RuntimeSourcingTimeoutOpt = termtest.OptExpectTimeout(3 * time.Minute) RuntimeBuildSourcingTimeoutOpt = termtest.OptExpectTimeout(6 * time.Minute) ) - -// SpawnInsideShell spawns the state tool executable to be tested with arguments. -// This function differs from Spawn in that it runs the command in a shell on Windows CI. -// Our Windows integration tests are run on bash. Due to the way the PTY library runs a new -// command we need to run the command inside a shell in order to setup the correct process -// tree. Without this integration tests run in bash will incorrectly identify the partent shell -// as bash, rather than the actual shell that is running the command -func (s *Session) SpawnInsideShell(args ...string) *SpawnedCmd { - opts := []SpawnOptSetter{OptArgs(args...)} - if runtime.GOOS == "windows" && condition.OnCI() { - opts = append(opts, OptRunInsideShell(true)) - } - return s.SpawnCmdWithOpts(s.Exe, opts...) -} - -// SpawnCmdInsideShellWithOpts spawns the executable to be tested with arguments and options. -// This function differs from SpawnCmdWithOpts in that it runs the command in a shell on Windows CI. -// See SpawnInsideShell for more information. -func (s *Session) SpawnCmdInsideShellWithOpts(exe string, opts ...SpawnOptSetter) *SpawnedCmd { - if runtime.GOOS == "windows" && condition.OnCI() { - opts = append(opts, OptRunInsideShell(true)) - } - return s.SpawnCmdWithOpts(exe, opts...) -} - -// SpawnInsideShell spawns the state tool executable to be tested with arguments. -// This function differs from SpawnWithOpts in that it runs the command in a shell on Windows CI. -// See SpawnInsideShell for more information. -func (s *Session) SpawnInsideShellWithOpts(opts ...SpawnOptSetter) *SpawnedCmd { - if runtime.GOOS == "windows" && condition.OnCI() { - opts = append(opts, OptRunInsideShell(true)) - } - return s.SpawnCmdWithOpts(s.Exe, opts...) -} diff --git a/test/integration/activate_int_test.go b/test/integration/activate_int_test.go index 24057ad6b0..d91ed46918 100644 --- a/test/integration/activate_int_test.go +++ b/test/integration/activate_int_test.go @@ -54,7 +54,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateWithoutRuntime() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/Empty") + cp := ts.Spawn("activate", "ActiveState-CLI/Empty") cp.Expect("Activated") cp.ExpectInput() @@ -134,7 +134,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateUsingCommitID() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/Empty#6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8", "--path", ts.Dirs.Work) + cp := ts.Spawn("activate", "ActiveState-CLI/Empty#6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8", "--path", ts.Dirs.Work) cp.Expect("Activated") cp.ExpectInput() @@ -149,7 +149,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivateNotOnPath() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnInsideShell("activate", "activestate-cli/empty", "--path", ts.Dirs.Work) + cp := ts.Spawn("activate", "activestate-cli/empty", "--path", ts.Dirs.Work) cp.Expect("Activated") cp.ExpectInput() @@ -177,7 +177,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivatePythonByHostOnly() { defer close() projectName := "Python-LinuxWorks" - cp := ts.SpawnInsideShell("activate", "cli-integration-tests/"+projectName, "--path="+ts.Dirs.Work) + cp := ts.Spawn("activate", "cli-integration-tests/"+projectName, "--path="+ts.Dirs.Work) if runtime.GOOS == "linux" { cp.Expect("Creating a Virtual Environment") @@ -218,7 +218,7 @@ func (suite *ActivateIntegrationTestSuite) activatePython(version string, extraE namespace := "ActiveState-CLI/Python" + version - cp := ts.SpawnInsideShellWithOpts( + cp := ts.SpawnWithOpts( e2e.OptArgs("activate", namespace), e2e.OptAppendEnv(extraEnv...), ) @@ -294,7 +294,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_PythonPath() { namespace := "ActiveState-CLI/Python3" - cp := ts.SpawnInsideShell("activate", namespace) + cp := ts.Spawn("activate", namespace) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) // ensure that shell is functional @@ -334,7 +334,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_SpaceInCacheDir() { err := fileutils.MkdirUnlessExists(cacheDir) suite.Require().NoError(err) - cp := ts.SpawnInsideShellWithOpts( + cp := ts.SpawnWithOpts( e2e.OptArgs("activate", "ActiveState-CLI/Python3"), e2e.OptAppendEnv(fmt.Sprintf("%s=%s", constants.CacheEnvVarName, cacheDir)), ) @@ -358,7 +358,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivatePerlCamel() { close := suite.addForegroundSvc(ts) defer close() - cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/Perl") + cp := ts.Spawn("activate", "ActiveState-CLI/Perl") cp.Expect("Downloading", termtest.OptExpectTimeout(40*time.Second)) cp.Expect("Installing", termtest.OptExpectTimeout(140*time.Second)) @@ -399,7 +399,7 @@ version: %s ts.PrepareCommitIdFile("6d79f2ae-f8b5-46bd-917a-d4b2558ec7b8") // Activate in the subdirectory - c2 := ts.SpawnInsideShellWithOpts( + c2 := ts.SpawnWithOpts( e2e.OptArgs("activate"), e2e.OptWD(filepath.Join(ts.Dirs.Work, "foo", "bar", "baz")), ) @@ -474,7 +474,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_FromCache() { // Note: cannot use Empty project since we need artifacts to download and install. // Pick the langless project, which just has some small, non-language artifacts. - cp := ts.SpawnInsideShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) + cp := ts.Spawn("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) suite.assertCompletedStatusBarReport(cp.Output()) @@ -482,7 +482,7 @@ func (suite *ActivateIntegrationTestSuite) TestActivate_FromCache() { cp.ExpectExitCode(0) // next activation is cached - cp = ts.SpawnInsideShell("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) + cp = ts.Spawn("activate", "ActiveState-CLI/langless", "--path", ts.Dirs.Work) cp.ExpectInput(e2e.RuntimeSourcingTimeoutOpt) cp.SendLine("exit") diff --git a/test/integration/shell_int_test.go b/test/integration/shell_int_test.go index 9fbe4365ea..77183146e1 100644 --- a/test/integration/shell_int_test.go +++ b/test/integration/shell_int_test.go @@ -466,7 +466,7 @@ events:`, lang, splat), 1) suite.Require().NoError(fileutils.WriteFile(asyFilename, []byte(contents))) // Verify that running a script as a command with an argument containing special characters works. - cp = ts.SpawnInsideShell("shell") + cp = ts.Spawn("shell") cp.Expect("Activated", e2e.RuntimeSourcingTimeoutOpt) cp.ExpectInput() cp.SendLine(`args "<3"`) From 77c970fc6f8400e35125f88ad0e9d3225af0960f Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Tue, 3 Sep 2024 13:44:00 -0700 Subject: [PATCH 20/38] Fix shells tests --- test/integration/shell_int_test.go | 6 +++++- test/integration/shells_int_test.go | 11 +++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/test/integration/shell_int_test.go b/test/integration/shell_int_test.go index 77183146e1..43fe711297 100644 --- a/test/integration/shell_int_test.go +++ b/test/integration/shell_int_test.go @@ -489,7 +489,11 @@ 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 diff --git a/test/integration/shells_int_test.go b/test/integration/shells_int_test.go index f75211b204..82a7ec5c9f 100644 --- a/test/integration/shells_int_test.go +++ b/test/integration/shells_int_test.go @@ -6,6 +6,7 @@ import ( "runtime" "testing" + "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/testhelpers/suite" "github.com/ActiveState/cli/internal/fileutils" @@ -48,7 +49,10 @@ func (suite *ShellsIntegrationTestSuite) TestShells() { } // 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 +62,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") From b0886eaf76831d2e11f758f80fc1937733f87f66 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Tue, 3 Sep 2024 14:10:29 -0700 Subject: [PATCH 21/38] Shells test fix --- test/integration/shell_int_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/integration/shell_int_test.go b/test/integration/shell_int_test.go index 43fe711297..a82050f157 100644 --- a/test/integration/shell_int_test.go +++ b/test/integration/shell_int_test.go @@ -500,7 +500,11 @@ func (suite *ShellIntegrationTestSuite) TestWindowsShells() { cp.SendLine("exit") cp.ExpectExitCode(0) - cp = ts.SpawnCmd("powershell", "-Command", "state", "shell") + 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 From 8af3168b4cf584066dfdf726e83ee26164a28f31 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Sep 2024 15:06:44 -0700 Subject: [PATCH 22/38] Remove opt --- test/integration/deploy_int_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/deploy_int_test.go b/test/integration/deploy_int_test.go index 28b2621daf..f51e686baa 100644 --- a/test/integration/deploy_int_test.go +++ b/test/integration/deploy_int_test.go @@ -168,7 +168,6 @@ func (suite *DeployIntegrationTestSuite) TestDeployPython() { "cmd.exe", e2e.OptArgs("/k", filepath.Join(targetPath, "bin", "shell.bat")), e2e.OptAppendEnv("PATHEXT=.COM;.EXE;.BAT;.LNK", "SHELL="), - e2e.OptRunInsideShell(true), ) } else { cp = ts.SpawnCmdWithOpts( From e2df1a77ad085e98a0ba6746d253f7f62a62750d Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Sep 2024 15:07:14 -0700 Subject: [PATCH 23/38] Remove logging calls --- internal/subshell/subshell.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 67fb4d9111..6f5e8d21c6 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -245,7 +245,6 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } func detectShellParent() string { - logging.Debug("Detecting shell from parent process") p, err := process.NewProcess(int32(os.Getppid())) if err != nil && !errors.As(err, ptr.To(&os.PathError{})) { logging.Error("Failed to get parent process: %v", err) @@ -254,7 +253,6 @@ func detectShellParent() string { for p != nil && p.Pid != 0 { name, err := p.Name() if err == nil { - logging.Debug("Searching for supported shell in parent process: %s", name) if supportedShellName(name) { return name } From e6eeb7bc2c48fa050a511dd77f4ff360608011ea Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 14:59:42 -0700 Subject: [PATCH 24/38] Fix env var name --- internal/subshell/subshell.go | 2 +- internal/subshell/subshell_lin_mac.go | 2 +- internal/subshell/subshell_win.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 6f5e8d21c6..48bb2f3bb3 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -200,7 +200,7 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } if binary == "" { - binary = OS_DEFULAT + binary = OS_DEFAULT } path := resolveBinaryPath(binary) diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index fe66d31c41..4a1dc705c3 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/internal/subshell/subshell_lin_mac.go @@ -21,7 +21,7 @@ var supportedShells = []SubShell{ const ( SHELL_ENV_VAR = "SHELL" - OS_DEFULAT = "bash" + OS_DEFAULT = "bash" ) func supportedShellName(name string) bool { diff --git a/internal/subshell/subshell_win.go b/internal/subshell/subshell_win.go index 6ad787901f..7775acbee5 100644 --- a/internal/subshell/subshell_win.go +++ b/internal/subshell/subshell_win.go @@ -19,7 +19,7 @@ var supportedShells = []SubShell{ const ( SHELL_ENV_VAR = "COMSPEC" - OS_DEFULAT = "cmd.exe" + OS_DEFAULT = "cmd.exe" ) func supportedShellName(name string) bool { From b6ddb92d259d9b6b8a1019037f52d520bfbab967 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 15:00:41 -0700 Subject: [PATCH 25/38] Make shell name comparison case-insensitive --- internal/subshell/subshell_lin_mac.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index 4a1dc705c3..2bb9f62520 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/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" @@ -26,7 +28,7 @@ const ( func supportedShellName(name string) bool { for _, subshell := range supportedShells { - if name == subshell.Shell() { + if strings.EqualFold(name, subshell.Shell()) { return true } } From 7907c120c3b6d3b0ed5f35768aaaf6a2d7d13514 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 15:04:52 -0700 Subject: [PATCH 26/38] Handle filepaths --- internal/subshell/subshell.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 48bb2f3bb3..361293e4b8 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -4,6 +4,7 @@ import ( "errors" "os" "os/exec" + "path" "path/filepath" "runtime" "strings" @@ -253,6 +254,9 @@ func detectShellParent() string { for p != nil && p.Pid != 0 { name, err := p.Name() if err == nil { + if strings.Contains(name, string(filepath.Separator)) { + name = path.Base(name) + } if supportedShellName(name) { return name } From 723b52091b423dc5aa3631342d1fdc20a171ea86 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 15:06:25 -0700 Subject: [PATCH 27/38] Update argument name Handle case insensitivitiy --- internal/subshell/subshell_lin_mac.go | 4 ++-- internal/subshell/subshell_win.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/subshell/subshell_lin_mac.go b/internal/subshell/subshell_lin_mac.go index 2bb9f62520..9a666e16cb 100644 --- a/internal/subshell/subshell_lin_mac.go +++ b/internal/subshell/subshell_lin_mac.go @@ -26,9 +26,9 @@ const ( OS_DEFAULT = "bash" ) -func supportedShellName(name string) bool { +func supportedShellName(filename string) bool { for _, subshell := range supportedShells { - if strings.EqualFold(name, subshell.Shell()) { + if strings.EqualFold(filename, subshell.Shell()) { return true } } diff --git a/internal/subshell/subshell_win.go b/internal/subshell/subshell_win.go index 7775acbee5..b0332a8008 100644 --- a/internal/subshell/subshell_win.go +++ b/internal/subshell/subshell_win.go @@ -5,6 +5,7 @@ package subshell import ( "fmt" + "strings" "github.com/ActiveState/cli/internal/subshell/bash" "github.com/ActiveState/cli/internal/subshell/cmd" @@ -22,9 +23,9 @@ const ( OS_DEFAULT = "cmd.exe" ) -func supportedShellName(name string) bool { +func supportedShellName(filename string) bool { for _, subshell := range supportedShells { - if name == fmt.Sprintf("%s.exe", subshell.Shell()) { + if strings.EqualFold(filename, fmt.Sprintf("%s.exe", subshell.Shell())) { return true } } From c97aadb210d9a2ac73ddb52eabd491d693d87a3d Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 15:09:36 -0700 Subject: [PATCH 28/38] Use configured first --- internal/subshell/subshell.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 361293e4b8..e167f87124 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -188,14 +188,11 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { binary = os.Getenv(constants.OverrideShellEnvVarName) } + binary = configured if binary == "" { binary = detectShellParent() } - if binary == "" { - binary = configured - } - if binary == "" { binary = os.Getenv(SHELL_ENV_VAR) } From 4cf5cdabf1ce0e89987bb1865a5471b8c7ee2931 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 15:25:15 -0700 Subject: [PATCH 29/38] Clear configured shell in test --- test/integration/shells_int_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/integration/shells_int_test.go b/test/integration/shells_int_test.go index 82a7ec5c9f..6e342afd65 100644 --- a/test/integration/shells_int_test.go +++ b/test/integration/shells_int_test.go @@ -6,8 +6,12 @@ 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/stretchr/testify/require" "github.com/ActiveState/cli/internal/fileutils" "github.com/ActiveState/cli/internal/testhelpers/e2e" @@ -48,6 +52,12 @@ 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, "") + require.NoError(t, err) + // Run the checkout in a particular shell. cp = ts.SpawnShellWithOpts( shell, From f00d77c99d96270aedc494b9c04c94983d033fb0 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 15:58:20 -0700 Subject: [PATCH 30/38] Clear configured shell on Windows test --- test/integration/shell_int_test.go | 7 +++++++ test/integration/shells_int_test.go | 3 +-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/test/integration/shell_int_test.go b/test/integration/shell_int_test.go index a82050f157..b5d0114856 100644 --- a/test/integration/shell_int_test.go +++ b/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" @@ -500,6 +501,12 @@ func (suite *ShellIntegrationTestSuite) TestWindowsShells() { cp.SendLine("exit") cp.ExpectExitCode(0) + // 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"), diff --git a/test/integration/shells_int_test.go b/test/integration/shells_int_test.go index 6e342afd65..b2a2b3b045 100644 --- a/test/integration/shells_int_test.go +++ b/test/integration/shells_int_test.go @@ -11,7 +11,6 @@ import ( "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/ActiveState/cli/internal/fileutils" "github.com/ActiveState/cli/internal/testhelpers/e2e" @@ -56,7 +55,7 @@ func (suite *ShellsIntegrationTestSuite) TestShells() { cfg, err := config.NewCustom(ts.Dirs.Config, singlethread.New(), true) suite.Require().NoError(err) err = cfg.Set(subshell.ConfigKeyShell, "") - require.NoError(t, err) + suite.Require().NoError(err) // Run the checkout in a particular shell. cp = ts.SpawnShellWithOpts( From e532c53b4a891283737aa696719ef9e3fe4faa89 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 4 Sep 2024 16:22:23 -0700 Subject: [PATCH 31/38] Revert config setting on Windows --- internal/testhelpers/e2e/session.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index f84b71007f..030e279c23 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -189,12 +189,6 @@ func new(t *testing.T, retainDirs, updatePath bool, extraEnv ...string) *Session require.NoError(session.T, err) } - if runtime.GOOS == "windows" { - if err := cfg.Set(subshell.ConfigKeyShell, "cmd.exe"); err != nil { - require.NoError(session.T, err) - } - } - return session } From df47edd559c010067cd36b44fe4cdb10d337b075 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 5 Sep 2024 09:48:13 -0700 Subject: [PATCH 32/38] Respect override --- internal/subshell/subshell.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index e167f87124..19dc6c4933 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -188,7 +188,10 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { binary = os.Getenv(constants.OverrideShellEnvVarName) } - binary = configured + if binary == "" { + binary = configured + } + if binary == "" { binary = detectShellParent() } From 4f23d47b80d56785e9bf7e3a88f90a81f5a317e1 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 5 Sep 2024 10:20:37 -0700 Subject: [PATCH 33/38] Test revert backtick change --- internal/testhelpers/e2e/spawn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testhelpers/e2e/spawn.go b/internal/testhelpers/e2e/spawn.go index fff1cd25b8..bf73ffcd22 100644 --- a/internal/testhelpers/e2e/spawn.go +++ b/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 \"\"" + send = "echo \"`\"" expect = `` } else { send = `echo ^` From 7cb0820e3e59793bfc68297dfe2c2dcd40ea3a87 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 5 Sep 2024 11:02:22 -0700 Subject: [PATCH 34/38] Disable override for install scripts tests and clear config --- test/integration/install_scripts_int_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/integration/install_scripts_int_test.go b/test/integration/install_scripts_int_test.go index 8846c44353..87e8d5e424 100644 --- a/test/integration/install_scripts_int_test.go +++ b/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") From 396fe68200822569023cf29892072251b8090413 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 5 Sep 2024 16:07:45 -0700 Subject: [PATCH 35/38] Update internal/subshell/subshell.go Co-authored-by: Nathan Rijksen --- internal/subshell/subshell.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 19dc6c4933..4769ccda5d 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -248,7 +248,7 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { func detectShellParent() string { p, err := process.NewProcess(int32(os.Getppid())) if err != nil && !errors.As(err, ptr.To(&os.PathError{})) { - logging.Error("Failed to get parent process: %v", err) + logging.Error("Failed to get parent process: %s", errs.JoinMessage(err)) } for p != nil && p.Pid != 0 { From f57b755de82163ab1b387fe557a436d3dd7caa23 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 5 Sep 2024 16:10:17 -0700 Subject: [PATCH 36/38] Make configured a fallback --- internal/subshell/subshell.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 4769ccda5d..a0a0746c9a 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -189,11 +189,11 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { } if binary == "" { - binary = configured + binary = detectShellParent() } if binary == "" { - binary = detectShellParent() + binary = configured } if binary == "" { From bc1e2eca0e129777db102da6f21c473354d6656b Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 5 Sep 2024 16:10:38 -0700 Subject: [PATCH 37/38] Make process error a multilog error --- internal/subshell/subshell.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index a0a0746c9a..8844de8e69 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -248,7 +248,7 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { func detectShellParent() string { p, err := process.NewProcess(int32(os.Getppid())) if err != nil && !errors.As(err, ptr.To(&os.PathError{})) { - logging.Error("Failed to get parent process: %s", errs.JoinMessage(err)) + multilog.Error("Failed to get parent process: %s", errs.JoinMessage(err)) } for p != nil && p.Pid != 0 { From a7ac5d8ba1b6de7f0653e7e0b6ae5bc123a3a08e Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Fri, 6 Sep 2024 10:21:00 -0700 Subject: [PATCH 38/38] Add back powershell change --- internal/testhelpers/e2e/spawn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testhelpers/e2e/spawn.go b/internal/testhelpers/e2e/spawn.go index bf73ffcd22..fff1cd25b8 100644 --- a/internal/testhelpers/e2e/spawn.go +++ b/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 \"`\"" + send = "echo \"\"" expect = `` } else { send = `echo ^`