evilmartians / lefthook

Fast and powerful Git hooks manager for any type of projects.
MIT License
4.75k stars 211 forks source link

Documentation/Feature request: `git stash` on `pre-commit` and `pre-push` #131

Open 0xCLARITY opened 4 years ago

0xCLARITY commented 4 years ago

I love how fast lefthook is, but a problem I'm running into is that my hooks will sometimes fail locally even though they should pass, or pass locally but fail on our CI server because lefthook doesn't automatically stash uncommitted changes before running the hooks.

Is there an easy way to configure this manually? (I didn't see one in the examples) Ideally I'd still be able to have parallel: true, so wouldn't rely on an order of operations.

Arkweid commented 4 years ago

Hi @hbergren Take a look at this Discourse examples. Config: https://github.com/discourse/discourse/blob/master/lefthook.yml CI: https://github.com/discourse/discourse/blob/0872a1182d5702cf24bfce8958fc7cf12339c5ae/.travis.yml#L77

0xCLARITY commented 4 years ago

I see. So the only way to do this is to write custom linting scripts in lefthook.yml that only run on staged_files or some custom selection of files?

I was hoping for an easy way to stash uncommitted changes before and after a hook so that I could let my script functionality live in package.json and do something like this:

pre-push:
  parallel: true
  commands:
    eslint:
      tags: style
      run: npm run lintNoFix
      ...

where in package.json: "lintNoFix": "eslint . --ext .ts --max-warnings 0 && prettier --check '**/*.{md,json}'",.

But it sounds like the only solution as of right now is to write out something like this:

pre-push:
  parallel: true
  commands:
    prettier:
      tags: style
      files: git diff --name-only HEAD @{push}
      glob: '*.{md,json}'
      run: npx prettier {files}

    eslint:
      tags: style
      files: git diff --name-only HEAD @{push}
      glob: '*.ts'
      run: npx eslint --max-warnings 0 {files}

Is that correct?

Arkweid commented 4 years ago

Yea, for my point of view it very strange to commit something after auto-fixers without observing changes. A lot of them could broke your code or provide unexpected behaviour. That is why lefthook doesn't provide that behavior out of the box.

oldnote commented 1 month ago
Did not understand what you mean re > Yea, for my point of view it very strange to commit something after auto-fixers without observing changes. A lot of them could broke your code or provide unexpected behaviour. That is why lefthook doesn't provide that behavior out of the box.

Wanna add another example into this conversation.

Often I make a lot of changes in my golang project and then I construct separate commits from them. I want each commit to be a working application, so I've configured pre-commit hook to run unit tests before commit. When I commit a new test but forget to include some modules that this test depends on in the commit then I get false-positive pass from pre-commit hook since those files are available as non-staged changes:

pre-commit:
  commands:
    task-test:
      glob: "*_test.go"
      run: go test {staged_files}

I tried doing stashing manually:

pre-commit:
  commands:
    task-test:
      glob: "*_test.go"
      run: |
        git stash push -k -u -m "pre-commit stash"
        go test {staged_files}
        git stash pop

But it doesn't seem to do anything. Any ideas what I may be doing wrong?

oldnote commented 1 month ago

Found a working solution. Not sure about downsides, probably it doesn't run commands in parallel:

pre-commit:
  commands:
    git-stash:
      priority: 1
      run: git stash push -k -u  # Stash non-staged files

    task-test:
      priority: 2
      glob: "*_test.go"
      run: go test {staged_files}

    git-stash-pop:
      priority: 0  # Run last
      run: git stash pop
mrexox commented 1 month ago

@oldnote , lefthook must stash untracked files in pre-commit hook in latest versions. Do you have issues with stashing? What version do you use?

oldnote commented 1 month ago

@mrexox just updated to the latest version, yet no stashing by default

lefthook version - 1.7.12

lefthook.yml ```yml output: - summary - success - failure - skips pre-commit: commands: task-test: glob: "*_test.go" run: go test {staged_files} ```
How I perform testing 1. Given untracked files A.go, A_test.go and B.go where A.go depends on B.go (imports struct) 1. git add A.go A_test.go 1. git commit -m "wip" 1. Pre-commit runs without any error
OS Darwin Nikitas-MacBook-Air.local 23.5.0 Darwin Kernel Version 23.5.0: Wed May 1 20:19:05 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T8112 arm64

Using solution from my previous post - everything works fine (pre-commit fails as it should). Could it be I'm missing some settings in my .yml file or I need to run some lefthook commands after I edited my .yml file?

mrexox commented 1 month ago

Ok, thank you. I will try to reproduce this and will provide the feedback. There might be a bug.

mrexox commented 4 weeks ago

@oldnote , do you have the same lefthook version when it's run under git commit and installed globally? Please, check this by removing the output setting.

I tried to reproduce this but the pre-commit hook does not execute because {staged_files} get filtered by glob and contain only A_test.go

$ ls
A.go         A_test.go    B.go         lefthook.yml main.go

$ git status --short
 M A.go
 M A_test.go
 M B.go

$ git add A.go A_test.go
$ git status --short
M  A.go
M  A_test.go
M B.go

$ git commit -m 'test'
╭───────────────────────────────────────╮
│ 🥊 lefthook v1.7.11  hook: pre-commit │
╰───────────────────────────────────────╯
sync hooks: ✔️  (pre-commit)
┃  task-test ❯

# command-line-arguments [command-line-arguments.test]
./A_test.go:10:5: undefined: A
FAIL    command-line-arguments [build failed]
FAIL

  ────────────────────────────────────
summary: (done in 0.39 seconds)
🥊  task-test

Please, remove the output setting and add LEFTHOOK_VERBOSE=1 before git commit command and share the logs, so I could better understand what's the source of the problem.

oldnote commented 4 weeks ago

@mrexox

tried to reproduce this but the pre-commit hook does not execute because {staged_files} get filtered by glob and contain only A_test.go

I guess because in my case I'm testing by adding files into existing project 🤔.

There is the full output of my actions ```shell ➜ git status --short --untracked-files M lefthook.yml ?? app/testinglefthook/A.go ?? app/testinglefthook/A_test.go ?? app/testinglefthook/B.go ➜ cat lefthook.yml pre-commit: commands: task-test: glob: "*_test.go" run: go test {staged_files} ➜ cat app/testinglefthook/A.go package testinglefthook func DoA() B { return B{} } ➜ cat app/testinglefthook/A_test.go package testinglefthook_test import ( "finance-tracker/app/testinglefthook" "testing" ) func TestDoA(t *testing.T) { testinglefthook.DoA() } ➜ cat app/testinglefthook/B.go package testinglefthook type B struct{} ➜ git add app/testinglefthook/A.go app/testinglefthook/A_test.go ➜ LEFTHOOK_VERBOSE=1 git commit -m "test" + '[' '' = 0 ']' + call_lefthook run pre-commit + test -n '' + lefthook -h + lefthook run pre-commit │ [lefthook] cmd: [git rev-parse --show-toplevel] │ [lefthook] stdout: /Users/nikitavovcenko/Desktop/projects/study-go-finance-tracker │ [lefthook] cmd: [git rev-parse --git-path hooks] │ [lefthook] stdout: .git/hooks │ [lefthook] cmd: [git rev-parse --git-path info] │ [lefthook] stdout: .git/info │ [lefthook] cmd: [git rev-parse --git-dir] │ [lefthook] stdout: .git │ [lefthook] cmd: [git hash-object -t tree /dev/null] │ [lefthook] stdout: 4b825dc642cb6eb9a060e54bf8d69288fbee4904 ╭───────────────────────────────────────╮ │ 🥊 lefthook v1.7.12 hook: pre-commit │ ╰───────────────────────────────────────╯ │ [lefthook] cmd: [git status --short --porcelain] │ [lefthook] dir: /Users/nikitavovcenko/Desktop/projects/study-go-finance-tracker │ [lefthook] stdout: A app/testinglefthook/A.go A app/testinglefthook/A_test.go M lefthook.yml ?? app/testinglefthook/B.go │ [lefthook] cmd: [git diff --name-only --cached --diff-filter=ACMR] │ [lefthook] dir: /Users/nikitavovcenko/Desktop/projects/study-go-finance-tracker │ [lefthook] stdout: app/testinglefthook/A.go app/testinglefthook/A_test.go │ [lefthook] files before filters: [app/testinglefthook/A.go app/testinglefthook/A_test.go] │ [lefthook] files after filters: [app/testinglefthook/A_test.go] │ [lefthook] files after escaping: [app/testinglefthook/A_test.go] │ [lefthook] executing: go test app/testinglefthook/A_test.go ┃ task-test ❯ ok command-line-arguments 0.523s │ [lefthook] cmd: [git stash list] │ [lefthook] dir: /Users/nikitavovcenko/Desktop/projects/study-go-finance-tracker │ [lefthook] stdout: stash@{0}: WIP on main: 3c2d7b1 no-ref: Replace SQLite with postgres (add compose, upd tests) ──────────────────────────────────── summary: (done in 1.04 seconds) ✔️ task-test + '[' '' = 0 ']' + call_lefthook run prepare-commit-msg .git/COMMIT_EDITMSG message + test -n '' + lefthook -h + lefthook run prepare-commit-msg .git/COMMIT_EDITMSG message │ [lefthook] cmd: [git rev-parse --show-toplevel] │ [lefthook] stdout: /Users/nikitavovcenko/Desktop/projects/study-go-finance-tracker │ [lefthook] cmd: [git rev-parse --git-path hooks] │ [lefthook] stdout: .git/hooks │ [lefthook] cmd: [git rev-parse --git-path info] │ [lefthook] stdout: .git/info │ [lefthook] cmd: [git rev-parse --git-dir] │ [lefthook] stdout: .git │ [lefthook] cmd: [git hash-object -t tree /dev/null] │ [lefthook] stdout: 4b825dc642cb6eb9a060e54bf8d69288fbee4904 ╭───────────────────────────────────────────────╮ │ 🥊 lefthook v1.7.12 hook: prepare-commit-msg │ ╰───────────────────────────────────────────────╯ │ [lefthook] skip: Hook prepare-commit-msg doesn't exist in the config [main 48eb072] test 2 files changed, 15 insertions(+) create mode 100644 app/testinglefthook/A.go create mode 100644 app/testinglefthook/A_test.go ```
mrexox commented 4 weeks ago

Oh, I see, you have a dependency that's automatically included and lefthook does not hide the untracked files. Yes, this might be an issue. I will try to figure out how this could be fixed.