From 78069b28402adc2008d0303b1e985d541148773c Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Mon, 4 Sep 2023 15:19:52 +0300 Subject: [PATCH] fix: split command with file templates into chunks (#541) * set the maximum line length for Windows, macOS and Linux * split the command if it uses file templates in run string * execute the split command sequentially --- .gitignore | 3 - .lefthook.toml | 15 + Makefile | 2 +- docs/usage.md | 2 +- internal/lefthook/run.go | 21 +- .../{runner => run/exec}/execute_unix.go | 64 ++-- internal/lefthook/run/exec/execute_windows.go | 67 ++++ internal/lefthook/run/exec/executor.go | 20 ++ .../{runner => run/filter}/filters.go | 27 +- .../{runner => run/filter}/filters_test.go | 24 +- internal/lefthook/run/prepare_command.go | 303 ++++++++++++++++++ internal/lefthook/run/prepare_command_test.go | 222 +++++++++++++ .../{runner => run}/prepare_script.go | 6 +- internal/lefthook/{runner => run}/result.go | 2 +- internal/lefthook/{runner => run}/runner.go | 81 ++--- .../lefthook/{runner => run}/runner_test.go | 12 +- internal/lefthook/runner/execute_windows.go | 59 ---- internal/lefthook/runner/executor.go | 20 -- internal/lefthook/runner/prepare_command.go | 194 ----------- 19 files changed, 765 insertions(+), 379 deletions(-) create mode 100644 .lefthook.toml rename internal/lefthook/{runner => run/exec}/execute_unix.go (51%) create mode 100644 internal/lefthook/run/exec/execute_windows.go create mode 100644 internal/lefthook/run/exec/executor.go rename internal/lefthook/{runner => run/filter}/filters.go (52%) rename internal/lefthook/{runner => run/filter}/filters_test.go (88%) create mode 100644 internal/lefthook/run/prepare_command.go create mode 100644 internal/lefthook/run/prepare_command_test.go rename internal/lefthook/{runner => run}/prepare_script.go (90%) rename internal/lefthook/{runner => run}/result.go (95%) rename internal/lefthook/{runner => run}/runner.go (85%) rename internal/lefthook/{runner => run}/runner_test.go (98%) delete mode 100644 internal/lefthook/runner/execute_windows.go delete mode 100644 internal/lefthook/runner/executor.go delete mode 100644 internal/lefthook/runner/prepare_command.go diff --git a/.gitignore b/.gitignore index bfd1d16a..3fb90110 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,6 @@ .vscode/ .idea/ -.lefthook-local/ -.lefthook/ /lefthook-local.yml -/lefthook.yml /lefthook tmp/ diff --git a/.lefthook.toml b/.lefthook.toml new file mode 100644 index 00000000..f0194a6f --- /dev/null +++ b/.lefthook.toml @@ -0,0 +1,15 @@ +[pre-commit] +parallel = true + +[pre-commit.commands.lint] +run = "make lint" +glob = "*.go" +stage_fixed = true + +[pre-commit.commands.test] +run = "make test" +glob = "*.go" + +[pre-commit.commands.lychee] +glob = "*.md" +run = "lychee {staged_files}" diff --git a/Makefile b/Makefile index 6c8fc21e..cd044ada 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ bin/golangci-lint: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$(go env GOPATH)/bin v1.54.1 lint: bin/golangci-lint - $$(go env GOPATH)/bin/golangci-lint run + $$(go env GOPATH)/bin/golangci-lint run --fix .ONESHELL: version: diff --git a/docs/usage.md b/docs/usage.md index 7e3ec6d9..ec251d51 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -219,7 +219,7 @@ When you try to commit `git commit -m "haha bad commit text"` script `commitlint ### Parallel execution You can enable parallel execution if you want to speed up your checks. -Lets get example from [discourse](https://github.com/discourse/discourse/blob/master/.travis.yml#L77-L83) project. +Lets imagine we have the following rules to lint the whole project: ``` bundle exec rubocop --parallel && \ diff --git a/internal/lefthook/run.go b/internal/lefthook/run.go index e2866496..bd3770c1 100644 --- a/internal/lefthook/run.go +++ b/internal/lefthook/run.go @@ -9,7 +9,7 @@ import ( "time" "github.com/evilmartians/lefthook/internal/config" - "github.com/evilmartians/lefthook/internal/lefthook/runner" + "github.com/evilmartians/lefthook/internal/lefthook/run" "github.com/evilmartians/lefthook/internal/log" "github.com/evilmartians/lefthook/internal/version" ) @@ -105,11 +105,10 @@ Run 'lefthook install' manually.`, } startTime := time.Now() - resultChan := make(chan runner.Result, len(hook.Commands)+len(hook.Scripts)) + resultChan := make(chan run.Result, len(hook.Commands)+len(hook.Scripts)) - run := runner.NewRunner( - runner.Opts{ - Fs: l.Fs, + runner := run.NewRunner( + run.Options{ Repo: l.repo, Hook: hook, HookName: hookName, @@ -140,11 +139,11 @@ Run 'lefthook install' manually.`, } go func() { - run.RunAll(sourceDirs) + runner.RunAll(sourceDirs) close(resultChan) }() - var results []runner.Result + var results []run.Result for res := range resultChan { results = append(results, res) } @@ -154,7 +153,7 @@ Run 'lefthook install' manually.`, } for _, result := range results { - if result.Status == runner.StatusErr { + if result.Status == run.StatusErr { return errors.New("") // No error should be printed } } @@ -164,7 +163,7 @@ Run 'lefthook install' manually.`, func printSummary( duration time.Duration, - results []runner.Result, + results []run.Result, logSettings log.SkipSettings, ) { if len(results) == 0 { @@ -180,7 +179,7 @@ func printSummary( if !logSettings.SkipSuccess() { for _, result := range results { - if result.Status != runner.StatusOk { + if result.Status != run.StatusOk { continue } @@ -190,7 +189,7 @@ func printSummary( if !logSettings.SkipFailure() { for _, result := range results { - if result.Status != runner.StatusErr { + if result.Status != run.StatusErr { continue } diff --git a/internal/lefthook/runner/execute_unix.go b/internal/lefthook/run/exec/execute_unix.go similarity index 51% rename from internal/lefthook/runner/execute_unix.go rename to internal/lefthook/run/exec/execute_unix.go index e74594e3..630f5a41 100644 --- a/internal/lefthook/runner/execute_unix.go +++ b/internal/lefthook/run/exec/execute_unix.go @@ -1,7 +1,7 @@ //go:build !windows // +build !windows -package runner +package exec import ( "fmt" @@ -19,36 +19,55 @@ import ( type CommandExecutor struct{} -func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error { - stdin := os.Stdin - if opts.interactive && !isatty.IsTerminal(os.Stdin.Fd()) { +func (e CommandExecutor) Execute(opts Options, out io.Writer) error { + in := os.Stdin + if opts.Interactive && !isatty.IsTerminal(os.Stdin.Fd()) { tty, err := os.Open("/dev/tty") if err == nil { defer tty.Close() - stdin = tty + in = tty } else { log.Errorf("Couldn't enable TTY input: %s\n", err) } } - command := exec.Command("sh", "-c", strings.Join(opts.args, " ")) - - rootDir, _ := filepath.Abs(opts.root) - command.Dir = rootDir - - envList := make([]string, len(opts.env)) - for name, value := range opts.env { - envList = append( - envList, + root, _ := filepath.Abs(opts.Root) + envs := make([]string, len(opts.Env)) + for name, value := range opts.Env { + envs = append( + envs, fmt.Sprintf("%s=%s", strings.ToUpper(name), os.ExpandEnv(value)), ) } - command.Env = append(os.Environ(), envList...) + // 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 { + return err + } + } + + return nil +} + +func (e CommandExecutor) RawExecute(command []string, out io.Writer) error { + cmd := exec.Command(command[0], command[1:]...) + + cmd.Stdout = out + cmd.Stderr = os.Stderr + + 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, " ")) + command.Dir = root + command.Env = append(os.Environ(), envs...) - if opts.interactive { + if interactive { command.Stdout = out - command.Stdin = stdin + command.Stdin = in command.Stderr = os.Stderr err := command.Start() if err != nil { @@ -62,7 +81,7 @@ func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error { defer func() { _ = p.Close() }() - go func() { _, _ = io.Copy(p, stdin) }() + go func() { _, _ = io.Copy(p, in) }() _, _ = io.Copy(out, p) } @@ -71,12 +90,3 @@ func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error { return command.Wait() } - -func (e CommandExecutor) RawExecute(command []string, out io.Writer) error { - cmd := exec.Command(command[0], command[1:]...) - - cmd.Stdout = out - cmd.Stderr = os.Stderr - - return cmd.Run() -} diff --git a/internal/lefthook/run/exec/execute_windows.go b/internal/lefthook/run/exec/execute_windows.go new file mode 100644 index 00000000..b13e930f --- /dev/null +++ b/internal/lefthook/run/exec/execute_windows.go @@ -0,0 +1,67 @@ +package exec + +import ( + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" +) + +type CommandExecutor struct{} + +func (e CommandExecutor) Execute(opts Options, out io.Writer) error { + root, _ := filepath.Abs(opts.Root) + envs := make([]string, len(opts.Env)) + for name, value := range opts.Env { + envs = append( + envs, + fmt.Sprintf("%s=%s", strings.ToUpper(name), os.ExpandEnv(value)), + ) + } + + for _, args := range opts.Commands { + if err := e.executeOne(args, root, envs, opts.Interactive, os.Stdin, out); err != nil { + return err + } + } + + return nil +} + +func (e CommandExecutor) RawExecute(command []string, out io.Writer) error { + cmd := exec.Command(command[0], command[1:]...) + + cmd.Stdout = out + cmd.Stderr = os.Stderr + + 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]) + command.SysProcAttr = &syscall.SysProcAttr{ + CmdLine: strings.Join(args, " "), + } + command.Dir = root + command.Env = append(os.Environ(), envs...) + + if interactive { + command.Stdout = os.Stdout + } else { + command.Stdout = out + } + + command.Stdin = in + command.Stderr = os.Stderr + err := command.Start() + if err != nil { + return err + } + + defer func() { _ = command.Process.Kill() }() + + return command.Wait() +} diff --git a/internal/lefthook/run/exec/executor.go b/internal/lefthook/run/exec/executor.go new file mode 100644 index 00000000..6b326351 --- /dev/null +++ b/internal/lefthook/run/exec/executor.go @@ -0,0 +1,20 @@ +package exec + +import ( + "io" +) + +// Options contains the data that controls the execution. +type Options struct { + Name, Root, FailText string + Commands [][]string + Env map[string]string + Interactive bool +} + +// Executor provides an interface for command execution. +// It is used here for testing purpose mostly. +type Executor interface { + Execute(opts Options, out io.Writer) error + RawExecute(command []string, out io.Writer) error +} diff --git a/internal/lefthook/runner/filters.go b/internal/lefthook/run/filter/filters.go similarity index 52% rename from internal/lefthook/runner/filters.go rename to internal/lefthook/run/filter/filters.go index 1efbd431..00af16fc 100644 --- a/internal/lefthook/runner/filters.go +++ b/internal/lefthook/run/filter/filters.go @@ -1,13 +1,32 @@ -package runner +package filter import ( "regexp" "strings" "github.com/gobwas/glob" + + "github.com/evilmartians/lefthook/internal/config" + "github.com/evilmartians/lefthook/internal/log" ) -func filterGlob(vs []string, matcher string) []string { +func Apply(command *config.Command, files []string) []string { + if len(files) == 0 { + return nil + } + + log.Debug("[lefthook] files before filters:\n", files) + + files = byGlob(files, command.Glob) + files = byExclude(files, command.Exclude) + files = byRoot(files, command.Root) + + log.Debug("[lefthook] files after filters:\n", files) + + return files +} + +func byGlob(vs []string, matcher string) []string { if matcher == "" { return vs } @@ -23,7 +42,7 @@ func filterGlob(vs []string, matcher string) []string { return vsf } -func filterExclude(vs []string, matcher string) []string { +func byExclude(vs []string, matcher string) []string { if matcher == "" { return vs } @@ -37,7 +56,7 @@ func filterExclude(vs []string, matcher string) []string { return vsf } -func filterRelative(vs []string, matcher string) []string { +func byRoot(vs []string, matcher string) []string { if matcher == "" { return vs } diff --git a/internal/lefthook/runner/filters_test.go b/internal/lefthook/run/filter/filters_test.go similarity index 88% rename from internal/lefthook/runner/filters_test.go rename to internal/lefthook/run/filter/filters_test.go index dda1a589..996525ed 100644 --- a/internal/lefthook/runner/filters_test.go +++ b/internal/lefthook/run/filter/filters_test.go @@ -1,4 +1,4 @@ -package runner +package filter import ( "fmt" @@ -10,8 +10,14 @@ func slicesEqual(a, b []string) bool { return false } - for i, item := range a { - if item != b[i] { + r := make(map[string]struct{}) + + for _, item := range a { + r[item] = struct{}{} + } + + for _, item := range b { + if _, ok := r[item]; !ok { return false } } @@ -19,7 +25,7 @@ func slicesEqual(a, b []string) bool { return true } -func TestFilterGlob(t *testing.T) { +func TestByGlob(t *testing.T) { for i, tt := range [...]struct { source, result []string glob string @@ -51,7 +57,7 @@ func TestFilterGlob(t *testing.T) { }, } { t.Run(fmt.Sprintf("%d:", i), func(t *testing.T) { - res := filterGlob(tt.source, tt.glob) + res := byGlob(tt.source, tt.glob) if !slicesEqual(res, tt.result) { t.Errorf("expected %v to be equal to %v", res, tt.result) } @@ -59,7 +65,7 @@ func TestFilterGlob(t *testing.T) { } } -func TestFilterExclude(t *testing.T) { +func TestByExclude(t *testing.T) { for i, tt := range [...]struct { source, result []string exclude string @@ -91,7 +97,7 @@ func TestFilterExclude(t *testing.T) { }, } { t.Run(fmt.Sprintf("%d:", i), func(t *testing.T) { - res := filterExclude(tt.source, tt.exclude) + res := byExclude(tt.source, tt.exclude) if !slicesEqual(res, tt.result) { t.Errorf("expected %v to be equal to %v", res, tt.result) } @@ -99,7 +105,7 @@ func TestFilterExclude(t *testing.T) { } } -func TestFilterRelative(t *testing.T) { +func TestByRoot(t *testing.T) { for i, tt := range [...]struct { source, result []string path string @@ -126,7 +132,7 @@ func TestFilterRelative(t *testing.T) { }, } { t.Run(fmt.Sprintf("%d:", i), func(t *testing.T) { - res := filterRelative(tt.source, tt.path) + res := byRoot(tt.source, tt.path) if !slicesEqual(res, tt.result) { t.Errorf("expected %v to be equal to %v", res, tt.result) } diff --git a/internal/lefthook/run/prepare_command.go b/internal/lefthook/run/prepare_command.go new file mode 100644 index 00000000..63533017 --- /dev/null +++ b/internal/lefthook/run/prepare_command.go @@ -0,0 +1,303 @@ +package run + +import ( + "errors" + "fmt" + "runtime" + "strings" + + "gopkg.in/alessio/shellescape.v1" + + "github.com/evilmartians/lefthook/internal/config" + "github.com/evilmartians/lefthook/internal/lefthook/run/filter" + "github.com/evilmartians/lefthook/internal/log" +) + +// An object that describes the single command's run option. +type run struct { + commands [][]string + files []string +} + +// Stats for template replacements in a command string. +type template struct { + files []string + cnt int +} + +const ( + // https://serverfault.com/questions/69430/what-is-the-maximum-length-of-a-command-line-in-mac-os-x + // https://support.microsoft.com/en-us/help/830473/command-prompt-cmd-exe-command-line-string-limitation + // https://unix.stackexchange.com/a/120652 + maxCommandLengthDarwin = 260000 // 262144 + maxCommandLengthWindows = 8000 // 8191 + maxCommandLengthLinux = 130000 // 131072 +) + +func (r *Runner) prepareCommand(name string, command *config.Command) (*run, error) { + if command.DoSkip(r.Repo.State()) { + return nil, errors.New("settings") + } + + if intersect(r.Hook.ExcludeTags, command.Tags) { + return nil, errors.New("tags") + } + + if intersect(r.Hook.ExcludeTags, []string{name}) { + return nil, errors.New("name") + } + + if err := command.Validate(); err != nil { + r.fail(name, err) + return nil, err + } + + args, err, skipReason := r.buildRun(command) + if err != nil { + log.Error(err) + return nil, errors.New("error") + } + if skipReason != nil { + return nil, skipReason + } + + return args, nil +} + +func (r *Runner) buildRun(command *config.Command) (*run, error, error) { + filesCmd := r.Hook.Files + if len(command.Files) > 0 { + filesCmd = command.Files + } + if len(filesCmd) > 0 { + filesCmd = replacePositionalArguments(filesCmd, r.GitArgs) + } + + var stagedFiles func() ([]string, error) + switch { + case len(r.Files) > 0: + stagedFiles = func() ([]string, error) { return r.Files, nil } + case r.AllFiles: + stagedFiles = r.Repo.AllFiles + default: + stagedFiles = r.Repo.StagedFiles + } + + filesFns := map[string]func() ([]string, error){ + config.SubStagedFiles: stagedFiles, + config.PushFiles: r.Repo.PushFiles, + config.SubAllFiles: r.Repo.AllFiles, + config.SubFiles: func() ([]string, error) { + return r.Repo.FilesByCommand(filesCmd) + }, + } + + templates := make(map[string]*template) + + for filesType, fn := range filesFns { + cnt := strings.Count(command.Run, filesType) + if cnt == 0 { + continue + } + + templ := &template{cnt: cnt} + templates[filesType] = templ + + files, err := fn() + if err != nil { + return nil, fmt.Errorf("error replacing %s: %w", filesType, err), nil + } + + files = filter.Apply(command, files) + if len(files) == 0 { + return nil, nil, errors.New("no files for inspection") + } + + templ.files = files + } + + // Checking substitutions and skipping execution if it is empty. + // + // Special case for `files` option: return if the result of files command is empty. + if len(filesCmd) > 0 && templates[config.SubFiles] == nil { + files, err := filesFns[config.SubFiles]() + if err != nil { + return nil, fmt.Errorf("error calling replace command for %s: %w", config.SubFiles, err), nil + } + + files = filter.Apply(command, files) + + if len(files) == 0 { + return nil, nil, errors.New("no files for inspection") + } + } + + runString := command.Run + runString = replacePositionalArguments(runString, r.GitArgs) + + var maxlen int + switch runtime.GOOS { + case "windows": + maxlen = maxCommandLengthWindows + case "darwin": + maxlen = maxCommandLengthDarwin + default: + maxlen = maxCommandLengthLinux + } + result := replaceInChunks(runString, templates, maxlen) + + if len(result.files) == 0 && config.HookUsesStagedFiles(r.HookName) { + if templates[config.SubStagedFiles] != nil && len(templates[config.SubStagedFiles].files) == 0 { + return nil, nil, errors.New("no matching staged files") + } + + files, err := r.Repo.StagedFiles() + if err == nil { + if len(filter.Apply(command, files)) == 0 { + return nil, nil, errors.New("no matching staged files") + } + } + } + + if len(result.files) == 0 && config.HookUsesPushFiles(r.HookName) { + if templates[config.PushFiles] != nil && len(templates[config.PushFiles].files) == 0 { + return nil, nil, errors.New("no matching push files") + } + + files, err := r.Repo.PushFiles() + if err == nil { + if len(filter.Apply(command, files)) == 0 { + return nil, nil, errors.New("no matching push files") + } + } + } + + return result, nil, nil +} + +func replacePositionalArguments(str string, args []string) string { + str = strings.ReplaceAll(str, "{0}", strings.Join(args, " ")) + for i, arg := range args { + str = strings.ReplaceAll(str, fmt.Sprintf("{%d}", i+1), arg) + } + return str +} + +// Escape file names to prevent unexpected bugs. +func escapeFiles(files []string) []string { + var filesEsc []string + for _, fileName := range files { + if len(fileName) > 0 { + filesEsc = append(filesEsc, shellescape.Quote(fileName)) + } + } + + log.Debug("[lefthook] files after escaping:\n", filesEsc) + + return filesEsc +} + +func replaceInChunks(str string, templates map[string]*template, maxlen int) *run { + if len(templates) == 0 { + return &run{ + commands: [][]string{strings.Split(str, " ")}, + } + } + + var cnt int + + allFiles := make([]string, 0) + for name, template := range templates { + if template.cnt == 0 { + continue + } + + cnt += template.cnt + maxlen += template.cnt * len(name) + allFiles = append(allFiles, template.files...) + template.files = escapeFiles(template.files) + } + + maxlen -= len(str) + + if cnt > 0 { + maxlen /= cnt + } + + var exhausted int + commands := make([][]string, 0) + for { + command := str + for name, template := range templates { + added, rest := getNChars(template.files, maxlen) + if len(rest) == 0 { + exhausted += 1 + } else { + template.files = rest + } + command = replaceQuoted(command, name, added) + } + + log.Debug("[lefthook] executing: ", command) + commands = append(commands, strings.Split(command, " ")) + if exhausted >= len(templates) { + break + } + } + + return &run{ + commands: commands, + files: allFiles, + } +} + +func getNChars(s []string, n int) ([]string, []string) { + if len(s) == 0 { + return nil, nil + } + + var cnt int + for i, str := range s { + cnt += len(str) + if i > 0 { + cnt += 1 // a space + } + if cnt > n { + if i == 0 { + i = 1 + } + return s[:i], s[i:] + } + } + + return s, nil +} + +func replaceQuoted(source, substitution string, files []string) string { + for _, elem := range [][]string{ + {"\"", "\"" + substitution + "\""}, + {"'", "'" + substitution + "'"}, + {"", substitution}, + } { + quote := elem[0] + sub := elem[1] + if !strings.Contains(source, sub) { + continue + } + + quotedFiles := files + if len(quote) != 0 { + quotedFiles = make([]string, 0, len(files)) + for _, fileName := range files { + quotedFiles = append(quotedFiles, + quote+surroundingQuotesRegexp.ReplaceAllString(fileName, "$1")+quote) + } + } + + source = strings.ReplaceAll( + source, sub, strings.Join(quotedFiles, " "), + ) + } + + return source +} diff --git a/internal/lefthook/run/prepare_command_test.go b/internal/lefthook/run/prepare_command_test.go new file mode 100644 index 00000000..884b32bd --- /dev/null +++ b/internal/lefthook/run/prepare_command_test.go @@ -0,0 +1,222 @@ +package run + +import ( + "fmt" + "testing" +) + +func slicesEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + + r := make(map[string]struct{}) + + for _, item := range a { + r[item] = struct{}{} + } + + for _, item := range b { + if _, ok := r[item]; !ok { + return false + } + } + + return true +} + +func TestGetNChars(t *testing.T) { + for i, tt := range [...]struct { + source, cut, rest []string + n int + }{ + { + source: []string{"str1", "str2", "str3"}, + n: 0, + cut: []string{"str1"}, + rest: []string{"str2", "str3"}, + }, + { + source: []string{"str1", "str2", "str3"}, + n: 4, + cut: []string{"str1"}, + rest: []string{"str2", "str3"}, + }, + { + source: []string{"str1", "str2", "str3"}, + n: 6, + cut: []string{"str1"}, + rest: []string{"str2", "str3"}, + }, + { + source: []string{"str1", "str2", "str3"}, + n: 8, + cut: []string{"str1"}, // because we need to add a space + rest: []string{"str2", "str3"}, + }, + { + source: []string{"str1", "str2", "str3"}, + n: 9, + cut: []string{"str1", "str2"}, + rest: []string{"str3"}, + }, + { + source: []string{"str1", "str2", "str3"}, + n: 500, + cut: []string{"str1", "str2", "str3"}, + rest: nil, + }, + { + source: nil, + n: 2, + cut: nil, + rest: nil, + }, + } { + t.Run(fmt.Sprintf("test %d", i), func(t *testing.T) { + cut, rest := getNChars(tt.source, tt.n) + + if !slicesEqual(cut, tt.cut) { + t.Errorf("expected cut %v to be equal to %v", cut, tt.cut) + } + if !slicesEqual(rest, tt.rest) { + t.Errorf("expected rest %v to be equal to %v", rest, tt.rest) + } + }) + } +} + +func TestReplaceInChunks(t *testing.T) { + for i, tt := range [...]struct { + str string + templates map[string]*template + maxlen int + res *run + }{ + { + str: "echo {staged_files}", + templates: map[string]*template{ + "{staged_files}": { + files: []string{"file1", "file2", "file3"}, + cnt: 1, + }, + }, + maxlen: 300, + res: &run{ + commands: [][]string{{"echo", "file1", "file2", "file3"}}, + files: []string{"file1", "file2", "file3"}, + }, + }, + { + str: "echo {staged_files}", + templates: map[string]*template{ + "{staged_files}": { + files: []string{"file1", "file2", "file3"}, + cnt: 1, + }, + }, + maxlen: 10, + res: &run{ + commands: [][]string{ + {"echo", "file1"}, + {"echo", "file2"}, + {"echo", "file3"}, + }, + files: []string{"file1", "file2", "file3"}, + }, + }, + { + str: "echo {files} && git add {files}", + templates: map[string]*template{ + "{files}": { + files: []string{"file1", "file2", "file3"}, + cnt: 2, + }, + }, + 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"}, + }, + files: []string{"file1", "file2", "file3"}, + }, + }, + { + str: "echo {files} && git add {files}", + templates: map[string]*template{ + "{files}": { + files: []string{"file1", "file2", "file3"}, + cnt: 2, + }, + }, + maxlen: 51, + res: &run{ + commands: [][]string{ + {"echo", "file1", "file2", "file3", "&&", "git", "add", "file1", "file2", "file3"}, + }, + files: []string{"file1", "file2", "file3"}, + }, + }, + { + str: "echo {push_files} && git add {files}", + templates: map[string]*template{ + "{push_files}": { + files: []string{"push-file"}, + cnt: 1, + }, + "{files}": { + files: []string{"file1", "file2"}, + cnt: 1, + }, + }, + maxlen: 10, + res: &run{ + commands: [][]string{ + {"echo", "push-file", "&&", "git", "add", "file1"}, + {"echo", "push-file", "&&", "git", "add", "file2"}, + }, + files: []string{"push-file", "file1", "file2"}, + }, + }, + { + str: "echo {push_files} && git add {files}", + templates: map[string]*template{ + "{push_files}": { + files: []string{"push1", "push2", "push3"}, + cnt: 1, + }, + "{files}": { + files: []string{"file1", "file2"}, + cnt: 1, + }, + }, + maxlen: 27, + res: &run{ + commands: [][]string{ + {"echo", "push1", "&&", "git", "add", "file1"}, + {"echo", "push2", "&&", "git", "add", "file2"}, + {"echo", "push3", "&&", "git", "add", "file2"}, + }, + files: []string{"push1", "push2", "push3", "file1", "file2"}, + }, + }, + } { + t.Run(fmt.Sprintf("test %d", i), func(t *testing.T) { + res := replaceInChunks(tt.str, tt.templates, tt.maxlen) + if !slicesEqual(res.files, tt.res.files) { + t.Errorf("expected files %v to be equal to %v", res.files, tt.res.files) + } + + if len(res.commands) != len(tt.res.commands) { + 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]) { + t.Errorf("expected command %v to be equal to %v", command, tt.res.commands[i]) + } + } + } + }) + } +} diff --git a/internal/lefthook/runner/prepare_script.go b/internal/lefthook/run/prepare_script.go similarity index 90% rename from internal/lefthook/runner/prepare_script.go rename to internal/lefthook/run/prepare_script.go index c275e14e..76613c81 100644 --- a/internal/lefthook/runner/prepare_script.go +++ b/internal/lefthook/run/prepare_script.go @@ -1,4 +1,4 @@ -package runner +package run import ( "errors" @@ -26,9 +26,9 @@ func (r *Runner) prepareScript(script *config.Script, path string, file os.FileI // Make sure file is executable if (file.Mode() & executableMask) == 0 { - if err := r.Fs.Chmod(path, executableFileMode); err != nil { + 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(), "") + r.fail(file.Name(), nil) return nil, errors.New("system error") } } diff --git a/internal/lefthook/runner/result.go b/internal/lefthook/run/result.go similarity index 95% rename from internal/lefthook/runner/result.go rename to internal/lefthook/run/result.go index c2b04425..f245c94d 100644 --- a/internal/lefthook/runner/result.go +++ b/internal/lefthook/run/result.go @@ -1,4 +1,4 @@ -package runner +package run const ( StatusOk status = iota diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/run/runner.go similarity index 85% rename from internal/lefthook/runner/runner.go rename to internal/lefthook/run/runner.go index 68fb59a8..6dc24aae 100644 --- a/internal/lefthook/runner/runner.go +++ b/internal/lefthook/run/runner.go @@ -1,4 +1,4 @@ -package runner +package run import ( "bytes" @@ -18,6 +18,8 @@ import ( "github.com/evilmartians/lefthook/internal/config" "github.com/evilmartians/lefthook/internal/git" + "github.com/evilmartians/lefthook/internal/lefthook/run/exec" + "github.com/evilmartians/lefthook/internal/lefthook/run/filter" "github.com/evilmartians/lefthook/internal/log" ) @@ -30,8 +32,7 @@ const ( var surroundingQuotesRegexp = regexp.MustCompile(`^'(.*)'$`) -type Opts struct { - Fs afero.Fs +type Options struct { Repo *git.Repository Hook *config.Hook HookName string @@ -46,17 +47,17 @@ type Opts struct { // Runner responds for actual execution and handling the results. type Runner struct { - Opts + Options partiallyStagedFiles []string failed atomic.Bool - executor Executor + executor exec.Executor } -func NewRunner(opts Opts) *Runner { +func NewRunner(opts Options) *Runner { return &Runner{ - Opts: opts, - executor: CommandExecutor{}, + Options: opts, + executor: exec.CommandExecutor{}, } } @@ -95,8 +96,8 @@ func (r *Runner) RunAll(sourceDirs []string) { r.postHook() } -func (r *Runner) fail(name, text string) { - r.ResultChan <- resultFail(name, text) +func (r *Runner) fail(name string, err error) { + r.ResultChan <- resultFail(name, err.Error()) r.failed.Store(true) } @@ -112,11 +113,11 @@ func (r *Runner) runLFSHook() error { lfsRequiredFile := filepath.Join(r.Repo.RootPath, git.LFSRequiredFile) lfsConfigFile := filepath.Join(r.Repo.RootPath, git.LFSConfigFile) - requiredExists, err := afero.Exists(r.Fs, lfsRequiredFile) + requiredExists, err := afero.Exists(r.Repo.Fs, lfsRequiredFile) if err != nil { return err } - configExists, err := afero.Exists(r.Fs, lfsConfigFile) + configExists, err := afero.Exists(r.Repo.Fs, lfsConfigFile) if err != nil { return err } @@ -223,7 +224,7 @@ func (r *Runner) postHook() { } func (r *Runner) runScripts(dir string) { - files, err := afero.ReadDir(r.Fs, dir) // ReadDir already sorts files by .Name() + files, err := afero.ReadDir(r.Repo.Fs, dir) // ReadDir already sorts files by .Name() if err != nil || len(files) == 0 { return } @@ -288,13 +289,13 @@ func (r *Runner) runScript(script *config.Script, path string, file os.FileInfo) defer log.StartSpinner() } - finished := r.run(ExecuteOptions{ - name: file.Name(), - root: r.Repo.RootPath, - args: args, - failText: script.FailText, - interactive: script.Interactive && !r.DisableTTY, - env: script.Env, + finished := r.run(exec.Options{ + Name: file.Name(), + Root: r.Repo.RootPath, + Commands: [][]string{args}, + FailText: script.FailText, + Interactive: script.Interactive && !r.DisableTTY, + Env: script.Env, }, r.Hook.Follow) if finished && config.HookUsesStagedFiles(r.HookName) && script.StageFixed { @@ -356,7 +357,7 @@ func (r *Runner) runCommands() { } func (r *Runner) runCommand(name string, command *config.Command) { - args, err := r.prepareCommand(name, command) + run, err := r.prepareCommand(name, command) if err != nil { r.logSkip(name, err.Error()) return @@ -367,17 +368,17 @@ func (r *Runner) runCommand(name string, command *config.Command) { defer log.StartSpinner() } - finished := r.run(ExecuteOptions{ - name: name, - root: filepath.Join(r.Repo.RootPath, command.Root), - args: args.all, - failText: command.FailText, - interactive: command.Interactive && !r.DisableTTY, - env: command.Env, + finished := r.run(exec.Options{ + Name: name, + Root: filepath.Join(r.Repo.RootPath, command.Root), + Commands: run.commands, + FailText: command.FailText, + Interactive: command.Interactive && !r.DisableTTY, + Env: command.Env, }, r.Hook.Follow) if finished && config.HookUsesStagedFiles(r.HookName) && command.StageFixed { - files := args.files + files := run.files if len(files) == 0 { var err error @@ -387,7 +388,7 @@ func (r *Runner) runCommand(name string, command *config.Command) { return } - files = filterFiles(command, files) + files = filter.Apply(command, files) } if len(command.Root) > 0 { @@ -406,12 +407,12 @@ func (r *Runner) addStagedFiles(files []string) { } } -func (r *Runner) run(opts ExecuteOptions, follow bool) bool { - log.SetName(opts.name) - defer log.UnsetName(opts.name) +func (r *Runner) run(opts exec.Options, follow bool) bool { + log.SetName(opts.Name) + defer log.UnsetName(opts.Name) - if (follow || opts.interactive) && !r.SkipSettings.SkipExecution() { - r.logExecute(opts.name, nil, nil) + if (follow || opts.Interactive) && !r.SkipSettings.SkipExecution() { + r.logExecute(opts.Name, nil, nil) var out io.Writer if r.SkipSettings.SkipExecutionOutput() { @@ -422,9 +423,9 @@ func (r *Runner) run(opts ExecuteOptions, follow bool) bool { err := r.executor.Execute(opts, out) if err != nil { - r.fail(opts.name, opts.failText) + r.fail(opts.Name, errors.New(opts.FailText)) } else { - r.success(opts.name) + r.success(opts.Name) } return err == nil @@ -434,12 +435,12 @@ func (r *Runner) run(opts ExecuteOptions, follow bool) bool { err := r.executor.Execute(opts, out) if err != nil { - r.fail(opts.name, opts.failText) + r.fail(opts.Name, errors.New(opts.FailText)) } else { - r.success(opts.name) + r.success(opts.Name) } - r.logExecute(opts.name, err, out) + r.logExecute(opts.Name, err, out) return err == nil } diff --git a/internal/lefthook/runner/runner_test.go b/internal/lefthook/run/runner_test.go similarity index 98% rename from internal/lefthook/runner/runner_test.go rename to internal/lefthook/run/runner_test.go index 6a98a055..e16e24fc 100644 --- a/internal/lefthook/runner/runner_test.go +++ b/internal/lefthook/run/runner_test.go @@ -1,4 +1,4 @@ -package runner +package run import ( "errors" @@ -14,15 +14,16 @@ import ( "github.com/evilmartians/lefthook/internal/config" "github.com/evilmartians/lefthook/internal/git" + "github.com/evilmartians/lefthook/internal/lefthook/run/exec" ) type TestExecutor struct{} -func (e TestExecutor) Execute(opts ExecuteOptions, _out io.Writer) (err error) { - if opts.args[0] == "success" { +func (e TestExecutor) Execute(opts exec.Options, _out io.Writer) (err error) { + if opts.Commands[0][0] == "success" { err = nil } else { - err = errors.New(opts.args[0]) + err = errors.New(opts.Commands[0][0]) } return @@ -727,8 +728,7 @@ func TestRunAll(t *testing.T) { resultChan := make(chan Result, len(tt.hook.Commands)+len(tt.hook.Scripts)) executor := TestExecutor{} runner := &Runner{ - Opts: Opts{ - Fs: fs, + Options: Options{ Repo: repo, Hook: tt.hook, HookName: tt.hookName, diff --git a/internal/lefthook/runner/execute_windows.go b/internal/lefthook/runner/execute_windows.go deleted file mode 100644 index d443ccf3..00000000 --- a/internal/lefthook/runner/execute_windows.go +++ /dev/null @@ -1,59 +0,0 @@ -package runner - -import ( - "fmt" - "io" - "os" - "os/exec" - "path/filepath" - "strings" - "syscall" -) - -type CommandExecutor struct{} - -func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error { - command := exec.Command(opts.args[0]) - command.SysProcAttr = &syscall.SysProcAttr{ - CmdLine: strings.Join(opts.args, " "), - } - - rootDir, _ := filepath.Abs(opts.root) - command.Dir = rootDir - - envList := make([]string, len(opts.env)) - for name, value := range opts.env { - envList = append( - envList, - fmt.Sprintf("%s=%s", strings.ToUpper(name), os.ExpandEnv(value)), - ) - } - - command.Env = append(os.Environ(), envList...) - - if opts.interactive { - command.Stdout = os.Stdout - } else { - command.Stdout = out - } - - command.Stdin = os.Stdin - command.Stderr = os.Stderr - err := command.Start() - if err != nil { - return err - } - - defer func() { _ = command.Process.Kill() }() - - return command.Wait() -} - -func (e CommandExecutor) RawExecute(command []string, out io.Writer) error { - cmd := exec.Command(command[0], command[1:]...) - - cmd.Stdout = out - cmd.Stderr = os.Stderr - - return cmd.Run() -} diff --git a/internal/lefthook/runner/executor.go b/internal/lefthook/runner/executor.go deleted file mode 100644 index c3eca880..00000000 --- a/internal/lefthook/runner/executor.go +++ /dev/null @@ -1,20 +0,0 @@ -package runner - -import ( - "io" -) - -// ExecutorOptions contains the options that control the execution. -type ExecuteOptions struct { - name, root, failText string - args []string - env map[string]string - interactive bool -} - -// Executor provides an interface for command execution. -// It is used here for testing purpose mostly. -type Executor interface { - Execute(opts ExecuteOptions, out io.Writer) error - RawExecute(command []string, out io.Writer) error -} diff --git a/internal/lefthook/runner/prepare_command.go b/internal/lefthook/runner/prepare_command.go deleted file mode 100644 index c24eb67f..00000000 --- a/internal/lefthook/runner/prepare_command.go +++ /dev/null @@ -1,194 +0,0 @@ -package runner - -import ( - "errors" - "fmt" - "strings" - - "gopkg.in/alessio/shellescape.v1" - - "github.com/evilmartians/lefthook/internal/config" - "github.com/evilmartians/lefthook/internal/log" -) - -type commandArgs struct { - all []string - files []string -} - -func (r *Runner) prepareCommand(name string, command *config.Command) (*commandArgs, error) { - if command.DoSkip(r.Repo.State()) { - return nil, errors.New("settings") - } - - if intersect(r.Hook.ExcludeTags, command.Tags) { - return nil, errors.New("tags") - } - - if intersect(r.Hook.ExcludeTags, []string{name}) { - return nil, errors.New("name") - } - - if err := command.Validate(); err != nil { - r.fail(name, "") - return nil, errors.New("invalid config") - } - - args, err, skipReason := r.buildCommandArgs(command) - if err != nil { - log.Error(err) - return nil, errors.New("error") - } - if skipReason != nil { - return nil, skipReason - } - - return args, nil -} - -func (r *Runner) buildCommandArgs(command *config.Command) (*commandArgs, error, error) { - filesCommand := r.Hook.Files - if command.Files != "" { - filesCommand = command.Files - } - - stagedFiles := r.Repo.StagedFiles - if len(r.Files) > 0 { - stagedFiles = func() ([]string, error) { return r.Files, nil } - } else if r.AllFiles { - stagedFiles = r.Repo.AllFiles - } - - filesTypeToFn := map[string]func() ([]string, error){ - config.SubStagedFiles: stagedFiles, - config.PushFiles: r.Repo.PushFiles, - config.SubAllFiles: r.Repo.AllFiles, - config.SubFiles: func() ([]string, error) { - filesCommand = r.replacePositionalArguments(filesCommand) - return r.Repo.FilesByCommand(filesCommand) - }, - } - - filesFiltered := make([]string, 0) - runString := command.Run - for filesType, filesFn := range filesTypeToFn { - // Checking substitutions and skipping execution if it is empty. - // - // Special case - `files` option: return if the result of files - // command is empty. - if strings.Contains(runString, filesType) || - filesCommand != "" && filesType == config.SubFiles { - files, err := filesFn() - if err != nil { - return nil, fmt.Errorf("error replacing %s: %w", filesType, err), nil - } - if len(files) == 0 { - return nil, nil, errors.New("no files for inspection") - } - - filtered := filterFiles(command, files) - filesFiltered = append(filesFiltered, filtered...) - - prepared := escapeFiles(filtered) - if len(prepared) == 0 { - return nil, nil, errors.New("no files for inspection") - } - - runString = replaceQuoted(runString, filesType, prepared) - } - } - - if len(filesFiltered) == 0 && config.HookUsesStagedFiles(r.HookName) { - files, err := r.Repo.StagedFiles() - if err == nil { - if len(filterFiles(command, files)) == 0 { - return nil, nil, errors.New("no matching staged files") - } - } - } - - if len(filesFiltered) == 0 && config.HookUsesPushFiles(r.HookName) { - files, err := r.Repo.PushFiles() - if err == nil { - if len(filterFiles(command, files)) == 0 { - return nil, nil, errors.New("no matching push files") - } - } - } - - runString = r.replacePositionalArguments(runString) - - log.Debug("[lefthook] executing: ", runString) - - return &commandArgs{ - files: filesFiltered, - all: strings.Split(runString, " "), - }, nil, nil -} - -func (r *Runner) replacePositionalArguments(runString string) string { - runString = strings.ReplaceAll(runString, "{0}", strings.Join(r.GitArgs, " ")) - for i, gitArg := range r.GitArgs { - runString = strings.ReplaceAll(runString, fmt.Sprintf("{%d}", i+1), gitArg) - } - return runString -} - -func filterFiles(command *config.Command, files []string) []string { - if files == nil { - return []string{} - } - - log.Debug("[lefthook] files before filters:\n", files) - - files = filterGlob(files, command.Glob) - files = filterExclude(files, command.Exclude) - files = filterRelative(files, command.Root) - - log.Debug("[lefthook] files after filters:\n", files) - - return files -} - -// Escape file names to prevent unexpected bugs. -func escapeFiles(files []string) []string { - var filesEsc []string - for _, fileName := range files { - if len(fileName) > 0 { - filesEsc = append(filesEsc, shellescape.Quote(fileName)) - } - } - - log.Debug("[lefthook] files after escaping:\n", filesEsc) - - return filesEsc -} - -func replaceQuoted(source, substitution string, files []string) string { - for _, elem := range [][]string{ - {"\"", "\"" + substitution + "\""}, - {"'", "'" + substitution + "'"}, - {"", substitution}, - } { - quote := elem[0] - sub := elem[1] - if !strings.Contains(source, sub) { - continue - } - - quotedFiles := files - if len(quote) != 0 { - quotedFiles = make([]string, 0, len(files)) - for _, fileName := range files { - quotedFiles = append(quotedFiles, - quote+surroundingQuotesRegexp.ReplaceAllString(fileName, "$1")+quote) - } - } - - source = strings.ReplaceAll( - source, sub, strings.Join(quotedFiles, " "), - ) - } - - return source -}