Skip to content

Commit

Permalink
chore: refactor commands passing
Browse files Browse the repository at this point in the history
* Pass a slice of strings (commands), split the command inside the
  executor
  • Loading branch information
mrexox committed Sep 12, 2023
1 parent 22eaf62 commit 232a763
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 43 deletions.
8 changes: 4 additions & 4 deletions internal/lefthook/run/exec/execute_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ func (e CommandExecutor) Execute(opts Options, out io.Writer) error {

// We can have one command split into separate to fit into shell command max length.
// In this case we execute those commands one by one.
for _, args := range opts.Commands {
if err := e.executeOne(args, root, envs, opts.Interactive, in, out); err != nil {
for _, command := range opts.Commands {
if err := e.executeOne(command, root, envs, opts.Interactive, in, out); err != nil {
return err
}
}
Expand All @@ -60,8 +60,8 @@ func (e CommandExecutor) RawExecute(command []string, out io.Writer) error {
return cmd.Run()
}

func (e CommandExecutor) executeOne(args []string, root string, envs []string, interactive bool, in io.Reader, out io.Writer) error {
command := exec.Command("sh", "-c", strings.Join(args, " "))
func (e CommandExecutor) executeOne(cmdstr string, root string, envs []string, interactive bool, in io.Reader, out io.Writer) error {
command := exec.Command("sh", "-c", cmdstr)
command.Dir = root
command.Env = append(os.Environ(), envs...)

Expand Down
11 changes: 6 additions & 5 deletions internal/lefthook/run/exec/execute_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ func (e CommandExecutor) Execute(opts Options, out io.Writer) error {
)
}

for _, args := range opts.Commands {
if err := e.executeOne(args, root, envs, opts.Interactive, os.Stdin, out); err != nil {
for _, command := range opts.Commands {
if err := e.executeOne(command, root, envs, opts.Interactive, os.Stdin, out); err != nil {
return err
}
}
Expand All @@ -40,10 +40,11 @@ func (e CommandExecutor) RawExecute(command []string, out io.Writer) error {
return cmd.Run()
}

func (e CommandExecutor) executeOne(args []string, root string, envs []string, interactive bool, in io.Reader, out io.Writer) error {
command := exec.Command(args[0])
func (e CommandExecutor) executeOne(cmdstr string, root string, envs []string, interactive bool, in io.Reader, out io.Writer) error {
cmdargs := strings.Split(cmdstr, " ")
command := exec.Command(cmdargs[0])
command.SysProcAttr = &syscall.SysProcAttr{
CmdLine: strings.Join(args, " "),
CmdLine: strings.Join(cmdargs, " "),
}
command.Dir = root
command.Env = append(os.Environ(), envs...)
Expand Down
2 changes: 1 addition & 1 deletion internal/lefthook/run/exec/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
// Options contains the data that controls the execution.
type Options struct {
Name, Root, FailText string
Commands [][]string
Commands []string
Env map[string]string
Interactive bool
}
Expand Down
8 changes: 4 additions & 4 deletions internal/lefthook/run/prepare_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

// An object that describes the single command's run option.
type run struct {
commands [][]string
commands []string
files []string
}

Expand Down Expand Up @@ -200,7 +200,7 @@ func escapeFiles(files []string) []string {
func replaceInChunks(str string, templates map[string]*template, maxlen int) *run {
if len(templates) == 0 {
return &run{
commands: [][]string{strings.Split(str, " ")},
commands: []string{str},
}
}

Expand All @@ -225,7 +225,7 @@ func replaceInChunks(str string, templates map[string]*template, maxlen int) *ru
}

var exhausted int
commands := make([][]string, 0)
commands := make([]string, 0)
for {
command := str
for name, template := range templates {
Expand All @@ -239,7 +239,7 @@ func replaceInChunks(str string, templates map[string]*template, maxlen int) *ru
}

log.Debug("[lefthook] executing: ", command)
commands = append(commands, strings.Split(command, " "))
commands = append(commands, command)
if exhausted >= len(templates) {
break
}
Expand Down
36 changes: 18 additions & 18 deletions internal/lefthook/run/prepare_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func TestReplaceInChunks(t *testing.T) {
},
maxlen: 300,
res: &run{
commands: [][]string{{"echo", "file1", "file2", "file3"}},
commands: []string{"echo file1 file2 file3"},
files: []string{"file1", "file2", "file3"},
},
},
Expand All @@ -117,10 +117,10 @@ func TestReplaceInChunks(t *testing.T) {
},
maxlen: 10,
res: &run{
commands: [][]string{
{"echo", "file1"},
{"echo", "file2"},
{"echo", "file3"},
commands: []string{
"echo file1",
"echo file2",
"echo file3",
},
files: []string{"file1", "file2", "file3"},
},
Expand All @@ -135,9 +135,9 @@ func TestReplaceInChunks(t *testing.T) {
},
maxlen: 49, // (49 - 17(len of command without templates)) / 2 = 16, but we need 17 (3 words + 2 spaces)
res: &run{
commands: [][]string{
{"echo", "file1", "file2", "&&", "git", "add", "file1", "file2"},
{"echo", "file3", "&&", "git", "add", "file3"},
commands: []string{
"echo file1 file2 && git add file1 file2",
"echo file3 && git add file3",
},
files: []string{"file1", "file2", "file3"},
},
Expand All @@ -152,8 +152,8 @@ func TestReplaceInChunks(t *testing.T) {
},
maxlen: 51,
res: &run{
commands: [][]string{
{"echo", "file1", "file2", "file3", "&&", "git", "add", "file1", "file2", "file3"},
commands: []string{
"echo file1 file2 file3 && git add file1 file2 file3",
},
files: []string{"file1", "file2", "file3"},
},
Expand All @@ -172,9 +172,9 @@ func TestReplaceInChunks(t *testing.T) {
},
maxlen: 10,
res: &run{
commands: [][]string{
{"echo", "push-file", "&&", "git", "add", "file1"},
{"echo", "push-file", "&&", "git", "add", "file2"},
commands: []string{
"echo push-file && git add file1",
"echo push-file && git add file2",
},
files: []string{"push-file", "file1", "file2"},
},
Expand All @@ -193,10 +193,10 @@ func TestReplaceInChunks(t *testing.T) {
},
maxlen: 27,
res: &run{
commands: [][]string{
{"echo", "push1", "&&", "git", "add", "file1"},
{"echo", "push2", "&&", "git", "add", "file2"},
{"echo", "push3", "&&", "git", "add", "file2"},
commands: []string{
"echo push1 && git add file1",
"echo push2 && git add file2",
"echo push3 && git add file2",
},
files: []string{"push1", "push2", "push3", "file1", "file2"},
},
Expand All @@ -212,7 +212,7 @@ func TestReplaceInChunks(t *testing.T) {
t.Errorf("expected commands to be %d instead of %d", len(tt.res.commands), len(res.commands))
} else {
for i, command := range res.commands {
if !slicesEqual(command, tt.res.commands[i]) {
if command != tt.res.commands[i] {
t.Errorf("expected command %v to be equal to %v", command, tt.res.commands[i])
}
}
Expand Down
14 changes: 7 additions & 7 deletions internal/lefthook/run/prepare_script.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,37 +9,37 @@ import (
"github.com/evilmartians/lefthook/internal/log"
)

func (r *Runner) prepareScript(script *config.Script, path string, file os.FileInfo) ([]string, error) {
func (r *Runner) prepareScript(script *config.Script, path string, file os.FileInfo) (string, error) {
if script.DoSkip(r.Repo.State()) {
return nil, errors.New("settings")
return "", errors.New("settings")
}

if intersect(r.Hook.ExcludeTags, script.Tags) {
return nil, errors.New("excluded tags")
return "", errors.New("excluded tags")
}

// Skip non-regular files (dirs, symlinks, sockets, etc.)
if !file.Mode().IsRegular() {
log.Debugf("[lefthook] file %s is not a regular file, skipping", file.Name())
return nil, errors.New("not a regular file")
return "", errors.New("not a regular file")
}

// Make sure file is executable
if (file.Mode() & executableMask) == 0 {
if err := r.Repo.Fs.Chmod(path, executableFileMode); err != nil {
log.Errorf("Couldn't change file mode to make file executable: %s", err)
r.fail(file.Name(), nil)
return nil, errors.New("system error")
return "", errors.New("system error")
}
}

var args []string
if len(script.Runner) > 0 {
args = strings.Split(script.Runner, " ")
args = append(args, script.Runner)
}

args = append(args, path)
args = append(args, r.GitArgs...)

return args, nil
return strings.Join(args, " "), nil
}
4 changes: 2 additions & 2 deletions internal/lefthook/run/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func (r *Runner) runScripts(dir string) {
}

func (r *Runner) runScript(script *config.Script, path string, file os.FileInfo) {
args, err := r.prepareScript(script, path, file)
command, err := r.prepareScript(script, path, file)
if err != nil {
r.logSkip(file.Name(), err.Error())
return
Expand All @@ -292,7 +292,7 @@ func (r *Runner) runScript(script *config.Script, path string, file os.FileInfo)
finished := r.run(exec.Options{
Name: file.Name(),
Root: r.Repo.RootPath,
Commands: [][]string{args},
Commands: []string{command},
FailText: script.FailText,
Interactive: script.Interactive && !r.DisableTTY,
Env: script.Env,
Expand Down
4 changes: 2 additions & 2 deletions internal/lefthook/run/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import (
type TestExecutor struct{}

func (e TestExecutor) Execute(opts exec.Options, _out io.Writer) (err error) {
if opts.Commands[0][0] == "success" {
if strings.HasPrefix(opts.Commands[0], "success") {
err = nil
} else {
err = errors.New(opts.Commands[0][0])
err = errors.New(opts.Commands[0])
}

return
Expand Down

0 comments on commit 232a763

Please sign in to comment.