From 64fe4e2c838a76deac8c1394a0e59d2f7e96e211 Mon Sep 17 00:00:00 2001 From: sewn Date: Sun, 11 Feb 2024 10:47:08 +0300 Subject: [PATCH] cmd/vinegar: move stderr pipe to wine, minor doc changes --- cmd/vinegar/binary.go | 31 ++---------------- cmd/vinegar/main.go | 3 +- wine/cmd.go | 73 +++++++++++++++++++++++++++++++++++++++++++ wine/tricks.go | 2 +- wine/wine.go | 20 ++---------- 5 files changed, 82 insertions(+), 47 deletions(-) create mode 100644 wine/cmd.go diff --git a/cmd/vinegar/binary.go b/cmd/vinegar/binary.go index ae9be9f4..c3eebb1f 100644 --- a/cmd/vinegar/binary.go +++ b/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 { diff --git a/cmd/vinegar/main.go b/cmd/vinegar/main.go index 70c33e3b..61719610 100644 --- a/cmd/vinegar/main.go +++ b/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" diff --git a/wine/cmd.go b/wine/cmd.go new file mode 100644 index 00000000..92d0477b --- /dev/null +++ b/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() +} diff --git a/wine/tricks.go b/wine/tricks.go index 9c4466a7..dbd738fe 100644 --- a/wine/tricks.go +++ b/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. diff --git a/wine/wine.go b/wine/wine.go index 8030b72d..17659b67 100644 --- a/wine/wine.go +++ b/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")