cmd/vinegar: move stderr pipe to wine, minor doc changes

cmd/vinegar/binary.go

@@ -7,7 +7,6 @@ import (
 	"log"
 	"log/slog"
 	"os"
-	"os/exec"
 	"os/signal"
 	"path/filepath"
 	"strings"
@@ -242,9 +241,7 @@ func (b *Binary) Run(args ...string) error {
 		return fmt.Errorf("%s command: %w", b.Type, err)
 	}
 
-	// Act as the signal holder, as roblox/wine will not do anything with the INT signal.
-	// Additionally, if Vinegar got TERM, it will also immediately exit, but roblox
-	// continues running if the signal holder was not present.
+	// Roblox will keep running if it was sent SIGINT; requiring acting as the signal holder.
 	c := make(chan os.Signal, 1)
 	signal.Notify(c, os.Interrupt, syscall.SIGTERM)
 	go func() {
@@ -255,7 +252,7 @@ func (b *Binary) Run(args ...string) error {
 		// Only kill Roblox if it hasn't exited
 		if cmd.ProcessState == nil {
 			slog.Warn("Killing Roblox", "pid", cmd.Process.Pid)
-			// This way, cmd.Run() will return and the wineprefix killer will be ran.
+			// This way, cmd.Run() will return and vinegar (should) exit.
 			cmd.Process.Kill()
 		}
 
@@ -359,34 +356,12 @@ func (b *Binary) Tail(name string) {
 	}
 }
 
-func (b *Binary) Command(args ...string) (*exec.Cmd, error) {
+func (b *Binary) Command(args ...string) (*wine.Cmd, error) {
 	if strings.HasPrefix(strings.Join(args, " "), "roblox-studio:1") {
 		args = []string{"-protocolString", args[0]}
 	}
 
 	cmd := b.Prefix.Wine(filepath.Join(b.Dir, b.Type.Executable()), args...)
-	cmd.Stderr = nil
-	cmd.Stdout = nil
-
-	// There was a long discussion in #winehq regarding starting wine from 
-	// Go with os/exec when it's stderr and stdout was set to a file. This
-	// behavior causes wineserver to start alongside the process instead of
-	// the background, creating issues such as Wineserver waiting for processes
-	// alongside Roblox - having timeout issues, etc. A pipe is required to
-	// mitigate this behavior..
-	//
-	// Please help me. I've been going insane.
-	cmdErrPipe, err := cmd.StderrPipe()
-	if err != nil {
-		return nil, fmt.Errorf("stderr pipe: %w", err)
-	}
-
-	cmdOutPipe, err := cmd.StdoutPipe()
-	if err != nil {
-		return nil, fmt.Errorf("stdout pipe: %w", err)
-	}
-
-	go io.Copy(b.Prefix.Stderr, io.MultiReader(cmdErrPipe, cmdOutPipe))
 
 	launcher := strings.Fields(b.Config.Launcher)
 	if len(launcher) >= 1 {

cmd/vinegar/main.go

@@ -3,13 +3,14 @@ package main
 import (
 	"flag"
 	"fmt"
-	"golang.org/x/term"
 	"log"
 	"log/slog"
 	"os"
 	"path/filepath"
 	"time"
 
+	"golang.org/x/term"
+
 	"github.com/vinegarhq/vinegar/config"
 	"github.com/vinegarhq/vinegar/config/editor"
 	"github.com/vinegarhq/vinegar/internal/dirs"

wine/cmd.go

@@ -0,0 +1,73 @@
+package wine
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"io/fs"
+	"os"
+	"os/exec"
+)
+
+type Cmd struct {
+	*exec.Cmd
+}
+
+// Command returns the Cmd struct to execute the named program with the given arguments.
+// It is reccomended to use [Wine] to run wine as opposed to Command.
+//
+// There was a long discussion in #winehq regarding starting wine from
+// Go with os/exec when it's stderr and stdout was set to a file. This
+// behavior causes wineserver to start alongside the process instead of
+// the background, creating issues such as Wineserver waiting for processes
+// alongside the executable - having timeout issues, etc.
+// A stderr pipe will be made to mitigate this behavior in Start.
+//
+// For further information regarding Command, refer to [exec.Command].
+func (p *Prefix) Command(name string, arg ...string) *Cmd {
+	cmd := exec.Command(name, arg...)
+	cmd.Env = append(cmd.Environ(),
+		"WINEPREFIX="+p.dir,
+	)
+
+	cmd.Stderr = p.Stderr
+	cmd.Stdout = p.Stdout
+
+	return &Cmd{
+		Cmd: cmd,
+	}
+}
+
+// Refer to [exec.Cmd.Run].
+func (c *Cmd) Run() error {
+	if err := c.Start(); err != nil {
+		return err
+	}
+	return c.Wait()
+}
+
+// Refer to [exec.Cmd.Start] and [Command].
+func (c *Cmd) Start() error {
+	if c.Process != nil {
+		return errors.New("exec: already started")
+	}
+
+	if c.Stderr != nil && c.Stderr != os.Stderr {
+		pfxStderr := c.Stderr
+		c.Stderr = nil
+
+		cmdErrPipe, err := c.StderrPipe()
+		if err != nil {
+			return fmt.Errorf("stderr pipe: %w", err)
+		}
+
+		go func() {
+			_, err := io.Copy(pfxStderr, cmdErrPipe)
+			if err != nil && !errors.Is(err, fs.ErrClosed) {
+				panic(err)
+			}
+		}()
+	}
+
+	return c.Cmd.Start()
+}

wine/tricks.go

@@ -6,7 +6,7 @@ import (
 
 // Winetricks runs winetricks within the Prefix.
 func (p *Prefix) Winetricks() error {
-	return p.command("winetricks").Run()
+	return p.Command("winetricks").Run()
 }
 
 // SetDPI sets the Prefix's DPI to the named DPI.

wine/wine.go

@@ -103,12 +103,10 @@ func (p *Prefix) Dir() string {
 	return p.dir
 }
 
-// Wine returns a new [exec.Cmd] with wine64 as the named program.
-//
-// The path of wine64 will either be from $PATH or from Prefix's Root.
-func (p *Prefix) Wine(exe string, arg ...string) *exec.Cmd {
+// Wine returns a new Cmd with the prefix's Wine as the named program.
+func (p *Prefix) Wine(exe string, arg ...string) *Cmd {
 	arg = append([]string{exe}, arg...)
-	cmd := p.command(p.wine, arg...)
+	cmd := p.Command(p.wine, arg...)
 
 	if strings.Contains(strings.ToLower(p.wine), "ulwgl") {
 		cmd.Env = append(cmd.Environ(), "PROTON_VERB=runinprefix")
@@ -132,18 +130,6 @@ func (p *Prefix) Update() error {
 	return p.Wine("wineboot", "-u").Run()
 }
 
-func (p *Prefix) command(name string, arg ...string) *exec.Cmd {
-	cmd := exec.Command(name, arg...)
-	cmd.Env = append(cmd.Environ(),
-		"WINEPREFIX="+p.dir,
-	)
-
-	cmd.Stderr = p.Stderr
-	cmd.Stdout = p.Stdout
-
-	return cmd
-}
-
 // Version returns the wineprefix's Wine version.
 func (p *Prefix) Version() string {
 	cmd := p.Wine("--version")