gitui icon indicating copy to clipboard operation
gitui copied to clipboard

Pre-commit hooks are executed only when launched from the git root

Open savchenko opened this issue 1 year ago β€’ 2 comments

Describe the bug Pre-commit hook is ignored.

To Reproduce Steps to reproduce the behavior:

  1. mkdir .githooks && git config --local core.hooksPath .githooks
  2. Create .githooks/pre-commit with echo "Hook triggered" && exit 1 and chmod +x it.
  3. git commit now fails, as expected.
  4. gitui ignores the hook.

Expected behavior pre-commit behaviour is the same when using git CLI.

Screenshots N/A.

Context (please complete the following information):

  • OS/Distro + Version: Debian 12
  • GitUI Version: gitui 0.25.2
  • Rust version: rustc 1.76.0

Additional context Relates to:

  • https://github.com/extrawurst/gitui/pull/1054
  • (maybe) https://github.com/extrawurst/gitui/issues/1711

savchenko avatar Jun 28 '24 06:06 savchenko

It seems your repro steps are not correct: you say you create mkdir .githooks but then you create ./githooks/pre-commit (which is a subtle but very different folder name). if I fix this and create the pre-commit hook inside the path you configure: .githooks/pre-commit then gitui correct blocks the commit.

why in your case git rejects and gitui does not is beyond me

Screenshot 2024-06-28 at 09 12 51

extrawurst avatar Jun 28 '24 07:06 extrawurst

@extrawurst , a typo, fixed the original message. Git picks it up as it is configured to do so, in the local config:

[core]
    hooksPath = .githooks/

Turns out gitui correctly triggers the hook only when launched from the root folder (git rev-parse --show-toplevel).

Corresponding log record:

12:16:35 [TRACE] (1) git2_hooks::hookspath: [git2-hooks/src/hookspath.rs:114] run hook '".githooks/pre-commit"' in '"/home/user/foo/"'

And here is the successful attempt to commit the same file from one of the subfolders (e.g. /home/user/foo/src)

12:22:10 [ERROR] theme error ["/home/user/.config/gitui/theme.ron"]: 24:35: Failed to parse Colors: data did not match any variant of untagged enum ColorFormat
12:22:10 [TRACE] (1) gitui::app: [src/app.rs:154] open repo at: RefCell { value: Path(".") }
12:22:10 [TRACE] (6) mio::poll: [/home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mio-0.8.11/src/poll.rs:551] registering event source with poller: token=Token(0), interests=READABLE
12:22:10 [TRACE] (6) mio::poll: [/home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mio-0.8.11/src/poll.rs:551] registering event source with poller: token=Token(1), interests=READABLE
12:22:10 [TRACE] (1) asyncgit::status: [asyncgit/src/status.rs:102] request: [hash: 5275764462638972305] (type: WorkingDir)
12:22:10 [TRACE] (1) asyncgit::status: [asyncgit/src/status.rs:102] request: [hash: 4542120333235219920] (type: Stage)
12:22:10 [TRACE] (2) asyncgit::status: [asyncgit/src/status.rs:160] status fetched: 5275764462638972305 (type: WorkingDir)
12:22:10 [TRACE] (4) asyncgit::status: [asyncgit/src/status.rs:160] status fetched: 4542120333235219920 (type: Stage)
12:22:10 [TRACE] (1) gitui: [src/main.rs:215] app start: 21 ms
12:22:10 [TRACE] (1) gitui::app: [src/app.rs:375] update
12:22:10 [TRACE] (1) asyncgit::status: [asyncgit/src/status.rs:102] request: [hash: 7094696850961575051] (type: WorkingDir)
12:22:10 [TRACE] (1) asyncgit::status: [asyncgit/src/status.rs:102] request: [hash: 12783852074258472187] (type: Stage)
12:22:10 [TRACE] (2) asyncgit::status: [asyncgit/src/status.rs:160] status fetched: 7094696850961575051 (type: WorkingDir)
12:22:10 [TRACE] (4) asyncgit::status: [asyncgit/src/status.rs:160] status fetched: 12783852074258472187 (type: Stage)
12:22:10 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Status)
12:22:10 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Status)
12:22:10 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Status)
12:22:10 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Status)
12:22:11 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('w'), modifiers: KeyModifiers(0x0), kind: Press, state: KeyEventState(0x0) }))
12:22:11 [TRACE] (1) asyncgit::diff: [asyncgit/src/diff.rs:101] request DiffParams { path: "foo.py", diff_type: Stage, options: DiffOptions { ignore_whitespace: false, context: 3, interhunk_lines: 0 } }
12:22:11 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Diff)
12:22:11 [TRACE] (1) asyncgit::diff: [asyncgit/src/diff.rs:101] request DiffParams { path: "foo.py", diff_type: Stage, options: DiffOptions { ignore_whitespace: false, context: 3, interhunk_lines: 0 } }
12:22:11 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('c'), modifiers: KeyModifiers(0x0), kind: Press, state: KeyEventState(0x0) }))
12:22:11 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('R'), modifiers: KeyModifiers(SHIFT), kind: Press, state: KeyEventState(0x0) }))
12:22:12 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('o'), modifiers: KeyModifiers(0x0), kind: Press, state: KeyEventState(0x0) }))
12:22:12 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('g'), modifiers: KeyModifiers(0x0), kind: Press, state: KeyEventState(0x0) }))
12:22:12 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('u'), modifiers: KeyModifiers(0x0), kind: Press, state: KeyEventState(0x0) }))
12:22:12 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('e'), modifiers: KeyModifiers(0x0), kind: Press, state: KeyEventState(0x0) }))
12:22:12 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('!'), modifiers: KeyModifiers(0x0), kind: Press, state: KeyEventState(0x0) }))
12:22:14 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('d'), modifiers: KeyModifiers(CONTROL), kind: Press, state: KeyEventState(0x0) }))
12:22:14 [TRACE] (1) gitui::app: [src/app.rs:375] update
12:22:14 [TRACE] (1) asyncgit::diff: [asyncgit/src/diff.rs:101] request DiffParams { path: "foo.py", diff_type: Stage, options: DiffOptions { ignore_whitespace: false, context: 3, interhunk_lines: 0 } }
12:22:14 [TRACE] (1) asyncgit::status: [asyncgit/src/status.rs:102] request: [hash: 17309038260000894423] (type: WorkingDir)
12:22:14 [TRACE] (1) asyncgit::status: [asyncgit/src/status.rs:102] request: [hash: 9501808690652183810] (type: Stage)
12:22:14 [TRACE] (4) asyncgit::status: [asyncgit/src/status.rs:160] status fetched: 9501808690652183810 (type: Stage)
12:22:14 [TRACE] (5) asyncgit::status: [asyncgit/src/status.rs:160] status fetched: 17309038260000894423 (type: WorkingDir)
12:22:14 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Diff)
12:22:14 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Status)
12:22:14 [TRACE] (1) gitui::app: [src/app.rs:395] update_async: Git(Status)
12:22:14 [TRACE] (1) gitui::app: [src/app.rs:278] event: Input(Key(KeyEvent { code: Char('c'), modifiers: KeyModifiers(CONTROL), kind: Press, state: KeyEventState(0x0) }))

savchenko avatar Jun 28 '24 09:06 savchenko

+1, the pre-commit hooks are run only when gitui is launched from the git root.

saiabishek1 avatar Nov 10 '24 11:11 saiabishek1

I'd like to work on this if no one already is.

wessamfathi avatar Mar 15 '25 22:03 wessamfathi

I cannot reproduce this issue on my end, as gitui runs the pre-commit correctly whether I'm running it from repo root or sub folders. I'm testing on the gitui repo, and from gitui/src and gitui/src/keys, and it works correctly in all cases.

Using the nightly 2025-01-14 build, macOS Sequoia, rustc 1.85.0.

Is this still an issue for you @savchenko @saiabishek1? Am I following the reproduction steps correctly?

wessamfathi avatar Mar 15 '25 23:03 wessamfathi

@wessamfathi could you add a unit test for this in any case - we already have a few for hooks, should be easy to cover (in case we don’t have one maybe already for this subfolder thing)

extrawurst avatar Mar 15 '25 23:03 extrawurst

I checked the hook test and it appears that there is one that does that job already test_hooks_commit_msg_reject_in_subfolder: it creates a folder foo and attempts to commit from within. I verified that it passes successfully on macOS.

Let me know if you mean something else and I'll do it. I'll try to test on Windows as well, but I cannot get OpenSSL to build no matter what I do.

wessamfathi avatar Mar 16 '25 10:03 wessamfathi

Yeah I felt we had made a test for this already in the past. If you could get it tested on windows that would be awesome

extrawurst avatar Mar 16 '25 21:03 extrawurst

The test passes on Windows πŸ‘

wessamfathi avatar Mar 17 '25 10:03 wessamfathi

@savchenko ok will close it as non-reproducible for now. feel free to reopen if this is still a problem on the most recent release

extrawurst avatar Mar 17 '25 10:03 extrawurst

@extrawurst , I can reliably reproduce it with gitui v0.27 on Debian 13 (testing). What data would help you to pinpoint the issue?

savchenko avatar Mar 18 '25 15:03 savchenko

Hi @savchenko: I've just tried to reproduce this using a fresh debian 13 container from docker.io, gitui 0.27 downloaded from the releases and the steps to reproduce from your initial issue. My commit is rejected as expected:

Image

Could you potentially create a syscall trace for us?

strace -fo gitui.strace.log gitui

... then try to commit in your test repo and upload gitui.strace.log.

In case you don't want to publish such a long log, I suggest you create a private repository, upload it there and invite me and anybody else who's interested.

naseschwarz avatar Mar 18 '25 16:03 naseschwarz

@naseschwarz , reproduction steps:

  1. Create a repo
  2. Add to .githooks/pre-commit
#!/bin/env bash

set -euo pipefail

cd "$(git rev-parse --show-toplevel)" || exit 1

file_types=("txt")
flag="foo"
staged_files=$(git diff --cached --name-only --diff-filter=ACM)

is_excluded() {
    git check-ignore -q "$1"
    return $?
}

if [ -n "$staged_files" ]; then
    for type in "${file_types[@]}"; do
        files=$(echo "$staged_files" | grep "\.$type$" || echo "")
        if [ -z "$files" ]; then
            continue
        fi
        for file in $files; do
            if ! is_excluded "$file"; then
                if ! head -n 5 "$file" | grep -q "$flag"; then
                    echo "[ERROR] $file is missing the flag."
                    exit 1
                fi
            fi
        done
    done
fi
  1. git config --local core.hooksPath .githooks/
  2. echo bar > foo.txt && git add foo.txt
  3. git commit

The above will result in [ERROR] foo.txt is missing the flag. printed to the stdout.

  1. (important) mkdir -p some/long/path && cd some/long/path
  2. Open gitui, press w, then c, type arbitrary message, press Enter.

The commit will succeed. Please find the strace log attached.

gitui.strace.log.zip

savchenko avatar Mar 18 '25 18:03 savchenko

Thanks, I can reproduce this now.

naseschwarz avatar Mar 18 '25 20:03 naseschwarz

@wessamfathi, do you still want to take care of this? I don't want to do a hostile takeover here. ;)

However, I've put together a failing test: https://github.com/gitui-org/gitui/commit/d59f168b271204c2380704801d88e3f5f91bb23e.

The failure is due to a combination of two conditions, which was not obvious based on the original steps to reproduce, but is now:

  1. core.hooksPath is set to a relative path
  2. gitui is launched with a working directory that is not the working tree root.

naseschwarz avatar Mar 18 '25 20:03 naseschwarz

@naseschwarz oh no worries! Feel free to take it. You're already on track, and now it feels a little bit over my head :)

I'll find something else for sure.

wessamfathi avatar Mar 18 '25 21:03 wessamfathi