Trouble with magit and git pre-commit hooks

So, I recently realized that the git pre-commit hook does **not** ensure that only the chunks in the index are part of the worktree when the pre-commit hook runs. Hence, I set out and add some `git stash`ing to my pre-commit hook to fix that myself. This is what I ended up with after some google-fu: ```sh #!/bin/sh # First, stash index and work dir, keeping only the # to-be-committed changes in the working directory. old_stash=$(git rev-parse -q --verify refs/stash) git stash push -q --keep-index --include-untracked new_stash=$(git rev-parse -q --verify refs/stash) # If there were no changes (e.g., `--amend` or `--allow-empty`) # then nothing was stashed, and we should skip everything, # including the tests themselves. (Presumably the tests passed # on the previous commit, so there is no need to re-run them.) if [ "$old_stash" = "$new_stash" ]; then echo "pre-commit script: no changes to test" exit 0 fi # Run lints/checks/tests/... script/check st=$? echo "got status $st" # Restore changes git reset --hard -q git stash pop --index -q # Exit with status from test-run: nonzero prevents commit exit $st ``` Now, this works fine when I use it from the git-command line, however, I run into trouble when I commit via magit. It fails with the following error: ``` error: pathspec ':/' did not match any file(s) known to git < normal testing output> error: could not restore untracked files from stash The stash entry is kept in case you need it again. ``` Anyone have any idea what is causing the difference between git-cli and magit here? Edit: after some more google-fu I found this [github thread](https://github.com/magit/magit/issues/3419) pointing out that you need to `unset GIT_LITERAL_PATHSPECS` at the start of the script, since git sets this variable for itself when magit uses the `--literal-pathspecs` option. It all works as it should now. Edit2: this is the final version with suggestions applied: ```bash #!/usr/bin/env bash set -xuo pipefail # required by magit unset GIT_LITERAL_PATHSPECS # First, stash index and work dir, keeping only the # to-be-committed changes in the working directory. old_stash=$(git rev-parse -q --verify refs/stash) git stash push -q --keep-index --include-untracked new_stash=$(git rev-parse -q --verify refs/stash) # If there were no changes (e.g., `--amend` or `--allow-empty`) # then nothing was stashed, and we should skip everything, # including the tests themselves. (Presumably the tests passed # on the previous commit, so there is no need to re-run them.) if [ "$old_stash" = "$new_stash" ]; then echo "pre-commit script: no changes to test" exit 0 fi # Run lints/checks/tests/... script/check st=$? # Restore changes git reset --hard -q && git stash pop --index -q # Exit with status from test-run: nonzero prevents commit exit $st ```

Add `set -xeuo pipefail` to the top of your script and run again. You'll get verbose logs of everything the script does, and it'll stop immediately if any command results in an error, instead of chugging along (the default). Once you've fixed it, remove the `x` — but leave the rest, and include it in every shell script you writer, forever. You'll thank me.

> I recently realized that [my] git pre-commit hook does not ensure that only the chunks in the index are part of the worktree Firstly, well done for even realising your hook was broken -- I once did a search for git pre-commit hooks for the specific purpose of seeing whether programmers were typically dealing with this, and out of dozens of examples I found only *one* that got it right. I came away with the firm impression that the majority of people writing pre-commit hooks are not testing what they think they're testing (unless they don't care at all about grooming commits, in which case maybe they are -- but only by accident). Stashing is one option (and how that one other script I found was doing it), and if your pre-commit hook needs to see the entire codebase then it's *probably* what you're stuck with for practical purposes. If your hook only needs the individual files being committed, however, then the approach I use is `git checkout-index -f --prefix="/path/to/dir/" "FILE"` after which I can perform my tests on `/path/to/dir/FILE` without touching the working copy at all, thus side-stepping any potential stash-related problems. Automated stash/pop processes make me nervous in general, because data loss is obviously a possibility, so I tend to avoid them. If you do stick with stashing, I advise being *extremely* rigorous about checking the exit status of every command, and probably be wary of `git stash pop` vs a `git stash apply` + `git stash drop` pairing (the latter allowing you to check assumptions before dropping the reference to the stashed changes). (You *can* recover such references if they haven't been garbage-collected by git yet, but it's a pain, so also consider logging all the relevant references for ease of recovery in case something unexpected happens.)

Note (because the quoted script uses `sh`) that `-o pipefail` is a `bash` thing.

It was `sh` since that is the default for git commit hooks (I believe at least?), so switching to `bash` instead is not a big deal.

Thanks for the tip, I switched to bash and used those options and it was quite helpful. I removed `e` because I still want the script to reset the worktree and index when the checks fail.

Unfortunately, the check I wanted to run (`cargo clippy`) runs on the whole project, so this approach was necessary. Otherwise it'd be a lot cleaner to just directly look at the index. I also believe that the pre-commit hook gets the filepaths in the index as arguments (don't quote me on this, I don't use this in my hook so I'm not certain), so that would be another way to check the specific files. I don't quite see the use of `stash apply` + `stash drop` vs just `stash pop`, since the latter will also retain the stash in case something goes wrong in the process of `pop`ping (something I've tested extensively now during this debug session).

You're probably right -- if you're always popping onto the exact same state that you had when you stashed then I can't think why it would fail. I simply experienced enough issues with `git stash pop` over the years that I stopped using it at all; but those issues were almost definitely in different circumstances. I'd still use Maximum Paranoia mode when writing the code, though. Even if you can't think of a reason for it to fail, it's a good idea to make it easy to recover if it does, just in case there was a scenario you hadn't considered.