Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/google/go-cmp v0.6.0
github.com/mattn/go-colorable v0.1.13
github.com/mattn/go-runewidth v0.0.16
github.com/mattn/go-shellwords v1.0.12
github.com/robfig/cron/v3 v3.0.1
github.com/yuin/goldmark v1.7.8
golang.org/x/sync v0.9.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc=
github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk=
github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
Expand Down
30 changes: 29 additions & 1 deletion process.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os/exec"
"sync"

"github.com/mattn/go-shellwords"
"golang.org/x/sync/errgroup"
"golang.org/x/sync/semaphore"
"golang.org/x/sys/execabs"
Expand Down Expand Up @@ -109,18 +110,38 @@ func (proc *concurrentProcess) wait() {
// newCommandRunner creates new external command runner for given executable. The executable path
// is resolved in this function.
func (proc *concurrentProcess) newCommandRunner(exe string, combineOutput bool) (*externalCommand, error) {
p, err := execabs.LookPath(exe)
var args []string
p, args, err := findExe(exe)
if err != nil {
return nil, err
}
cmd := &externalCommand{
proc: proc,
exe: p,
args: args,
combineOutput: combineOutput,
}
return cmd, nil
}

func findExe(exe string) (string, []string, error) {
p, err := execabs.LookPath(exe)
if err == nil {
return p, nil, nil
}
// See if the command string contains args. As it is best effort, we do not
// handle parse errors.
if exeArgs, _ := shellwords.Parse(exe); len(exeArgs) > 0 {
// We want to return the original error if this command isn't found so
// do not overwrite it.
if p, err2 := execabs.LookPath(exeArgs[0]); err2 == nil {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if p, err2 := execabs.LookPath(exeArgs[0]); err2 == nil {
if p, err := execabs.LookPath(exeArgs[0]); err == nil {

Would you shadow the outer err variable? It is okay because this variable is very short-lived.

return p, exeArgs[1:], nil
}
}

return "", nil, err
}

// externalCommand is struct to run specific command concurrently with concurrentProcess bounding
// number of processes at the same time. This type manages fatal errors while running the command
// by using errgroup.Group. The wait() method must be called at the end for checking if some fatal
Expand All @@ -129,13 +150,20 @@ type externalCommand struct {
proc *concurrentProcess
eg errgroup.Group
exe string
args []string
combineOutput bool
}

// run runs the command with given arguments and stdin. The callback function is called after the
// process runs. First argument is stdout and the second argument is an error while running the
// process.
func (cmd *externalCommand) run(args []string, stdin string, callback func([]byte, error) error) {
if len(cmd.args) > 0 {
var allArgs []string
allArgs = append(allArgs, cmd.args...)
allArgs = append(allArgs, args...)
args = allArgs
}
exec := &cmdExecution{cmd.exe, args, stdin, cmd.combineOutput}
cmd.proc.run(&cmd.eg, exec, callback)
}
Expand Down
22 changes: 22 additions & 0 deletions process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,28 @@ func TestProcessRunConcurrently(t *testing.T) {
}
}

func TestProcessRunWithArgs(t *testing.T) {
var done atomic.Bool
p := newConcurrentProcess(1)
echo := testSkipIfNoCommand(t, p, "echo hello")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't fail when newCommandRunner fails because it calls t.Skip.

Instead, we need to check if echo exists separately.

if _, err := execabs.LookPath("echo"); err != nil {
    t.Skipf("echo command is necessary to run this test: %s", err)
}

c, err := p.newCommandRunner("echo hello", false)
if err != nil {
    t.Fatalf(`parsing "echo hello" failed: %v`, err)
}

echo.run(nil, "", func(b []byte, err error) error {
if err != nil {
t.Error(err)
return err
}
if string(b) != "hello\n" {
t.Errorf("unexpected output: %q", b)
}
done.Store(true)
return nil
})
p.wait()

if !done.Load() {
t.Error("callback did not run")
}
}

func TestProcessRunMultipleCommandsConcurrently(t *testing.T) {
p := newConcurrentProcess(3)

Expand Down