From a697b9eb1283ca60d3ec3f5c1e3b4ade6ce72fae Mon Sep 17 00:00:00 2001 From: Praveen Kumar Date: Thu, 2 Jan 2025 21:25:56 +0530 Subject: [PATCH] Shell: Use ps command which works for linux and mac As part of 74d1f1ba4038129ac9ee0aaacedeb742b4375bd9 ps command has `--sort=-pid` which doesn't work with macOS by default so in this PR `--sort=-pid` option is removed and reverse sorting against pid is implemented in `inspectProcessOutputForRecentlyUsedShell`. This way this code works on mac/linux same way. fixes: #4537 --- pkg/os/shell/shell.go | 11 +++++++++-- pkg/os/shell/shell_unix.go | 2 +- pkg/os/shell/shell_unix_test.go | 4 ++-- pkg/os/shell/shell_windows.go | 2 +- pkg/os/shell/shell_windows_test.go | 2 +- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/pkg/os/shell/shell.go b/pkg/os/shell/shell.go index a9df3211b9..e469a35018 100644 --- a/pkg/os/shell/shell.go +++ b/pkg/os/shell/shell.go @@ -3,9 +3,11 @@ package shell import ( "fmt" "os" + "sort" "strconv" "strings" + "github.com/crc-org/crc/v2/pkg/crc/logging" crcos "github.com/crc-org/crc/v2/pkg/os" ) @@ -174,13 +176,13 @@ func detectShellByInvokingCommand(defaultShell string, command string, args []st if detectedShell == "" { return defaultShell } + logging.Debugf("Detected shell: %s", detectedShell) return detectedShell } // inspectProcessOutputForRecentlyUsedShell inspects output of ps command to detect currently active shell session. // -// Note : This method assumes that ps command has already sorted the processes by `pid` in reverse order. -// It parses the output into a struct, filters process types by name and returns the first element. +// It parses the output into a struct, filters process types by name then reverse sort it with pid and returns the first element. // // It takes one argument: // @@ -222,6 +224,11 @@ func inspectProcessOutputForRecentlyUsedShell(psCommandOutput string) string { } } } + // Reverse sort the processes by PID (higher to lower) + sort.Slice(processOutputs, func(i, j int) bool { + return processOutputs[i].processID > processOutputs[j].processID + }) + if len(processOutputs) > 0 { return processOutputs[0].output } diff --git a/pkg/os/shell/shell_unix.go b/pkg/os/shell/shell_unix.go index 09e321b440..e75f0f1896 100644 --- a/pkg/os/shell/shell_unix.go +++ b/pkg/os/shell/shell_unix.go @@ -15,7 +15,7 @@ var ( // detect detects user's current shell. func detect() (string, error) { - detectedShell := detectShellByInvokingCommand("", "ps", []string{"-o", "pid=,comm=", "--sort=-pid"}) + detectedShell := detectShellByInvokingCommand("", "ps", []string{"-o", "pid=,comm="}) if detectedShell == "" { fmt.Printf("The default lines below are for a sh/bash shell, you can specify the shell you're using, with the --shell flag.\n\n") return "", ErrUnknownShell diff --git a/pkg/os/shell/shell_unix_test.go b/pkg/os/shell/shell_unix_test.go index 247416bce8..a11b5a0db5 100644 --- a/pkg/os/shell/shell_unix_test.go +++ b/pkg/os/shell/shell_unix_test.go @@ -33,7 +33,7 @@ func TestUnknownShell(t *testing.T) { assert.Greater(t, nBytesRead, int64(0)) assert.Equal(t, "The default lines below are for a sh/bash shell, you can specify the shell you're using, with the --shell flag.\n\n", buf.String()) assert.Equal(t, "ps", mockCommandExecutor.commandName) - assert.Equal(t, []string{"-o", "pid=,comm=", "--sort=-pid"}, mockCommandExecutor.commandArgs) + assert.Equal(t, []string{"-o", "pid=,comm="}, mockCommandExecutor.commandArgs) assert.Empty(t, shell) } @@ -78,7 +78,7 @@ func TestDetect_GivenPsOutputContainsShell_ThenReturnShellProcessWithMostRecentP // Then assert.Equal(t, "ps", mockCommandExecutor.commandName) - assert.Equal(t, []string{"-o", "pid=,comm=", "--sort=-pid"}, mockCommandExecutor.commandArgs) + assert.Equal(t, []string{"-o", "pid=,comm="}, mockCommandExecutor.commandArgs) assert.Equal(t, tt.expectedShellType, shell) assert.NoError(t, err) }) diff --git a/pkg/os/shell/shell_windows.go b/pkg/os/shell/shell_windows.go index e901be827f..85f64f6228 100644 --- a/pkg/os/shell/shell_windows.go +++ b/pkg/os/shell/shell_windows.go @@ -64,7 +64,7 @@ func shellType(shell string, defaultShell string) string { case strings.Contains(strings.ToLower(shell), "cmd"): return "cmd" case strings.Contains(strings.ToLower(shell), "wsl"): - return detectShellByInvokingCommand("bash", "wsl", []string{"-e", "bash", "-c", "ps -ao pid=,comm= --sort=-pid"}) + return detectShellByInvokingCommand("bash", "wsl", []string{"-e", "bash", "-c", "ps -ao pid=,comm="}) case filepath.IsAbs(shell) && strings.Contains(strings.ToLower(shell), "bash"): return "bash" default: diff --git a/pkg/os/shell/shell_windows_test.go b/pkg/os/shell/shell_windows_test.go index 882af7fc66..767e6d47b6 100644 --- a/pkg/os/shell/shell_windows_test.go +++ b/pkg/os/shell/shell_windows_test.go @@ -85,5 +85,5 @@ func TestDetectShellInWindowsSubsystemLinux(t *testing.T) { // Then assert.Equal(t, "wsl", mockCommandExecutor.commandName) - assert.Equal(t, []string{"-e", "bash", "-c", "ps -ao pid=,comm= --sort=-pid"}, mockCommandExecutor.commandArgs) + assert.Equal(t, []string{"-e", "bash", "-c", "ps -ao pid=,comm="}, mockCommandExecutor.commandArgs) }