Skip to content

Commit

Permalink
fix: add batching to implicit commands (#695)
Browse files Browse the repository at this point in the history
* fix: add batching to implicit commands

* chore: move limits to a separate file

* chore: apply the similar struct-interface model for skip setting command executor as in git
  • Loading branch information
mrexox authored Apr 9, 2024
1 parent 3c17146 commit 0ea9433
Show file tree
Hide file tree
Showing 17 changed files with 244 additions and 198 deletions.
5 changes: 3 additions & 2 deletions internal/config/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/spf13/viper"

"github.com/evilmartians/lefthook/internal/git"
"github.com/evilmartians/lefthook/internal/system"
)

var errFilesIncompatible = errors.New("One of your runners contains incompatible file types")
Expand Down Expand Up @@ -44,8 +45,8 @@ func (c Command) Validate() error {
}

func (c Command) DoSkip(gitState git.State) bool {
skipChecker := NewSkipChecker(NewOsExec())
return skipChecker.Check(gitState, c.Skip, c.Only)
skipChecker := NewSkipChecker(system.Executor{})
return skipChecker.check(gitState, c.Skip, c.Only)
}

func (c Command) ExecutionPriority() int {
Expand Down
33 changes: 33 additions & 0 deletions internal/config/command_executor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package config

import (
"runtime"
)

// Executor is a general execution interface for implicit commands.
type Executor interface {
Execute(args []string, root string) (string, error)
}

// commandExecutor implements execution of a skip checks passed in a `run` option.
type commandExecutor struct {
exec Executor
}

// cmd runs plain string command in a subshell returning the success of it.
func (c *commandExecutor) cmd(commandLine string) bool {
if commandLine == "" {
return false
}

var args []string
if runtime.GOOS == "windows" {
args = []string{"powershell", "-Command", commandLine}
} else {
args = []string{"sh", "-c", commandLine}
}

_, err := c.exec.Execute(args, "")

return err == nil
}
35 changes: 0 additions & 35 deletions internal/config/exec.go

This file was deleted.

5 changes: 3 additions & 2 deletions internal/config/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/spf13/viper"

"github.com/evilmartians/lefthook/internal/git"
"github.com/evilmartians/lefthook/internal/system"
)

const CMD = "{cmd}"
Expand Down Expand Up @@ -43,8 +44,8 @@ func (h *Hook) Validate() error {
}

func (h *Hook) DoSkip(gitState git.State) bool {
skipChecker := NewSkipChecker(NewOsExec())
return skipChecker.Check(gitState, h.Skip, h.Only)
skipChecker := NewSkipChecker(system.Executor{})
return skipChecker.check(gitState, h.Skip, h.Only)
}

func unmarshalHooks(base, extra *viper.Viper) (*Hook, error) {
Expand Down
5 changes: 3 additions & 2 deletions internal/config/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/spf13/viper"

"github.com/evilmartians/lefthook/internal/git"
"github.com/evilmartians/lefthook/internal/system"
)

type Script struct {
Expand All @@ -29,8 +30,8 @@ type scriptRunnerReplace struct {
}

func (s Script) DoSkip(gitState git.State) bool {
skipChecker := NewSkipChecker(NewOsExec())
return skipChecker.Check(gitState, s.Skip, s.Only)
skipChecker := NewSkipChecker(system.Executor{})
return skipChecker.check(gitState, s.Skip, s.Only)
}

func (s Script) ExecutionPriority() int {
Expand Down
25 changes: 11 additions & 14 deletions internal/config/skip_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@ import (
"github.com/evilmartians/lefthook/internal/log"
)

type SkipChecker struct {
Executor Exec
type skipChecker struct {
exec *commandExecutor
}

func NewSkipChecker(executor Exec) *SkipChecker {
if executor == nil {
executor = NewOsExec()
}

return &SkipChecker{Executor: executor}
func NewSkipChecker(executor Executor) *skipChecker {
return &skipChecker{&commandExecutor{executor}}
}

func (sc *SkipChecker) Check(gitState git.State, skip interface{}, only interface{}) bool {
// check returns the result of applying a skip/only setting which can be a branch, git state, shell command, etc.
func (sc *skipChecker) check(gitState git.State, skip interface{}, only interface{}) bool {
if skip != nil {
if sc.matches(gitState, skip) {
return true
Expand All @@ -33,7 +30,7 @@ func (sc *SkipChecker) Check(gitState git.State, skip interface{}, only interfac
return false
}

func (sc *SkipChecker) matches(gitState git.State, value interface{}) bool {
func (sc *skipChecker) matches(gitState git.State, value interface{}) bool {
switch typedValue := value.(type) {
case bool:
return typedValue
Expand All @@ -45,7 +42,7 @@ func (sc *SkipChecker) matches(gitState git.State, value interface{}) bool {
return false
}

func (sc *SkipChecker) matchesSlices(gitState git.State, slice []interface{}) bool {
func (sc *skipChecker) matchesSlices(gitState git.State, slice []interface{}) bool {
for _, state := range slice {
switch typedState := state.(type) {
case string:
Expand All @@ -66,7 +63,7 @@ func (sc *SkipChecker) matchesSlices(gitState git.State, slice []interface{}) bo
return false
}

func (sc *SkipChecker) matchesRef(gitState git.State, typedState map[string]interface{}) bool {
func (sc *skipChecker) matchesRef(gitState git.State, typedState map[string]interface{}) bool {
ref, ok := typedState["ref"].(string)
if !ok {
return false
Expand All @@ -81,13 +78,13 @@ func (sc *SkipChecker) matchesRef(gitState git.State, typedState map[string]inte
return g.Match(gitState.Branch)
}

func (sc *SkipChecker) matchesCommands(typedState map[string]interface{}) bool {
func (sc *skipChecker) matchesCommands(typedState map[string]interface{}) bool {
commandLine, ok := typedState["run"].(string)
if !ok {
return false
}

result := sc.Executor.Cmd(commandLine)
result := sc.exec.cmd(commandLine)

log.Debugf("[lefthook] skip/only cmd: %s, result: %t", commandLine, result)

Expand Down
11 changes: 8 additions & 3 deletions internal/config/skip_checker_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
package config

import (
"errors"
"testing"

"github.com/evilmartians/lefthook/internal/git"
)

type mockExecutor struct{}

func (mc mockExecutor) Cmd(cmd string) bool {
return cmd == "success"
func (mc mockExecutor) Execute(args []string, _root string) (string, error) {
if len(args) == 3 && args[2] == "success" {
return "", nil
} else {
return "", errors.New("failure")
}
}

func TestDoSkip(t *testing.T) {
Expand Down Expand Up @@ -139,7 +144,7 @@ func TestDoSkip(t *testing.T) {
},
} {
t.Run(tt.name, func(t *testing.T) {
if skipChecker.Check(tt.state, tt.skip, tt.only) != tt.skipped {
if skipChecker.check(tt.state, tt.skip, tt.only) != tt.skipped {
t.Errorf("Expected: %v, Was %v", tt.skipped, !tt.skipped)
}
})
Expand Down
99 changes: 99 additions & 0 deletions internal/git/command_executor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package git

import (
"fmt"
"path/filepath"
"strings"

"github.com/evilmartians/lefthook/internal/system"
)

// Executor is a general execution interface for implicit commands.
// Added here mostly for mockable tests.
type Executor interface {
Execute(args []string, root string) (string, error)
}

// CommandExecutor provides some methods that take some effect on execution and/or result data.
type CommandExecutor struct {
exec Executor
root string
}

// NewExecutor returns an object that executes given commands in the OS.
func NewExecutor(exec Executor) *CommandExecutor {
return &CommandExecutor{exec: exec}
}

// Cmd runs plain string command. Trims spaces around output.
func (c CommandExecutor) Cmd(args []string) (string, error) {
out, err := c.exec.Execute(args, c.root)
if err != nil {
return "", err
}

return strings.TrimSpace(out), nil
}

// BatchedCmd runs the command with any number of appended arguments batched in chunks to match the OS limits.
func (c CommandExecutor) BatchedCmd(cmd []string, args []string) (string, error) {
maxlen := system.MaxCmdLen()
result := strings.Builder{}

argsBatched := batchByLength(args, maxlen-len(cmd))
for i, batch := range argsBatched {
out, err := c.Cmd(append(cmd, batch...))
if err != nil {
return "", fmt.Errorf("error in batch %d: %w", i, err)
}
result.WriteString(out)
result.WriteString("\n")
}

return result.String(), nil
}

// CmdLines runs plain string command, returns its output split by newline.
func (c CommandExecutor) CmdLines(cmd []string) ([]string, error) {
out, err := c.exec.Execute(cmd, c.root)
if err != nil {
return nil, err
}

return strings.Split(strings.TrimSpace(out), "\n"), nil
}

// CmdLines runs plain string command, returns its output split by newline.
func (c CommandExecutor) CmdLinesWithinFolder(cmd []string, folder string) ([]string, error) {
root := filepath.Join(c.root, folder)
out, err := c.exec.Execute(cmd, root)
if err != nil {
return nil, err
}

return strings.Split(strings.TrimSpace(out), "\n"), nil
}

func batchByLength(s []string, length int) [][]string {
batches := make([][]string, 0)

var acc, prev int
for i := range s {
acc += len(s[i])
if acc > length {
if i == prev {
batches = append(batches, s[prev:i+1])
prev = i + 1
} else {
batches = append(batches, s[prev:i])
prev = i
}
acc = len(s[i])
}
}
if acc > 0 {
batches = append(batches, s[prev:])
}

return batches
}
69 changes: 0 additions & 69 deletions internal/git/exec.go

This file was deleted.

Loading

0 comments on commit 0ea9433

Please sign in to comment.