Skip to content

Commit

Permalink
Shell: Use ps command which works for linux and mac
Browse files Browse the repository at this point in the history
As part of 74d1f1b 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
  • Loading branch information
praveenkumar committed Jan 6, 2025
1 parent c133e01 commit a697b9e
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 7 deletions.
11 changes: 9 additions & 2 deletions pkg/os/shell/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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:
//
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/os/shell/shell_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/os/shell/shell_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/os/shell/shell_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion pkg/os/shell/shell_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit a697b9e

Please sign in to comment.