Skip to content

Commit

Permalink
chore: remove redundant parallelisation (#690)
Browse files Browse the repository at this point in the history
* chore: remove redundant parallelisation

* chore: use skipError type for clear code
  • Loading branch information
mrexox committed Apr 4, 2024
1 parent 08d69af commit 9814621
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 149 deletions.
23 changes: 6 additions & 17 deletions internal/lefthook/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@ Run 'lefthook install' manually.`,
return err
}

startTime := time.Now()
resultChan := make(chan runner.Result, len(hook.Commands)+len(hook.Scripts))

if args.FilesFromStdin {
paths, err := io.ReadAll(os.Stdin)
if err != nil {
Expand Down Expand Up @@ -160,7 +157,6 @@ Run 'lefthook install' manually.`,
Hook: hook,
HookName: hookName,
GitArgs: gitArgs,
ResultChan: resultChan,
LogSettings: logSettings,
DisableTTY: cfg.NoTTY || args.NoTTY,
Files: args.Files,
Expand All @@ -169,15 +165,8 @@ Run 'lefthook install' manually.`,
},
)

go func() {
r.RunAll(ctx, sourceDirs)
close(resultChan)
}()

var results []runner.Result
for res := range resultChan {
results = append(results, res)
}
startTime := time.Now()
results := r.RunAll(ctx, sourceDirs)

if ctx.Err() != nil {
return errors.New("Interrupted")
Expand All @@ -186,7 +175,7 @@ Run 'lefthook install' manually.`,
printSummary(time.Since(startTime), results, logSettings)

for _, result := range results {
if result.Err != nil {
if result.Failure() {
return errors.New("") // No error should be printed
}
}
Expand Down Expand Up @@ -227,7 +216,7 @@ func printSummary(

if logSettings.LogSuccess() {
for _, result := range results {
if result.Err != nil {
if !result.Success() {
continue
}

Expand All @@ -237,11 +226,11 @@ func printSummary(

if logSettings.LogFailure() {
for _, result := range results {
if result.Err == nil {
if !result.Failure() {
continue
}

failText := result.Err.Error()
failText := result.Text()
if len(failText) != 0 {
failText = fmt.Sprintf(": %s", failText)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/lefthook/runner/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
Name, Root string
Commands []string
Env map[string]string
Interactive, UseStdin bool
Expand Down
38 changes: 16 additions & 22 deletions internal/lefthook/runner/prepare_command.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package runner

import (
"errors"
"fmt"
"runtime"
"strings"
Expand Down Expand Up @@ -36,35 +35,30 @@ const (

func (r *Runner) prepareCommand(name string, command *config.Command) (*run, error) {
if command.DoSkip(r.Repo.State()) {
return nil, errors.New("settings")
return nil, &skipError{"settings"}
}

if intersect(r.Hook.ExcludeTags, command.Tags) {
return nil, errors.New("tags")
return nil, &skipError{"tags"}
}

if intersect(r.Hook.ExcludeTags, []string{name}) {
return nil, errors.New("name")
return nil, &skipError{"name"}
}

if err := command.Validate(); err != nil {
r.fail(name, err)
return nil, err
}

args, err, skipReason := r.buildRun(command)
args, err := r.buildRun(command)
if err != nil {
log.Error(err)
return nil, errors.New("error")
}
if skipReason != nil {
return nil, skipReason
return nil, err
}

return args, nil
}

func (r *Runner) buildRun(command *config.Command) (*run, error, error) {
func (r *Runner) buildRun(command *config.Command) (*run, error) {
filesCmd := r.Hook.Files
if len(command.Files) > 0 {
filesCmd = command.Files
Expand Down Expand Up @@ -118,12 +112,12 @@ func (r *Runner) buildRun(command *config.Command) (*run, error, error) {

files, err := fn()
if err != nil {
return nil, fmt.Errorf("error replacing %s: %w", filesType, err), nil
return nil, fmt.Errorf("error replacing %s: %w", filesType, err)
}

files = filter.Apply(command, files)
if !r.Force && len(files) == 0 {
return nil, nil, errors.New("no files for inspection")
return nil, &skipError{"no files for inspection"}
}

templ.files = files
Expand All @@ -135,13 +129,13 @@ func (r *Runner) buildRun(command *config.Command) (*run, error, error) {
if !r.Force && 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
return nil, fmt.Errorf("error calling replace command for %s: %w", config.SubFiles, err)
}

files = filter.Apply(command, files)

if len(files) == 0 {
return nil, nil, errors.New("no files for inspection")
return nil, &skipError{"no files for inspection"}
}
}

Expand All @@ -160,30 +154,30 @@ func (r *Runner) buildRun(command *config.Command) (*run, error, error) {
result := replaceInChunks(runString, templates, maxlen)

if r.Force || len(result.files) != 0 {
return result, nil, nil
return result, nil
}

if config.HookUsesStagedFiles(r.HookName) {
ok, err := canSkipCommand(command, templates[config.SubStagedFiles], r.Repo.StagedFiles)
if err != nil {
return nil, err, nil
return nil, err
}
if ok {
return nil, nil, errors.New("no matching staged files")
return nil, &skipError{"no matching staged files"}
}
}

if config.HookUsesPushFiles(r.HookName) {
ok, err := canSkipCommand(command, templates[config.SubPushFiles], r.Repo.PushFiles)
if err != nil {
return nil, err, nil
return nil, err
}
if ok {
return nil, nil, errors.New("no matching push files")
return nil, &skipError{"no matching push files"}
}
}

return result, nil, nil
return result, nil
}

func canSkipCommand(command *config.Command, template *template, filesFn func() ([]string, error)) (bool, error) {
Expand Down
10 changes: 4 additions & 6 deletions internal/lefthook/runner/prepare_script.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package runner

import (
"errors"
"os"
"strings"

Expand All @@ -11,25 +10,24 @@ import (

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

if intersect(r.Hook.ExcludeTags, script.Tags) {
return "", errors.New("excluded tags")
return "", &skipError{"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 "", errors.New("not a regular file")
return "", &skipError{"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 "", errors.New("system error")
return "", err
}
}

Expand Down
40 changes: 40 additions & 0 deletions internal/lefthook/runner/result.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package runner

type status int8

const (
success status = iota
failure
skip
)

// Result contains name of a command/script and an optional fail string.
type Result struct {
Name string
status status
text string
}

func (r Result) Success() bool {
return r.status == success
}

func (r Result) Failure() bool {
return r.status == failure
}

func (r Result) Text() string {
return r.text
}

func skipped(name string) Result {
return Result{Name: name, status: skip}
}

func succeeded(name string) Result {
return Result{Name: name, status: success}
}

func failed(name, text string) Result {
return Result{Name: name, status: failure, text: text}
}
Loading

0 comments on commit 9814621

Please sign in to comment.