Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove redundant parallelisation #690

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
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
Loading