From 03d0e6a62b3401feb7cb92bc7418b51f64c6e869 Mon Sep 17 00:00:00 2001 From: Gustavo Parreira Date: Tue, 9 Dec 2025 18:23:20 +0000 Subject: [PATCH 1/3] fix: install mac games on macOS by default. kill child processes when freecarnival is killed --- cmd_install.go | 7 +++- cmd_launch.go | 7 +++- launch/launch.go | 91 ++++++++++++++++++++++++++++++++++-------------- 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/cmd_install.go b/cmd_install.go index bf0986a..4cb9eea 100644 --- a/cmd_install.go +++ b/cmd_install.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "github.com/gustash/freecarnival/auth" "github.com/gustash/freecarnival/download" @@ -65,7 +66,11 @@ the latest version for the current OS will be used.`, buildOS = auth.BuildOSMac case "": // Default based on current OS - buildOS = auth.BuildOSWindows // Default to Windows if not specified + if runtime.GOOS == "darwin" { + buildOS = auth.BuildOSMac + } else { + buildOS = auth.BuildOSWindows + } default: return fmt.Errorf("invalid OS '%s': must be windows, linux, or mac", targetOS) } diff --git a/cmd_launch.go b/cmd_launch.go index 0da9675..c04496d 100644 --- a/cmd_launch.go +++ b/cmd_launch.go @@ -1,6 +1,7 @@ package main import ( + "context" "fmt" "github.com/gustash/freecarnival/auth" @@ -101,7 +102,11 @@ Use --wine to specify a custom Wine path, or --no-wine to disable Wine.`, WinePrefix: winePrefix, NoWine: noWine, } - if err := launch.Game(exe.Path, installInfo.OS, gameArgs, launchOpts); err != nil { + if err := launch.Game(cmd.Context(), exe.Path, installInfo.OS, gameArgs, launchOpts); err != nil { + // Context cancellation (Ctrl+C) is not an error - user intentionally killed the game + if err == context.Canceled { + return nil + } return fmt.Errorf("failed to launch game: %w", err) } diff --git a/launch/launch.go b/launch/launch.go index e078ae6..f11b060 100644 --- a/launch/launch.go +++ b/launch/launch.go @@ -2,14 +2,17 @@ package launch import ( + "context" "fmt" "os" "os/exec" "path/filepath" "runtime" "strings" + "syscall" "github.com/gustash/freecarnival/auth" + "github.com/gustash/freecarnival/logger" ) // Executable represents a launchable executable. @@ -157,7 +160,8 @@ func isIgnoredExecutable(name string) bool { } // Game launches the specified executable with optional arguments. -func Game(executablePath string, buildOS auth.BuildOS, args []string, opts *Options) error { +// It waits for the process to complete and kills it if the context is cancelled. +func Game(ctx context.Context, executablePath string, buildOS auth.BuildOS, args []string, opts *Options) error { if _, err := os.Stat(executablePath); os.IsNotExist(err) { return fmt.Errorf("executable not found: %s", executablePath) } @@ -166,41 +170,26 @@ func Game(executablePath string, buildOS auth.BuildOS, args []string, opts *Opti opts = &Options{} } - // On macOS, use 'open' command for .app bundles - if runtime.GOOS == "darwin" && strings.Contains(executablePath, ".app/Contents/MacOS/") { - appPath := executablePath - if idx := strings.Index(executablePath, ".app/"); idx != -1 { - appPath = executablePath[:idx+4] - } - - cmdArgs := []string{"-a", appPath} - if len(args) > 0 { - cmdArgs = append(cmdArgs, "--args") - cmdArgs = append(cmdArgs, args...) - } - - cmd := exec.Command("open", cmdArgs...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - return cmd.Run() - } - needsWine := buildOS == auth.BuildOSWindows && runtime.GOOS != "windows" && !opts.NoWine if needsWine { - return launchWithWine(executablePath, args, opts) + return launchWithWine(ctx, executablePath, args, opts) } - cmd := exec.Command(executablePath, args...) + return launchNative(ctx, executablePath, args) +} + +func launchNative(ctx context.Context, executablePath string, args []string) error { + cmd := exec.CommandContext(ctx, executablePath, args...) cmd.Dir = filepath.Dir(executablePath) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr cmd.Stdin = os.Stdin - return cmd.Start() + return launchProcess(ctx, cmd) } -func launchWithWine(executablePath string, args []string, opts *Options) error { +func launchWithWine(ctx context.Context, executablePath string, args []string, opts *Options) error { winePath := opts.WinePath if winePath == "" { winePath = findWine() @@ -217,7 +206,7 @@ func launchWithWine(executablePath string, args []string, opts *Options) error { } cmdArgs := append([]string{executablePath}, args...) - cmd := exec.Command(winePath, cmdArgs...) + cmd := exec.CommandContext(ctx, winePath, cmdArgs...) cmd.Dir = filepath.Dir(executablePath) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -227,7 +216,57 @@ func launchWithWine(executablePath string, args []string, opts *Options) error { cmd.Env = append(os.Environ(), "WINEPREFIX="+opts.WinePrefix) } - return cmd.Start() + return launchProcess(ctx, cmd) +} + +func launchProcess(ctx context.Context, cmd *exec.Cmd) error { + // Set up process group on Unix systems + if runtime.GOOS != "windows" { + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + } + } + + if err := cmd.Start(); err != nil { + return err + } + + // Wait for process or context cancellation + done := make(chan error, 1) + go func() { + done <- cmd.Wait() + }() + + select { + case <-ctx.Done(): + logger.Info("Terminating game process...") + if err := killProcessGroup(cmd); err != nil { + logger.Warn("Failed to kill process group", "error", err) + } + return ctx.Err() + case err := <-done: + return err + } +} + +// killProcessGroup kills the process and its entire process group +func killProcessGroup(cmd *exec.Cmd) error { + if cmd.Process == nil { + return nil + } + + if runtime.GOOS == "windows" { + return cmd.Process.Kill() + } + + // On Unix, kill the entire process group + pgid, err := syscall.Getpgid(cmd.Process.Pid) + if err != nil { + return cmd.Process.Kill() + } + + // Kill process group (negative PID kills the group) + return syscall.Kill(-pgid, syscall.SIGTERM) } var defaultWineCandidates = []string{ From 8a7171fb6a820e7ea7df9097ae7b3e3fec597278 Mon Sep 17 00:00:00 2001 From: Gustavo Parreira Date: Tue, 9 Dec 2025 19:02:00 +0000 Subject: [PATCH 2/3] fix: address PR comments --- cmd_install.go | 2 ++ cmd_launch.go | 3 ++- launch/launch.go | 9 ++++++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cmd_install.go b/cmd_install.go index 4cb9eea..286dfad 100644 --- a/cmd_install.go +++ b/cmd_install.go @@ -66,6 +66,8 @@ the latest version for the current OS will be used.`, buildOS = auth.BuildOSMac case "": // Default based on current OS + // macOS: prefer native builds + // Linux: prefer Windows builds (for Wine compatibility) if runtime.GOOS == "darwin" { buildOS = auth.BuildOSMac } else { diff --git a/cmd_launch.go b/cmd_launch.go index c04496d..aaa9b5c 100644 --- a/cmd_launch.go +++ b/cmd_launch.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "github.com/gustash/freecarnival/auth" @@ -104,7 +105,7 @@ Use --wine to specify a custom Wine path, or --no-wine to disable Wine.`, } if err := launch.Game(cmd.Context(), exe.Path, installInfo.OS, gameArgs, launchOpts); err != nil { // Context cancellation (Ctrl+C) is not an error - user intentionally killed the game - if err == context.Canceled { + if errors.Is(err, context.Canceled) { return nil } return fmt.Errorf("failed to launch game: %w", err) diff --git a/launch/launch.go b/launch/launch.go index f11b060..a514543 100644 --- a/launch/launch.go +++ b/launch/launch.go @@ -180,7 +180,7 @@ func Game(ctx context.Context, executablePath string, buildOS auth.BuildOS, args } func launchNative(ctx context.Context, executablePath string, args []string) error { - cmd := exec.CommandContext(ctx, executablePath, args...) + cmd := exec.Command(executablePath, args...) cmd.Dir = filepath.Dir(executablePath) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -206,7 +206,7 @@ func launchWithWine(ctx context.Context, executablePath string, args []string, o } cmdArgs := append([]string{executablePath}, args...) - cmd := exec.CommandContext(ctx, winePath, cmdArgs...) + cmd := exec.Command(winePath, cmdArgs...) cmd.Dir = filepath.Dir(executablePath) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -243,6 +243,7 @@ func launchProcess(ctx context.Context, cmd *exec.Cmd) error { if err := killProcessGroup(cmd); err != nil { logger.Warn("Failed to kill process group", "error", err) } + <-done return ctx.Err() case err := <-done: return err @@ -256,7 +257,9 @@ func killProcessGroup(cmd *exec.Cmd) error { } if runtime.GOOS == "windows" { - return cmd.Process.Kill() + // Use taskkill to kill the process tree on Windows + taskkill := exec.Command("taskkill", "/PID", fmt.Sprintf("%d", cmd.Process.Pid), "/T", "/F") + return taskkill.Run() } // On Unix, kill the entire process group From 1da560a4049a85eab69e2ab49b6aa2bfededfe7f Mon Sep 17 00:00:00 2001 From: Gustavo Parreira Date: Tue, 9 Dec 2025 19:12:26 +0000 Subject: [PATCH 3/3] test: added test coverage for launch game --- launch/launch_test.go | 321 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 321 insertions(+) diff --git a/launch/launch_test.go b/launch/launch_test.go index b4f6cd6..edd4b20 100644 --- a/launch/launch_test.go +++ b/launch/launch_test.go @@ -1,9 +1,14 @@ package launch import ( + "context" "os" + "os/exec" "path/filepath" + "runtime" + "strings" "testing" + "time" "github.com/gustash/freecarnival/auth" ) @@ -318,3 +323,319 @@ func TestFindWine_Integration(t *testing.T) { t.Log("Wine not found (expected if not installed)") } } + +func TestGame_NativeExecutable(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create a simple test script that exits immediately + scriptContent := "" + scriptName := "test_game" + if runtime.GOOS == "windows" { + scriptName = "test_game.bat" + scriptContent = "@echo off\nexit /b 0" + } else { + scriptContent = "#!/bin/sh\nexit 0" + } + + scriptPath := filepath.Join(tmpDir, scriptName) + if err := os.WriteFile(scriptPath, []byte(scriptContent), 0o755); err != nil { + t.Fatalf("failed to create script: %v", err) + } + + ctx := context.Background() + buildOS := auth.BuildOSLinux + switch runtime.GOOS { + case "darwin": + buildOS = auth.BuildOSMac + case "windows": + buildOS = auth.BuildOSWindows + } + + err = Game(ctx, scriptPath, buildOS, nil, nil) + if err != nil { + t.Errorf("Game() failed: %v", err) + } +} + +func TestGame_WithArguments(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create a script that expects arguments + scriptContent := "" + scriptName := "test_game" + if runtime.GOOS == "windows" { + scriptName = "test_game.bat" + scriptContent = "@echo off\nif \"%1\" == \"--test\" exit /b 0\nexit /b 1" + } else { + scriptContent = "#!/bin/sh\nif [ \"$1\" = \"--test\" ]; then exit 0; else exit 1; fi" + } + + scriptPath := filepath.Join(tmpDir, scriptName) + if err := os.WriteFile(scriptPath, []byte(scriptContent), 0o755); err != nil { + t.Fatalf("failed to create script: %v", err) + } + + ctx := context.Background() + buildOS := auth.BuildOSLinux + switch runtime.GOOS { + case "darwin": + buildOS = auth.BuildOSMac + case "windows": + buildOS = auth.BuildOSWindows + } + + args := []string{"--test"} + err = Game(ctx, scriptPath, buildOS, args, nil) + if err != nil { + t.Errorf("Game() with args failed: %v", err) + } +} + +func TestGame_ContextCancellation(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create a script with an infinite loop + scriptContent := "" + scriptName := "test_game" + if runtime.GOOS == "windows" { + scriptName = "test_game.bat" + scriptContent = "@echo off\n:loop\ngoto loop" + } else { + scriptContent = "#!/bin/sh\nwhile true; do sleep 1; done" + } + + scriptPath := filepath.Join(tmpDir, scriptName) + if err := os.WriteFile(scriptPath, []byte(scriptContent), 0o755); err != nil { + t.Fatalf("failed to create script: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + buildOS := auth.BuildOSLinux + switch runtime.GOOS { + case "darwin": + buildOS = auth.BuildOSMac + case "windows": + buildOS = auth.BuildOSWindows + } + + // Launch in goroutine and cancel after a short delay + done := make(chan error, 1) + go func() { + done <- Game(ctx, scriptPath, buildOS, nil, nil) + }() + + // Wait a bit then cancel + time.Sleep(100 * time.Millisecond) + cancel() + + // Should return context.Canceled + select { + case err := <-done: + if err != context.Canceled { + t.Errorf("expected context.Canceled, got %v", err) + } + case <-time.After(5 * time.Second): + t.Error("process did not terminate after context cancellation") + } + + // Verify process is actually dead + time.Sleep(100 * time.Millisecond) // Give it a moment to clean up + if isProcessRunning(scriptName) { + t.Error("process is still running after cancellation") + } +} + +// isProcessRunning checks if a process with the given name is running +func isProcessRunning(name string) bool { + var cmd *exec.Cmd + if runtime.GOOS == "windows" { + cmd = exec.Command("tasklist", "/FI", "IMAGENAME eq "+name) + } else { + cmd = exec.Command("pgrep", "-f", name) + } + + output, err := cmd.Output() + if err != nil { + // pgrep returns exit code 1 when no processes found + return false + } + + // For Windows, check if the output contains the process name + if runtime.GOOS == "windows" { + return strings.Contains(string(output), name) + } + + // For Unix, pgrep returns PIDs if found + return len(strings.TrimSpace(string(output))) > 0 +} + +func TestGame_MissingExecutable(t *testing.T) { + ctx := context.Background() + nonExistentPath := "/tmp/nonexistent/game.exe" + + err := Game(ctx, nonExistentPath, auth.BuildOSLinux, nil, nil) + if err == nil { + t.Error("expected error for missing executable") + } +} + +func TestGame_WithWine(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Wine test not applicable on Windows") + } + + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create a fake wine executable + fakeWinePath := filepath.Join(tmpDir, "wine") + wineContent := "#!/bin/sh\nexit 0" + if err := os.WriteFile(fakeWinePath, []byte(wineContent), 0o755); err != nil { + t.Fatalf("failed to create fake wine: %v", err) + } + + // Create a fake Windows executable + gameExePath := filepath.Join(tmpDir, "game.exe") + if err := os.WriteFile(gameExePath, []byte("fake exe"), 0o644); err != nil { + t.Fatalf("failed to create fake exe: %v", err) + } + + ctx := context.Background() + opts := &Options{ + WinePath: fakeWinePath, + } + + err = Game(ctx, gameExePath, auth.BuildOSWindows, nil, opts) + if err != nil { + t.Errorf("Game() with Wine failed: %v", err) + } +} + +func TestGame_NoWineFlag(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Wine test not applicable on Windows") + } + + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create a fake Windows executable + gameExePath := filepath.Join(tmpDir, "game.exe") + exeContent := "#!/bin/sh\nexit 0" + if err := os.WriteFile(gameExePath, []byte(exeContent), 0o755); err != nil { + t.Fatalf("failed to create fake exe: %v", err) + } + + ctx := context.Background() + opts := &Options{ + NoWine: true, // Disable Wine + } + + // Should launch directly without Wine + err = Game(ctx, gameExePath, auth.BuildOSWindows, nil, opts) + if err != nil { + t.Errorf("Game() with NoWine failed: %v", err) + } +} + +func TestGame_MacAppBundle(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create a .app bundle structure + appPath := filepath.Join(tmpDir, "TestGame.app") + macOSPath := filepath.Join(appPath, "Contents", "MacOS") + if err := os.MkdirAll(macOSPath, 0o755); err != nil { + t.Fatalf("failed to create app bundle: %v", err) + } + + // Create executable inside bundle + execPath := filepath.Join(macOSPath, "TestGame") + execContent := "#!/bin/sh\nexit 0" + if err := os.WriteFile(execPath, []byte(execContent), 0o755); err != nil { + t.Fatalf("failed to create executable: %v", err) + } + + ctx := context.Background() + err = Game(ctx, execPath, auth.BuildOSMac, nil, nil) + if err != nil { + t.Errorf("Game() for macOS app failed: %v", err) + } +} + +func TestLaunchProcess_Success(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + scriptContent := "" + scriptName := "test_script" + if runtime.GOOS == "windows" { + scriptName = "test_script.bat" + scriptContent = "@echo off\nexit /b 0" + } else { + scriptContent = "#!/bin/sh\nexit 0" + } + + scriptPath := filepath.Join(tmpDir, scriptName) + if err := os.WriteFile(scriptPath, []byte(scriptContent), 0o755); err != nil { + t.Fatalf("failed to create script: %v", err) + } + + ctx := context.Background() + err = launchNative(ctx, scriptPath, nil) + if err != nil { + t.Errorf("launchNative() failed: %v", err) + } +} + +func TestLaunchProcess_NonZeroExit(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "launch-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + scriptContent := "" + scriptName := "test_script" + if runtime.GOOS == "windows" { + scriptName = "test_script.bat" + scriptContent = "@echo off\nexit /b 42" + } else { + scriptContent = "#!/bin/sh\nexit 42" + } + + scriptPath := filepath.Join(tmpDir, scriptName) + if err := os.WriteFile(scriptPath, []byte(scriptContent), 0o755); err != nil { + t.Fatalf("failed to create script: %v", err) + } + + ctx := context.Background() + err = launchNative(ctx, scriptPath, nil) + if err == nil { + t.Error("expected error for non-zero exit code") + } +}