From 34cee8b49e866affb9677953c2fd46148c502a58 Mon Sep 17 00:00:00 2001 From: Valentine Kiselev Date: Sun, 26 Jun 2022 12:42:42 +0300 Subject: [PATCH] fix: Bring back support for skipping lefthook on rebase flow (#289) * fix: Bring back support for skipping lefthook on rebase flow Signed-off-by: Valentin Kiselev * chore: Update initializations in tests Signed-off-by: Valentin Kiselev * chore(changelog): Update with PR #289 Signed-off-by: Valentin Kiselev --- CHANGELOG.md | 2 + internal/config/skip.go | 2 +- internal/git/repository.go | 53 +++++++++++--------- internal/git/state.go | 6 +-- internal/lefthook/add_test.go | 14 +++--- internal/lefthook/install_test.go | 14 +++--- internal/lefthook/lefthook.go | 2 +- internal/lefthook/run_test.go | 78 ++++++++++++++++++++++++++--- internal/lefthook/runner/runner.go | 2 +- internal/lefthook/uninstall_test.go | 14 +++--- 10 files changed, 132 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72ff9d3d..c976eb76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## master (unreleased) +- Support skipping on rebase ([PR #289](https://github.com/evilmartians/lefthook/pull/289) by @mrexox) + # 1.0.3 (2022-06-25) - Fix NPM package diff --git a/internal/config/skip.go b/internal/config/skip.go index cae8c8cd..5a6bb8a6 100644 --- a/internal/config/skip.go +++ b/internal/config/skip.go @@ -10,7 +10,7 @@ func isSkip(gitSkipState git.State, value interface{}) bool { return git.State(typedValue) == gitSkipState case []interface{}: for _, gitState := range typedValue { - if gitState == gitSkipState { + if git.State(gitState.(string)) == gitSkipState { return true } } diff --git a/internal/git/repository.go b/internal/git/repository.go index f174b2f2..117a68a3 100644 --- a/internal/git/repository.go +++ b/internal/git/repository.go @@ -6,6 +6,8 @@ import ( "path/filepath" "runtime" "strings" + + "github.com/spf13/afero" ) const ( @@ -19,14 +21,14 @@ const ( // Repository represents a git repository. type Repository struct { + Fs afero.Fs HooksPath string RootPath string - - gitPath string + GitPath string } // NewRepository returns a Repository or an error, if git repository it not initialized. -func NewRepository() (*Repository, error) { +func NewRepository(fs afero.Fs) (*Repository, error) { rootPath, err := execGit(cmdRootPath) if err != nil { return nil, err @@ -46,44 +48,33 @@ func NewRepository() (*Repository, error) { } return &Repository{ + Fs: fs, HooksPath: filepath.Join(rootPath, hooksSubpath), RootPath: rootPath, - gitPath: gitPath, + GitPath: gitPath, }, nil } // StagedFiles returns a list of staged files // or an error if git command fails. func (r *Repository) StagedFiles() ([]string, error) { - return FilesByCommand(cmdStagedFiles) + return r.FilesByCommand(cmdStagedFiles) } // StagedFiles returns a list of all files in repository // or an error if git command fails. func (r *Repository) AllFiles() ([]string, error) { - return FilesByCommand(cmdAllFiles) + return r.FilesByCommand(cmdAllFiles) } // PushFiles returns a list of files that are ready to be pushed // or an error if git command fails. func (r *Repository) PushFiles() ([]string, error) { - return FilesByCommand(cmdPushFiles) -} - -func execGit(command string) (string, error) { - args := strings.Split(command, " ") - cmd := exec.Command(args[0], args[1:]...) - - out, err := cmd.CombinedOutput() - if err != nil { - return "", err - } - - return strings.TrimSpace(string(out)), nil + return r.FilesByCommand(cmdPushFiles) } // FilesByCommand accepts git command and returns its result as a list of filepaths. -func FilesByCommand(command string) ([]string, error) { +func (r *Repository) FilesByCommand(command string) ([]string, error) { var cmd *exec.Cmd if runtime.GOOS == "windows" { commandArg := strings.Split(command, " ") @@ -99,10 +90,10 @@ func FilesByCommand(command string) ([]string, error) { lines := strings.Split(string(outputBytes), "\n") - return extractFiles(lines) + return r.extractFiles(lines) } -func extractFiles(lines []string) ([]string, error) { +func (r *Repository) extractFiles(lines []string) ([]string, error) { var files []string for _, line := range lines { @@ -111,7 +102,7 @@ func extractFiles(lines []string) ([]string, error) { continue } - isFile, err := isFile(file) + isFile, err := r.isFile(file) if err != nil { return nil, err } @@ -123,8 +114,8 @@ func extractFiles(lines []string) ([]string, error) { return files, nil } -func isFile(path string) (bool, error) { - stat, err := os.Stat(path) +func (r *Repository) isFile(path string) (bool, error) { + stat, err := r.Fs.Stat(path) if err != nil { if os.IsNotExist(err) { return false, nil @@ -134,3 +125,15 @@ func isFile(path string) (bool, error) { return !stat.IsDir(), nil } + +func execGit(command string) (string, error) { + args := strings.Split(command, " ") + cmd := exec.Command(args[0], args[1:]...) + + out, err := cmd.CombinedOutput() + if err != nil { + return "", err + } + + return strings.TrimSpace(string(out)), nil +} diff --git a/internal/git/state.go b/internal/git/state.go index a076d81b..0ab96921 100644 --- a/internal/git/state.go +++ b/internal/git/state.go @@ -24,15 +24,15 @@ func (r *Repository) State() State { } func (r *Repository) isMergeState() bool { - if _, err := os.Stat(filepath.Join(r.gitPath, "MERGE_HEAD")); os.IsNotExist(err) { + if _, err := r.Fs.Stat(filepath.Join(r.GitPath, "MERGE_HEAD")); os.IsNotExist(err) { return false } return true } func (r *Repository) isRebaseState() bool { - if _, mergeErr := os.Stat(filepath.Join(r.gitPath, "rebase-merge")); os.IsNotExist(mergeErr) { - if _, applyErr := os.Stat(filepath.Join(r.gitPath, "rebase-apply")); os.IsNotExist(applyErr) { + if _, mergeErr := r.Fs.Stat(filepath.Join(r.GitPath, "rebase-merge")); os.IsNotExist(mergeErr) { + if _, applyErr := r.Fs.Stat(filepath.Join(r.GitPath, "rebase-apply")); os.IsNotExist(applyErr) { return false } } diff --git a/internal/lefthook/add_test.go b/internal/lefthook/add_test.go index bd9c26d7..84f544d9 100644 --- a/internal/lefthook/add_test.go +++ b/internal/lefthook/add_test.go @@ -23,11 +23,6 @@ func TestLefthookAdd(t *testing.T) { return filepath.Join(root, ".git", "hooks", hook) } - repo := &git.Repository{ - HooksPath: hooksPath, - RootPath: root, - } - for n, tt := range [...]struct { name string args *AddArgs @@ -131,7 +126,14 @@ source_dir_local: .source_dir_local } { t.Run(fmt.Sprintf("%d: %s", n, tt.name), func(t *testing.T) { fs := afero.NewMemMapFs() - lefthook := &Lefthook{Options: &Options{Fs: fs}, repo: repo} + lefthook := &Lefthook{ + Options: &Options{Fs: fs}, + repo: &git.Repository{ + Fs: fs, + HooksPath: hooksPath, + RootPath: root, + }, + } if len(tt.config) > 0 { err := afero.WriteFile(fs, configPath, []byte(tt.config), 0o644) diff --git a/internal/lefthook/install_test.go b/internal/lefthook/install_test.go index 7fdb8241..1770384f 100644 --- a/internal/lefthook/install_test.go +++ b/internal/lefthook/install_test.go @@ -24,11 +24,6 @@ func TestLefthookInstall(t *testing.T) { return filepath.Join(root, ".git", "hooks", hook) } - repo := &git.Repository{ - HooksPath: hooksPath, - RootPath: root, - } - for n, tt := range [...]struct { name, config string args InstallArgs @@ -214,7 +209,14 @@ post-commit: }, } { fs := afero.NewMemMapFs() - lefthook := &Lefthook{Options: &Options{Fs: fs}, repo: repo} + lefthook := &Lefthook{ + Options: &Options{Fs: fs}, + repo: &git.Repository{ + Fs: fs, + HooksPath: hooksPath, + RootPath: root, + }, + } t.Run(fmt.Sprintf("%d: %s", n, tt.name), func(t *testing.T) { // Create configuration file diff --git a/internal/lefthook/lefthook.go b/internal/lefthook/lefthook.go index 4174794c..7d03319c 100644 --- a/internal/lefthook/lefthook.go +++ b/internal/lefthook/lefthook.go @@ -41,7 +41,7 @@ func initialize(opts *Options) (*Lefthook, error) { log.SetColors(!opts.NoColors) - repo, err := git.NewRepository() + repo, err := git.NewRepository(opts.Fs) if err != nil { return nil, err } diff --git a/internal/lefthook/run_test.go b/internal/lefthook/run_test.go index 76c90aae..7784f9ab 100644 --- a/internal/lefthook/run_test.go +++ b/internal/lefthook/run_test.go @@ -18,16 +18,13 @@ func TestRun(t *testing.T) { configPath := filepath.Join(root, "lefthook.yml") hooksPath := filepath.Join(root, ".git", "hooks") - - repo := &git.Repository{ - HooksPath: hooksPath, - RootPath: root, - } + gitPath := filepath.Join(root, ".git") for i, tt := range [...]struct { name, hook, config string gitArgs []string envs map[string]string + existingDirs []string error bool }{ { @@ -82,10 +79,79 @@ pre-commit: `, error: false, }, + { + name: "When in git rebase-merge flow", + hook: "pre-commit", + config: ` +pre-commit: + parallel: false + piped: true + commands: + echo: + skip: + - rebase + - merge + run: echo 'SHOULD NEVER RUN' +`, + existingDirs: []string{ + filepath.Join(gitPath, "rebase-merge"), + }, + error: false, + }, + { + name: "When in git rebase-apply flow", + hook: "pre-commit", + config: ` +pre-commit: + parallel: false + piped: true + commands: + echo: + skip: + - rebase + - merge + run: echo 'SHOULD NEVER RUN' +`, + existingDirs: []string{ + filepath.Join(gitPath, "rebase-apply"), + }, + error: false, + }, + { + name: "When not in rebase flow", + hook: "pre-commit", + config: ` +pre-commit: + parallel: false + piped: true + commands: + echo: + skip: + - rebase + - merge + run: echo 'SHOULD RUN' +`, + error: true, + }, } { t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) { fs := afero.NewMemMapFs() - lefthook := &Lefthook{Options: &Options{Fs: fs}, repo: repo} + lefthook := &Lefthook{ + Options: &Options{Fs: fs}, + repo: &git.Repository{ + Fs: fs, + HooksPath: hooksPath, + RootPath: root, + GitPath: gitPath, + }, + } + + // Create files that should exist + for _, path := range tt.existingDirs { + if err := fs.MkdirAll(path, 0o755); err != nil { + t.Errorf("unexpected error: %s", err) + } + } err := afero.WriteFile(fs, configPath, []byte(tt.config), 0o644) if err != nil { diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/runner/runner.go index 3a6be1d8..130db25b 100644 --- a/internal/lefthook/runner/runner.go +++ b/internal/lefthook/runner/runner.go @@ -203,7 +203,7 @@ func (r *Runner) buildCommandArgs(command *config.Command) []string { config.PushFiles: r.repo.PushFiles, config.SubAllFiles: r.repo.AllFiles, config.SubFiles: func() ([]string, error) { - return git.FilesByCommand(filesCommand) + return r.repo.FilesByCommand(filesCommand) }, } diff --git a/internal/lefthook/uninstall_test.go b/internal/lefthook/uninstall_test.go index 4e401d9a..4acd596d 100644 --- a/internal/lefthook/uninstall_test.go +++ b/internal/lefthook/uninstall_test.go @@ -23,11 +23,6 @@ func TestLefthookUninstall(t *testing.T) { return filepath.Join(root, ".git", "hooks", hook) } - repo := &git.Repository{ - HooksPath: hooksPath, - RootPath: root, - } - for n, tt := range [...]struct { name, config string args UninstallArgs @@ -100,7 +95,14 @@ func TestLefthookUninstall(t *testing.T) { } { t.Run(fmt.Sprintf("%d: %s", n, tt.name), func(t *testing.T) { fs := afero.NewMemMapFs() - lefthook := &Lefthook{Options: &Options{Fs: fs}, repo: repo} + lefthook := &Lefthook{ + Options: &Options{Fs: fs}, + repo: &git.Repository{ + Fs: fs, + HooksPath: hooksPath, + RootPath: root, + }, + } err := afero.WriteFile(fs, configPath, []byte(tt.config), 0o644) if err != nil {