From df40a66530a9133b43ca082c32f3f7445d25a4da Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Tue, 22 Nov 2022 11:05:59 +0300 Subject: [PATCH 1/4] feature: Skip based on branch name + Allow global skip rules Signed-off-by: Valentin Kiselev --- internal/config/hook.go | 18 +++++++--- internal/config/skip.go | 17 ++++++--- internal/git/state.go | 56 ++++++++++++++++++++++++++---- internal/lefthook/runner/runner.go | 9 +++-- 4 files changed, 82 insertions(+), 18 deletions(-) diff --git a/internal/config/hook.go b/internal/config/hook.go index cb8b53a6..4449f016 100644 --- a/internal/config/hook.go +++ b/internal/config/hook.go @@ -6,6 +6,8 @@ import ( "strings" "github.com/spf13/viper" + + "github.com/evilmartians/lefthook/internal/git" ) const CMD = "{cmd}" @@ -23,10 +25,11 @@ type Hook struct { // Unmarshaling it manually, so omit auto unmarshaling Scripts map[string]*Script `mapstructure:"?"` - Files string `mapstructure:"files"` - Parallel bool `mapstructure:"parallel"` - Piped bool `mapstructure:"piped"` - ExcludeTags []string `mapstructure:"exclude_tags"` + Files string `mapstructure:"files"` + Parallel bool `mapstructure:"parallel"` + Piped bool `mapstructure:"piped"` + ExcludeTags []string `mapstructure:"exclude_tags"` + Skip interface{} `mapstructure:"skip"` } func (h *Hook) Validate() error { @@ -37,6 +40,13 @@ func (h *Hook) Validate() error { return nil } +func (h *Hook) DoSkip(gitState git.State) bool { + if value := h.Skip; value != nil { + return isSkip(gitState, value) + } + return false +} + func unmarshalHooks(base, extra *viper.Viper) (*Hook, error) { if base == nil && extra == nil { return nil, nil diff --git a/internal/config/skip.go b/internal/config/skip.go index 5a6bb8a6..eec727ee 100644 --- a/internal/config/skip.go +++ b/internal/config/skip.go @@ -2,16 +2,23 @@ package config import "github.com/evilmartians/lefthook/internal/git" -func isSkip(gitSkipState git.State, value interface{}) bool { +func isSkip(gitState git.State, value interface{}) bool { switch typedValue := value.(type) { case bool: return typedValue case string: - return git.State(typedValue) == gitSkipState + return string(typedValue) == gitState.Step case []interface{}: - for _, gitState := range typedValue { - if git.State(gitState.(string)) == gitSkipState { - return true + for _, state := range typedValue { + switch typedState := state.(type) { + case string: + if string(typedState) == gitState.Step { + return true + } + case map[string]interface{}: + if typedState["ref"].(string) == gitState.Branch { + return true + } } } } diff --git a/internal/git/state.go b/internal/git/state.go index 0ab96921..69d6dfbf 100644 --- a/internal/git/state.go +++ b/internal/git/state.go @@ -1,26 +1,68 @@ package git import ( + "bufio" "os" "path/filepath" + "regexp" ) -type State string +type State struct { + Branch, Step string +} const ( - NilState State = "" - MergeState State = "merge" - RebaseState State = "rebase" + NilStep string = "" + MergeStep string = "merge" + RebaseStep string = "rebase" ) +var refBranchRegexp = regexp.MustCompile(`^ref:\s*refs/heads/(.+)$`) + func (r *Repository) State() State { + branch := r.Branch() if r.isMergeState() { - return MergeState + return State{ + Branch: branch, + Step: MergeStep, + } } if r.isRebaseState() { - return RebaseState + return State{ + Branch: branch, + Step: RebaseStep, + } + } + return State{ + Branch: branch, + Step: NilStep, + } +} + +func (r *Repository) Branch() string { + headFile := filepath.Join(r.GitPath, "HEAD") + if _, err := r.Fs.Stat(headFile); os.IsNotExist(err) { + return "" } - return NilState + + file, err := r.Fs.Open(headFile) + if err != nil { + return "" + } + defer file.Close() + + scanner := bufio.NewScanner(file) + scanner.Split(bufio.ScanLines) + + for scanner.Scan() { + match := refBranchRegexp.FindStringSubmatch(scanner.Text()) + + if len(match) > 1 { + return match[1] + } + } + + return "" } func (r *Repository) isMergeState() bool { diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/runner/runner.go index 9a130cac..37be0715 100644 --- a/internal/lefthook/runner/runner.go +++ b/internal/lefthook/runner/runner.go @@ -66,6 +66,11 @@ func (r *Runner) RunAll(hookName string, sourceDirs []string) { log.Error(err) } + if r.hook.Skip != nil && r.hook.DoSkip(r.repo.State()) { + logSkip(hookName, "(SKIP BY HOOK SETTING)") + return + } + log.StartSpinner() defer log.StopSpinner() @@ -210,7 +215,7 @@ func (r *Runner) runScripts(dir string) { } func (r *Runner) runScript(script *config.Script, path string, file os.FileInfo) { - if script.DoSkip(r.repo.State()) { + if script.Skip != nil && script.DoSkip(r.repo.State()) { logSkip(file.Name(), "(SKIP BY SETTINGS)") return } @@ -304,7 +309,7 @@ func (r *Runner) runCommands() { } func (r *Runner) runCommand(name string, command *config.Command) { - if command.DoSkip(r.repo.State()) { + if command.Skip != nil && command.DoSkip(r.repo.State()) { logSkip(name, "(SKIP BY SETTINGS)") return } From a3d620bef7ef6791a2364b181a93f87a56b887ec Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Tue, 22 Nov 2022 11:21:16 +0300 Subject: [PATCH 2/4] chore: Add tests Signed-off-by: Valentin Kiselev --- internal/config/load_test.go | 8 ++- internal/config/skip.go | 4 +- internal/lefthook/runner/runner_test.go | 70 ++++++++++++++++++++++++- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/internal/config/load_test.go b/internal/config/load_test.go index d5d3663f..0d920943 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -228,6 +228,8 @@ remote: config: examples/custom.yml pre-commit: + skip: + - ref: main commands: global: run: echo 'Global!' @@ -239,7 +241,8 @@ pre-commit: commands: lint: run: yarn lint - skip: true + skip: + - merge scripts: "test.sh": runner: bash @@ -256,10 +259,11 @@ pre-commit: }, Hooks: map[string]*Hook{ "pre-commit": { + Skip: []interface{}{map[string]interface{}{"ref": "main"}}, Commands: map[string]*Command{ "lint": { Run: "yarn lint", - Skip: true, + Skip: []interface{}{"merge"}, }, "global": { Run: "echo 'Global!'", diff --git a/internal/config/skip.go b/internal/config/skip.go index eec727ee..35d2c064 100644 --- a/internal/config/skip.go +++ b/internal/config/skip.go @@ -7,12 +7,12 @@ func isSkip(gitState git.State, value interface{}) bool { case bool: return typedValue case string: - return string(typedValue) == gitState.Step + return typedValue == gitState.Step case []interface{}: for _, state := range typedValue { switch typedState := state.(type) { case string: - if string(typedState) == gitState.Step { + if typedState == gitState.Step { return true } case map[string]interface{}: diff --git a/internal/lefthook/runner/runner_test.go b/internal/lefthook/runner/runner_test.go index 0a58113a..be5c711c 100644 --- a/internal/lefthook/runner/runner_test.go +++ b/internal/lefthook/runner/runner_test.go @@ -48,7 +48,7 @@ func TestRunAll(t *testing.T) { } for i, tt := range [...]struct { - name string + name, branch string args []string sourceDirs []string existingFiles []string @@ -154,6 +154,25 @@ func TestRunAll(t *testing.T) { }, success: []Result{{Name: "lint", Status: StatusOk}}, }, + { + name: "with global skip merge", + existingFiles: []string{ + filepath.Join(gitPath, "MERGE_HEAD"), + }, + hook: &config.Hook{ + Skip: "merge", + Commands: map[string]*config.Command{ + "test": { + Run: "success", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{}, + }, { name: "with skip rebase and merge in an array", existingFiles: []string{ @@ -174,6 +193,49 @@ func TestRunAll(t *testing.T) { }, success: []Result{{Name: "lint", Status: StatusOk}}, }, + { + name: "with global skip on ref", + branch: "main", + existingFiles: []string{ + filepath.Join(gitPath, "HEAD"), + }, + hook: &config.Hook{ + Skip: []interface{}{"merge", map[string]interface{}{"ref": "main"}}, + Commands: map[string]*config.Command{ + "test": { + Run: "success", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{}, + }, + { + name: "with global skip on another ref", + branch: "fix", + existingFiles: []string{ + filepath.Join(gitPath, "HEAD"), + }, + hook: &config.Hook{ + Skip: []interface{}{"merge", map[string]interface{}{"ref": "main"}}, + Commands: map[string]*config.Command{ + "test": { + Run: "success", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{ + {Name: "test", Status: StatusOk}, + {Name: "lint", Status: StatusOk}, + }, + }, { name: "with fail test", hook: &config.Hook{ @@ -263,6 +325,12 @@ func TestRunAll(t *testing.T) { } } + if len(tt.branch) > 0 { + if err := afero.WriteFile(fs, filepath.Join(gitPath, "HEAD"), []byte("ref: refs/heads/"+tt.branch), 0o644); err != nil { + t.Errorf("unexpected error: %s", err) + } + } + t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) { runner.RunAll(hookName, tt.sourceDirs) close(resultChan) From f0f58e063ffc9bd7998e8c91f3749c03c3f6906f Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Tue, 22 Nov 2022 11:24:55 +0300 Subject: [PATCH 3/4] docs: Update configuration.md about skip option Signed-off-by: Valentin Kiselev --- docs/configuration.md | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index df8b478e..0ca57a13 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -12,6 +12,7 @@ - [`ref`](#ref) - [`config`](#config) - [Hook](#git-hook) + - [`skip`](#skip) - [`files`](#files-global) - [`parallel`](#parallel) - [`piped`](#piped) @@ -518,11 +519,11 @@ pre-commit ### `skip` -You can skip commands or scripts using `skip` option. You can only skip when merging or rebasing if you want. +You can skip all or specific commands and scripts using `skip` option. You can also skip when merging, rebasing, or being on a specific branch. **Example** -Always skipping: +Always skipping a command: ```yml # lefthook.yml @@ -560,6 +561,21 @@ pre-commit: run: yarn lint ``` +Skipping the whole hook on `main` branch: + +```yml +# lefthook.yml + +pre-commit: + skip: + - ref: main + commands: + lint: + run: yarn lint + text: + run: yarn test +``` + **Notes** Always skipping is useful when you have a `lefthook-local.yml` config and you don't want to run some commands locally. So you just overwrite the `skip` option for them to be `true`. From 749bab260b9d5006f43a436d2dcac84f87fbe28f Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Tue, 22 Nov 2022 17:43:52 +0300 Subject: [PATCH 4/4] chore: Update changelog Signed-off-by: Valentin Kiselev --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46c288c8..388c13ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## master (unreleased) +- feature: Skip based on branch name and allow global skip rules ([PR #376](https://github.com/evilmartians/lefthook/pull/376) by @mrexox) - fix: Omit LFS output unless it is required ([PR #373](https://github.com/evilmartians/lefthook/pull/373) by @mrexox) ## 1.2.1 (2022-11-17)