all: use executil

Author: schzhn

internal/agh/agh.go

@@ -3,6 +3,12 @@ package agh
 
 import (
 	"context"
+	"fmt"
+	"strings"
+
+	"github.com/AdguardTeam/golibs/osutil"
+	"github.com/AdguardTeam/golibs/osutil/executil"
+	"github.com/AdguardTeam/golibs/testutil/fakeos/fakeexec"
 )
 
 // ConfigModifier defines an interface for updating the global configuration.
@@ -20,3 +26,124 @@ var _ ConfigModifier = EmptyConfigModifier{}
 
 // Apply implements the [ConfigModifier] for EmptyConfigModifier.
 func (em EmptyConfigModifier) Apply(ctx context.Context) {}
+
+// TODO(s.chzhen): !! Is there another way?
+//
+// TODO(s.chzhen): !! Docs, naming.
+//
+// TODO(s.chzhen):  Move to aghtest once the import cycle is resolved.
+type exitErr struct {
+	code osutil.ExitCode
+}
+
+// type check
+var _ executil.ExitCodeError = exitErr{}
+
+func (e exitErr) Error() (s string) {
+	return fmt.Sprintf("exit code %d", e.code)
+}
+
+func (e exitErr) ExitCode() (code osutil.ExitCode) {
+	return e.code
+}
+
+type ExternalCommand struct {
+	Err  error
+	Cmd  string
+	Out  string
+	Code int
+}
+
+func keyCommand(path string, args []string) (k string) {
+	return path + " " + strings.Join(args, " ")
+}
+
+func parseCommand(s string) (path string, args []string) {
+	f := strings.Fields(s)
+	if len(f) == 0 {
+		return "", nil
+	}
+
+	return f[0], f[1:]
+}
+
+// NewMultipleCommandConstructor is a helper function that returns a mock
+// [executil.CommandConstructor] for tests.
+func NewMultipleCommandConstructor(cmds ...ExternalCommand) (cs executil.CommandConstructor) {
+	table := make(map[string]ExternalCommand, len(cmds))
+	for _, ec := range cmds {
+		p, a := parseCommand(ec.Cmd)
+		table[keyCommand(p, a)] = ec
+	}
+
+	return &fakeexec.CommandConstructor{
+		OnNew: func(
+			_ context.Context,
+			conf *executil.CommandConfig,
+		) (c executil.Command, err error) {
+			ec := table[keyCommand(conf.Path, conf.Args)]
+
+			cmd := fakeexec.NewCommand()
+			cmd.OnStart = func(_ context.Context) (err error) {
+				if ec.Out != "" {
+					_, _ = conf.Stdout.Write([]byte(ec.Out))
+				}
+
+				return nil
+			}
+
+			cmd.OnWait = func(_ context.Context) (err error) {
+				if ec.Err != nil {
+					return ec.Err
+				}
+
+				if ec.Code != 0 {
+					return exitErr{code: ec.Code}
+				}
+
+				return nil
+			}
+
+			return cmd, nil
+		},
+	}
+}
+
+// NewCommandConstructor is a helper function that returns a mock
+// [executil.CommandConstructor] for tests.
+func NewCommandConstructor(
+	_ string,
+	code int,
+	stdout string,
+	cmdErr error,
+) (cs executil.CommandConstructor) {
+	return &fakeexec.CommandConstructor{
+		OnNew: func(
+			_ context.Context,
+			conf *executil.CommandConfig,
+		) (c executil.Command, err error) {
+			cmd := fakeexec.NewCommand()
+			cmd.OnStart = func(_ context.Context) (err error) {
+				if conf.Stdout != nil {
+					_, _ = conf.Stdout.Write([]byte(stdout))
+				}
+
+				return nil
+			}
+
+			cmd.OnWait = func(_ context.Context) (err error) {
+				if cmdErr != nil {
+					return cmdErr
+				}
+
+				if code != 0 {
+					return exitErr{code: code}
+				}
+
+				return nil
+			}
+
+			return cmd, nil
+		},
+	}
+}

internal/aghnet/net.go

@@ -18,6 +18,7 @@ import (
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/osutil"
+	"github.com/AdguardTeam/golibs/osutil/executil"
 )
 
 // DialContextFunc is the semantic alias for dialing functions, such as
@@ -27,7 +28,16 @@ type DialContextFunc = func(ctx context.Context, network, addr string) (conn net
 // Variables and functions to substitute in tests.
 var (
 	// aghosRunCommand is the function to run shell commands.
-	aghosRunCommand = aghos.RunCommand
+	//
+	// TODO(s.chzhen):  Use [aghos.RunCommand] directly.
+	aghosRunCommand = (func() func(string, ...string) (int, []byte, error) {
+		ctx := context.TODO()
+		cmdCons := executil.SystemCommandConstructor{}
+
+		return func(command string, arguments ...string) (int, []byte, error) {
+			return aghos.RunCommand(ctx, cmdCons, command, arguments...)
+		}
+	})()
 
 	// netInterfaces is the function to get the available network interfaces.
 	netInterfaceAddrs = net.InterfaceAddrs

internal/aghos/os.go

@@ -5,20 +5,23 @@ package aghos
 
 import (
 	"bufio"
+	"bytes"
 	"context"
 	"fmt"
 	"io"
 	"io/fs"
 	"log/slog"
 	"os"
-	"os/exec"
 	"path"
 	"runtime"
 	"slices"
 	"strconv"
 	"strings"
 
 	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/ioutil"
+	"github.com/AdguardTeam/golibs/osutil"
+	"github.com/AdguardTeam/golibs/osutil/executil"
 )
 
 // Default file, binary, and directory permissions.
@@ -50,23 +53,48 @@ func HaveAdminRights() (bool, error) {
 const MaxCmdOutputSize = 64 * 1024
 
 // RunCommand runs shell command.
-func RunCommand(command string, arguments ...string) (code int, output []byte, err error) {
-	cmd := exec.Command(command, arguments...)
-	out, err := cmd.Output()
-
-	out = out[:min(len(out), MaxCmdOutputSize)]
-
-	if err != nil {
-		if eerr := new(exec.ExitError); errors.As(err, &eerr) {
-			return eerr.ExitCode(), eerr.Stderr, nil
-		}
+//
+// TODO(s.chzhen):  Consider removing this after addressing the current behavior
+// where a non-zero exit code is returned together with a nil error.
+func RunCommand(
+	ctx context.Context,
+	cmdCons executil.CommandConstructor,
+	command string,
+	arguments ...string,
+) (code int, output []byte, err error) {
+	stdoutBuf := bytes.Buffer{}
+	stderrBuf := bytes.Buffer{}
+
+	err = executil.Run(
+		ctx,
+		cmdCons,
+		&executil.CommandConfig{
+			Path:   command,
+			Args:   arguments,
+			Stdout: ioutil.NewTruncatedWriter(&stdoutBuf, MaxCmdOutputSize),
+			Stderr: &stderrBuf,
+		},
+	)
+
+	if err == nil {
+		return osutil.ExitCodeSuccess, stdoutBuf.Bytes(), nil
+	}
 
-		return 1, nil, fmt.Errorf("command %q failed: %w: %s", command, err, out)
+	code, ok := executil.ExitCodeFromError(err)
+	if ok {
+		// Mirror the old behavior and return a nil-error on non-zero code
+		// status.
+		return code, stderrBuf.Bytes(), nil
 	}
 
-	return cmd.ProcessState.ExitCode(), out, nil
+	return osutil.ExitCodeFailure,
+		nil,
+		fmt.Errorf("command %q failed: %w: %s", command, err, stdoutBuf.Bytes())
 }
 
+// psArgs holds the default ps arguments to avoid per-call slice allocations.
+var psArgs = []string{"-A", "-o", "pid=", "-o", "comm="}
+
 // PIDByCommand searches for process named command and returns its PID ignoring
 // the PIDs from except.  If no processes found, the error returned.  l must not
 // be nil.
@@ -76,31 +104,34 @@ func PIDByCommand(
 	command string,
 	except ...int,
 ) (pid int, err error) {
+	const psCmd = "ps"
+
+	l.DebugContext(ctx, "executing", "cmd", psCmd, "args", psArgs)
+
 	// Don't use -C flag here since it's a feature of linux's ps
 	// implementation.  Use POSIX-compatible flags instead.
 	//
 	// See https://github.com/AdguardTeam/AdGuardHome/issues/3457.
-	cmd := exec.Command("ps", "-A", "-o", "pid=", "-o", "comm=")
-
-	var stdout io.ReadCloser
-	if stdout, err = cmd.StdoutPipe(); err != nil {
-		return 0, fmt.Errorf("getting the command's stdout pipe: %w", err)
-	}
-
-	if err = cmd.Start(); err != nil {
-		return 0, fmt.Errorf("start command executing: %w", err)
+	stdoutBuf := bytes.Buffer{}
+	err = executil.Run(
+		ctx,
+		executil.SystemCommandConstructor{},
+		&executil.CommandConfig{
+			Path:   psCmd,
+			Args:   psArgs,
+			Stdout: &stdoutBuf,
+		},
+	)
+	if err != nil {
+		return 0, fmt.Errorf("executing the command: %w", err)
 	}
 
 	var instNum int
-	pid, instNum, err = parsePSOutput(stdout, command, except)
+	pid, instNum, err = parsePSOutput(&stdoutBuf, command, except)
 	if err != nil {
 		return 0, err
 	}
 
-	if err = cmd.Wait(); err != nil {
-		return 0, fmt.Errorf("executing the command: %w", err)
-	}
-
 	switch instNum {
 	case 0:
 		// TODO(e.burkov):  Use constant error.
@@ -111,10 +142,6 @@ func PIDByCommand(
 		l.WarnContext(ctx, "instances found", "num", instNum, "command", command)
 	}
 
-	if code := cmd.ProcessState.ExitCode(); code != 0 {
-		return 0, fmt.Errorf("ps finished with code %d", code)
-	}
-
 	return pid, nil
 }
 

internal/arpdb/arpdb.go

@@ -4,25 +4,23 @@ package arpdb
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"fmt"
 	"log/slog"
 	"net"
 	"net/netip"
 	"slices"
 	"sync"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/osutil"
+	"github.com/AdguardTeam/golibs/osutil/executil"
 )
 
 // Variables and functions to substitute in tests.
 var (
-	// aghosRunCommand is the function to run shell commands.
-	aghosRunCommand = aghos.RunCommand
-
 	// rootDirFS is the filesystem pointing to the root directory.
 	rootDirFS = osutil.RootDirFS()
 )
@@ -40,7 +38,7 @@ type Interface interface {
 
 // New returns the [Interface] properly initialized for the OS.
 func New(logger *slog.Logger) (arp Interface) {
-	return newARPDB(logger)
+	return newARPDB(logger, executil.SystemCommandConstructor{})
 }
 
 // Empty is the [Interface] implementation that does nothing.
@@ -164,11 +162,12 @@ type parseNeighsFunc func(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (
 // cmdARPDB is the implementation of the [Interface] that uses command line to
 // retrieve data.
 type cmdARPDB struct {
-	logger *slog.Logger
-	parse  parseNeighsFunc
-	ns     *neighs
-	cmd    string
-	args   []string
+	logger  *slog.Logger
+	cmdCons executil.CommandConstructor
+	parse   parseNeighsFunc
+	ns      *neighs
+	cmd     string
+	args    []string
 }
 
 // type check
@@ -178,14 +177,26 @@ var _ Interface = (*cmdARPDB)(nil)
 func (arp *cmdARPDB) Refresh() (err error) {
 	defer func() { err = errors.Annotate(err, "cmd arpdb: %w") }()
 
-	code, out, err := aghosRunCommand(arp.cmd, arp.args...)
+	var stdout bytes.Buffer
+	err = executil.Run(
+		// TODO(s.chzhen):  Pass context.
+		context.TODO(),
+		arp.cmdCons,
+		&executil.CommandConfig{
+			Path:   arp.cmd,
+			Args:   arp.args,
+			Stdout: &stdout,
+		},
+	)
 	if err != nil {
+		if code, ok := executil.ExitCodeFromError(err); ok {
+			return fmt.Errorf("running command: unexpected exit code %d", code)
+		}
+
 		return fmt.Errorf("running command: %w", err)
-	} else if code != 0 {
-		return fmt.Errorf("running command: unexpected exit code %d", code)
 	}
 
-	sc := bufio.NewScanner(bytes.NewReader(out))
+	sc := bufio.NewScanner(&stdout)
 	ns := arp.parse(arp.logger, sc, arp.ns.len())
 	if err = sc.Err(); err != nil {
 		// TODO(e.burkov):  This error seems unreachable.  Investigate.