From e85694cc35bddf306ef476e651b5ca44390c6acc Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Thu, 21 Mar 2024 13:26:33 +0300 Subject: [PATCH 1/3] feat: add priorities to scripts --- internal/config/command.go | 4 +++ internal/config/script.go | 13 ++++--- internal/lefthook/run/runner.go | 52 ++++++++++++++++++---------- internal/lefthook/run/runner_test.go | 41 ++++++++++++++++++++-- 4 files changed, 86 insertions(+), 24 deletions(-) diff --git a/internal/config/command.go b/internal/config/command.go index 7fce4c1e..37b93f5d 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -44,6 +44,10 @@ func (c Command) DoSkip(gitState git.State) bool { return skipChecker.Check(gitState, c.Skip, c.Only) } +func (c Command) GetPriority() int { + return c.Priority +} + type commandRunReplace struct { Run string `mapstructure:"run"` } diff --git a/internal/config/script.go b/internal/config/script.go index f84faad4..5505b18a 100644 --- a/internal/config/script.go +++ b/internal/config/script.go @@ -12,10 +12,11 @@ import ( type Script struct { Runner string `json:"runner" mapstructure:"runner" toml:"runner" yaml:"runner"` - Skip interface{} `json:"skip,omitempty" mapstructure:"skip" toml:"skip,omitempty,inline" yaml:",omitempty"` - Only interface{} `json:"only,omitempty" mapstructure:"only" toml:"only,omitempty,inline" yaml:",omitempty"` - Tags []string `json:"tags,omitempty" mapstructure:"tags" toml:"tags,omitempty" yaml:",omitempty"` - Env map[string]string `json:"env,omitempty" mapstructure:"env" toml:"env,omitempty" yaml:",omitempty"` + Skip interface{} `json:"skip,omitempty" mapstructure:"skip" toml:"skip,omitempty,inline" yaml:",omitempty"` + Only interface{} `json:"only,omitempty" mapstructure:"only" toml:"only,omitempty,inline" yaml:",omitempty"` + Tags []string `json:"tags,omitempty" mapstructure:"tags" toml:"tags,omitempty" yaml:",omitempty"` + Env map[string]string `json:"env,omitempty" mapstructure:"env" toml:"env,omitempty" yaml:",omitempty"` + Priority int `json:"priority,omitempty" mapstructure:"priority" toml:"priority,omitempty" yaml:",omitempty"` FailText string `json:"fail_text,omitempty" mapstructure:"fail_text" toml:"fail_text,omitempty" yaml:"fail_text,omitempty"` Interactive bool `json:"interactive,omitempty" mapstructure:"interactive" toml:"interactive,omitempty" yaml:",omitempty"` @@ -32,6 +33,10 @@ type scriptRunnerReplace struct { Runner string `mapstructure:"runner"` } +func (s Script) GetPriority() int { + return s.Priority +} + func mergeScripts(base, extra *viper.Viper) (map[string]*Script, error) { if base == nil && extra == nil { return nil, nil diff --git a/internal/lefthook/run/runner.go b/internal/lefthook/run/runner.go index ee2b787a..8315a4ae 100644 --- a/internal/lefthook/run/runner.go +++ b/internal/lefthook/run/runner.go @@ -66,6 +66,11 @@ func NewRunner(opts Options) *Runner { } } +type executable interface { + *config.Command | *config.Script + GetPriority() int +} + // RunAll runs scripts and commands. // LFS hook is executed at first if needed. func (r *Runner) RunAll(ctx context.Context, sourceDirs []string) { @@ -235,10 +240,20 @@ func (r *Runner) runScripts(ctx context.Context, dir string) { return } + scripts := make([]string, 0, len(files)) + filesMap := make(map[string]os.FileInfo) + for _, file := range files { + filesMap[file.Name()] = file + scripts = append(scripts, file.Name()) + } + sortByPriority(scripts, r.Hook.Scripts) + interactiveScripts := make([]os.FileInfo, 0) var wg sync.WaitGroup - for _, file := range files { + for _, name := range scripts { + file := filesMap[name] + if ctx.Err() != nil { return } @@ -331,7 +346,7 @@ func (r *Runner) runCommands(ctx context.Context) { } } - sortCommands(commands, r.Hook.Commands) + sortByPriority(commands, r.Hook.Commands) interactiveCommands := make([]string, 0) var wg sync.WaitGroup @@ -531,28 +546,29 @@ func (r *Runner) logExecute(name string, err error, out io.Reader) { } } -// sortCommands sorts the command names by preceding numbers if they occur and special priority if it is set. -// If the command names starts with letter the command name will be sorted alphabetically. +// sortByPriority sorts the names by preceding numbers if they occur and special priority if it is set. +// If the names starts with letter the command name will be sorted alphabetically. +// If there's a `priority` field defined for a command or script it will be used instead of alphanumeric sorting. // // []string{"1_command", "10command", "3 command", "command5"} // -> 1_command, 3 command, 10command, command5 -func sortCommands(strs []string, commands map[string]*config.Command) { - sort.SliceStable(strs, func(i, j int) bool { - commandI, iOk := commands[strs[i]] - commandJ, jOk := commands[strs[j]] +func sortByPriority[E executable](array []string, exe map[string]E) { + sort.SliceStable(array, func(i, j int) bool { + exeI, iOk := exe[array[i]] + exeJ, jOk := exe[array[j]] - if iOk && commandI.Priority != 0 || jOk && commandJ.Priority != 0 { - if !iOk || commandI.Priority == 0 { + if iOk && exeI.GetPriority() != 0 || jOk && exeJ.GetPriority() != 0 { + if !iOk || exeI.GetPriority() == 0 { return false } - if !jOk || commandJ.Priority == 0 { + if !jOk || exeJ.GetPriority() == 0 { return true } - return commandI.Priority < commandJ.Priority + return exeI.GetPriority() < exeJ.GetPriority() } numEnds := -1 - for idx, ch := range strs[i] { + for idx, ch := range array[i] { if unicode.IsDigit(ch) { numEnds = idx } else { @@ -560,15 +576,15 @@ func sortCommands(strs []string, commands map[string]*config.Command) { } } if numEnds == -1 { - return strs[i] < strs[j] + return array[i] < array[j] } - numI, err := strconv.Atoi(strs[i][:numEnds+1]) + numI, err := strconv.Atoi(array[i][:numEnds+1]) if err != nil { - return strs[i] < strs[j] + return array[i] < array[j] } numEnds = -1 - for idx, ch := range strs[j] { + for idx, ch := range array[j] { if unicode.IsDigit(ch) { numEnds = idx } else { @@ -578,7 +594,7 @@ func sortCommands(strs []string, commands map[string]*config.Command) { if numEnds == -1 { return true } - numJ, err := strconv.Atoi(strs[j][:numEnds+1]) + numJ, err := strconv.Atoi(array[j][:numEnds+1]) if err != nil { return true } diff --git a/internal/lefthook/run/runner_test.go b/internal/lefthook/run/runner_test.go index 76bcd4c9..dea1b2fd 100644 --- a/internal/lefthook/run/runner_test.go +++ b/internal/lefthook/run/runner_test.go @@ -906,7 +906,8 @@ func TestReplaceQuoted(t *testing.T) { } } -func TestSortCommands(t *testing.T) { +//nolint:dupl +func TestSortByPriorityCommands(t *testing.T) { for i, tt := range [...]struct { name string names []string @@ -931,7 +932,43 @@ func TestSortCommands(t *testing.T) { }, } { t.Run(fmt.Sprintf("%d: %s", i+1, tt.name), func(t *testing.T) { - sortCommands(tt.names, tt.commands) + sortByPriority(tt.names, tt.commands) + for i, name := range tt.result { + if tt.names[i] != name { + t.Errorf("Not matching on index %d: %s != %s", i, name, tt.names[i]) + } + } + }) + } +} + +//nolint:dupl +func TestSortByPriorityScripts(t *testing.T) { + for i, tt := range [...]struct { + name string + names []string + scripts map[string]*config.Script + result []string + }{ + { + name: "alphanumeric sort", + names: []string{"10_a.sh", "1_a.sh", "2_a.sh", "5_b.sh"}, + scripts: map[string]*config.Script{}, + result: []string{"1_a.sh", "2_a.sh", "5_b.sh", "10_a.sh"}, + }, + { + name: "partial priority", + names: []string{"10.rb", "file.sh", "script.go", "5_a.sh"}, + scripts: map[string]*config.Script{ + "5_a.sh": {Priority: 10}, + "script.go": {Priority: 1}, + "10.rb": {}, + }, + result: []string{"script.go", "5_a.sh", "10.rb", "file.sh"}, + }, + } { + t.Run(fmt.Sprintf("%d: %s", i+1, tt.name), func(t *testing.T) { + sortByPriority(tt.names, tt.scripts) for i, name := range tt.result { if tt.names[i] != name { t.Errorf("Not matching on index %d: %s != %s", i, name, tt.names[i]) From 44eb0e36fcbb86c3267ef93b54f328e99547ea3f Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Thu, 21 Mar 2024 13:31:29 +0300 Subject: [PATCH 2/3] docs: mention scripts for priority option --- docs/configuration.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 6c1430b7..70d3793c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -60,6 +60,7 @@ Lefthook [supports](#config-file) YAML, JSON, and TOML configuration. In this do - [`stage_fixed`](#stage_fixed) - [`interactive`](#interactive) - [`use_stdin`](#use_stdin) + - [`priority`](#priority) - [Examples](#examples) - [More info](#more-info) @@ -1280,9 +1281,9 @@ Whether to use interactive mode. This applies the certain behavior: > > This option makes sense only when `parallel: false` or `piped: true` is set. > -> Value `0` is considered an `+Infinity`, so commands with `priority: 0` or without this setting will be run at the very end. +> Value `0` is considered an `+Infinity`, so commands or scripts with `priority: 0` or without this setting will be run at the very end. -Set command priority from 1 to +Infinity. This option can be used to configure the order of the sequential commands. +Set priority from 1 to +Infinity. This option can be used to configure the order of the sequential steps. **Example** @@ -1301,6 +1302,14 @@ post-checkout: db-seed: priority: 3 run: rails db:seed + + scripts: + "check-spelling.sh": + runner: bash + priority: 1 + "check-grammar.rb": + runner: ruby + priority: 2 ``` ## Script From e09d0a9fa3efea7394ea8a7e9f1a8533a4bc7add Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Mon, 1 Apr 2024 09:45:16 +0300 Subject: [PATCH 3/3] chore: better naming --- internal/config/command.go | 10 +++++----- internal/config/script.go | 10 +++++----- internal/lefthook/run/runner.go | 32 ++++++++++++++++---------------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/internal/config/command.go b/internal/config/command.go index 37b93f5d..dfdc6c07 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -31,6 +31,10 @@ type Command struct { StageFixed bool `json:"stage_fixed,omitempty" mapstructure:"stage_fixed" toml:"stage_fixed,omitempty" yaml:"stage_fixed,omitempty"` } +type commandRunReplace struct { + Run string `mapstructure:"run"` +} + func (c Command) Validate() error { if !isRunnerFilesCompatible(c.Run) { return errFilesIncompatible @@ -44,14 +48,10 @@ func (c Command) DoSkip(gitState git.State) bool { return skipChecker.Check(gitState, c.Skip, c.Only) } -func (c Command) GetPriority() int { +func (c Command) ExecutionPriority() int { return c.Priority } -type commandRunReplace struct { - Run string `mapstructure:"run"` -} - func mergeCommands(base, extra *viper.Viper) (map[string]*Command, error) { if base == nil && extra == nil { return nil, nil diff --git a/internal/config/script.go b/internal/config/script.go index 5505b18a..fe443ecd 100644 --- a/internal/config/script.go +++ b/internal/config/script.go @@ -24,16 +24,16 @@ type Script struct { StageFixed bool `json:"stage_fixed,omitempty" mapstructure:"stage_fixed" toml:"stage_fixed,omitempty" yaml:"stage_fixed,omitempty"` } +type scriptRunnerReplace struct { + Runner string `mapstructure:"runner"` +} + func (s Script) DoSkip(gitState git.State) bool { skipChecker := NewSkipChecker(NewOsExec()) return skipChecker.Check(gitState, s.Skip, s.Only) } -type scriptRunnerReplace struct { - Runner string `mapstructure:"runner"` -} - -func (s Script) GetPriority() int { +func (s Script) ExecutionPriority() int { return s.Priority } diff --git a/internal/lefthook/run/runner.go b/internal/lefthook/run/runner.go index 8315a4ae..265fa2e4 100644 --- a/internal/lefthook/run/runner.go +++ b/internal/lefthook/run/runner.go @@ -68,7 +68,7 @@ func NewRunner(opts Options) *Runner { type executable interface { *config.Command | *config.Script - GetPriority() int + ExecutionPriority() int } // RunAll runs scripts and commands. @@ -546,29 +546,29 @@ func (r *Runner) logExecute(name string, err error, out io.Reader) { } } -// sortByPriority sorts the names by preceding numbers if they occur and special priority if it is set. +// sortByPriority sorts the tags by preceding numbers if they occur and special priority if it is set. // If the names starts with letter the command name will be sorted alphabetically. // If there's a `priority` field defined for a command or script it will be used instead of alphanumeric sorting. // // []string{"1_command", "10command", "3 command", "command5"} // -> 1_command, 3 command, 10command, command5 -func sortByPriority[E executable](array []string, exe map[string]E) { - sort.SliceStable(array, func(i, j int) bool { - exeI, iOk := exe[array[i]] - exeJ, jOk := exe[array[j]] +func sortByPriority[E executable](tags []string, executables map[string]E) { + sort.SliceStable(tags, func(i, j int) bool { + exeI, okI := executables[tags[i]] + exeJ, okJ := executables[tags[j]] - if iOk && exeI.GetPriority() != 0 || jOk && exeJ.GetPriority() != 0 { - if !iOk || exeI.GetPriority() == 0 { + if okI && exeI.ExecutionPriority() != 0 || okJ && exeJ.ExecutionPriority() != 0 { + if !okI || exeI.ExecutionPriority() == 0 { return false } - if !jOk || exeJ.GetPriority() == 0 { + if !okJ || exeJ.ExecutionPriority() == 0 { return true } - return exeI.GetPriority() < exeJ.GetPriority() + return exeI.ExecutionPriority() < exeJ.ExecutionPriority() } numEnds := -1 - for idx, ch := range array[i] { + for idx, ch := range tags[i] { if unicode.IsDigit(ch) { numEnds = idx } else { @@ -576,15 +576,15 @@ func sortByPriority[E executable](array []string, exe map[string]E) { } } if numEnds == -1 { - return array[i] < array[j] + return tags[i] < tags[j] } - numI, err := strconv.Atoi(array[i][:numEnds+1]) + numI, err := strconv.Atoi(tags[i][:numEnds+1]) if err != nil { - return array[i] < array[j] + return tags[i] < tags[j] } numEnds = -1 - for idx, ch := range array[j] { + for idx, ch := range tags[j] { if unicode.IsDigit(ch) { numEnds = idx } else { @@ -594,7 +594,7 @@ func sortByPriority[E executable](array []string, exe map[string]E) { if numEnds == -1 { return true } - numJ, err := strconv.Atoi(array[j][:numEnds+1]) + numJ, err := strconv.Atoi(tags[j][:numEnds+1]) if err != nil { return true }