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 <mrexox@evilmartians.com>

* chore: Update initializations in tests

Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>

* chore(changelog): Update with PR #289

Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>

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

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
 			}
 		}

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
+}

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
 		}
 	}

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)

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

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
 	}

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 {

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)
 		},
 	}
 

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 {