Insure git hooks are ignored on checkout during conflicts

[TheSuperiorStanislav]

Issues arise when you have post-checkout hooks, and you have conflicts, and also updated .pre-commit-hooks.yaml file

plumbum.commands.processes.ProcessExecutionError: Unexpected exit code: 1
Command line: | /usr/bin/git checkout -- .github/settings.yaml
Stderr:       | [ERROR] Your pre-commit configuration is unstaged.
              | `git add .pre-commit-config.yaml` to fix this.

[pawamoy]

Can you explain your setup in more details please? Are you using pre-commit, the tool? Why does it prevent a git checkout -- file on an file unrelated to pre-commit config? Would git restore file bypass this check?

[TheSuperiorStanislav]

Can you explain your setup in more details please? Are you using pre-commit, the tool?

Yes

Why does it prevent a git checkout -- file on a file unrelated to pre-commit config? Would git restore file bypass this check?

It's similar to a test. I have hooks enabled for post-checkout (In the action project, it's a hook for poetry to sync deps). So it gets triggered by git checkout, but during sync, .pre-commit-hooks.yaml is updated, which pre-commit doesn't like.

[pawamoy]

OK, thanks. Could you try to see if git restore triggers the same post-checkout hook? That's the modern version of git checkout -- file.

[TheSuperiorStanislav]

OK, thanks. Could you try to see if git restore triggers the same post-checkout hook? That's the modern version of git checkout -- file.

Yes it triggers it

[pawamoy]

Thanks! I have to say I find it dumb though 🫠 Because clearly the intent here is to undo modifications in a file unrelated to pre-commit, not checking out another branch.

Erm, ChatGPT says:

From Git’s perspective, a checkout is any operation that updates the working tree from a tree object, not strictly a branch switch.

https://git-scm.com/docs/githooks

post-checkout

This hook is invoked when a git-checkout[1] or git-switch[1] is run after having updated the worktree. The hook is given three parameters: the ref of the previous HEAD, the ref of the new HEAD (which may or may not have changed), and a flag indicating whether the checkout was a branch checkout (changing branches, flag=1) or a file checkout (retrieving a file from the index, flag=0). This hook cannot affect the outcome of git switch or git checkout, other than that the hook’s exit status becomes the exit status of these two commands.

It is also run after git-clone[1], unless the --no-checkout (-n) option is used. The first parameter given to the hook is the null-ref, the second the ref of the new HEAD and the flag is always 1. Likewise for git worktree add unless --no-checkout is used.

This hook can be used to perform repository validity checks, auto-display differences from the previous HEAD if different, or set working dir metadata properties.

Maybe there's a way to check the flag on your side and do nothing when it's equal to 0 (file retrieved from index)? And/or when the old ref and new ref are the same.

I wonder if #2376 wouldn't fix this too.

But we might want to merge this as a quickfix in the meantime.

It's crazy how many things can get in the way when running Git operations programmatically 😄

[sisp]

I wonder, is it generally valid to ignore post-checkout hooks? AFAIK, for example Git LFS' smudge filter runs after checkout to resolve the LFS pointers and fetch the actual blobs from the LFS server. Not 100% sure whether that's relevant in this case though.

[TheSuperiorStanislav]

@sisp @pawamoy Is there anything that needs to be done to move this forward?

[sisp]

I wonder if #2376 wouldn't fix this too.

@pawamoy I imagine a similar problem would occur because there's also a git checkout call.

---

I wonder, is it generally valid to ignore post-checkout hooks? AFAIK, for example Git LFS' smudge filter runs after checkout to resolve the LFS pointers and fetch the actual blobs from the LFS server. Not 100% sure whether that's relevant in this case though.

I've read up on Git LFS, it uses a post-checkout hook for making locked files read-only in the working copy; this is probably irrelevant in our case. But I still wonder a bit about the general validity of ignoring all hooks here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.18%. Comparing base (3abf236) to head (c847a56).
⚠️ Report is 29 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2432      +/-   ##
==========================================
- Coverage   97.26%   97.18%   -0.08%     
==========================================
  Files          55       55              
  Lines        6288     6405     +117     
==========================================
+ Hits         6116     6225     +109     
- Misses        172      180       +8     

Flag	Coverage Δ
unittests	97.18% <100.00%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sisp]

LGTM! 🎉 Thanks for your contribution and patience with my recurring review suggestions. 🙇