Pre-commit does not always automatically re-apply patch when stopped with Ctrl-C

[ian-h-chamberlain]

As mentioned in this issue:

if you can reproduce your findings where it doesn't re-apply I'd be more interested in that

Originally posted by @asottile in #2251 (comment)

---

I reproduced it once, although unfortunately I can't share the config since it is from a private repository in my organization, I do have a log which might provide a clue where this is happening.

In this case, the affected repo uses around 35 hooks, some custom and some from various upstreams. I was doing a git commit --amend but forgot to stage some changes, so I canceled to try a git commit --amend -a instead, when I hit the issue.

version information

pre-commit version: 2.17.0
git --version: git version 2.32.0
sys.version:
    3.6.8 (default, Dec  2 2020, 12:54:58) 
    [GCC Apple LLVM 12.0.0 (clang-1200.0.32.27)]
sys.executable: /Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/bin/python
os.name: posix
sys.platform: darwin

error information

Interrupted (^C): KeyboardInterrupt: 

Traceback (most recent call last):
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/lib/python3.6/site-packages/pre_commit/error_handler.py", line 70, in error_handler
    yield
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/lib/python3.6/site-packages/pre_commit/main.py", line 375, in main
    args=args.rest[1:],
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/lib/python3.6/site-packages/pre_commit/commands/hook_impl.py", line 237, in hook_impl
    return retv | run(config, store, ns)
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/lib/python3.6/site-packages/pre_commit/commands/run.py", line 398, in run
    exit_stack.enter_context(staged_files_only(store.directory))
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/lib/python3.6/contextlib.py", line 330, in enter_context
    result = _cm_type.__enter__(cm)
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/lib/python3.6/site-packages/pre_commit/staged_files_only.py", line 98, in staged_files_only
    with _intent_to_add_cleared(), _unstaged_changes_cleared(patch_dir):
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/lib/python3.6/site-packages/pre_commit/staged_files_only.py", line 67, in _unstaged_changes_cleared
    cmd_output_b(*_CHECKOUT_CMD, env=no_checkout_env)
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/envs/i95-py3/lib/python3.6/site-packages/pre_commit/util.py", line 150, in cmd_output_b
    stdout_b, stderr_b = proc.communicate()
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/lib/python3.6/subprocess.py", line 863, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/lib/python3.6/subprocess.py", line 1534, in _communicate
    ready = selector.select(timeout)
  File "/Users/ichamberlain/.pyenv/versions/3.6.8/lib/python3.6/selectors.py", line 376, in select
    fd_event_list = self._poll.poll(timeout)
KeyboardInterrupt

[asottile]

can you show the whole output?

[ian-h-chamberlain]

Sure, there's not a whole lot to show since I hit ctrl-c early on in the process:

$ git commit  --amend 
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /Users/ichamberlain/.cache/pre-commit/patch1647287582-64497.
^C⏎                                                                                                                                                                                                                                                                  
Interrupted (^C): KeyboardInterrupt: 
Check the log at /Users/ichamberlain/.cache/pre-commit/pre-commit.log

I was able to get back to a normal state just by git apply /Users/ichamberlain/.cache/pre-commit/patch1647287582-64497.

[asottile]

that's some crazy quick reaction time -- that's the first ~100ms of execution. I'm not sure there's too much that can be done since it's ms before it would have set up the finalizer

[ian-h-chamberlain]

Hmm, without knowing too much about the code or how it works but reading https://github.com/pre-commit/pre-commit/blob/master/pre_commit/staged_files_only.py#L69

Would it make any sense to do something like this?

# ... 
        no_checkout_env = dict(os.environ, _PRE_COMMIT_SKIP_POST_CHECKOUT='1')

        try:
	        cmd_output_b(*_CHECKOUT_CMD, env=no_checkout_env)
            yield
        finally:
            # Try to apply the patch we saved

# ...

I am guessing the subprocess to perform the git checkout (or waiting for communicate()) is probably the slowest part of this process? It seems like in my case the checkout did in fact happen, since the unstaged changes were not re-applied, so perhaps it's taking just long enough for git to get mostly done that I was able to catch it before it reached the try: block...

[asottile]

the problem with that is it would apply in cases where it was cancelled to early causing a different failure

[asottile]

I was able to reproduce with sleep insertion but not actual execution -- but in theory it could pause there so I moved the call inside -- hopefully that should fix this!

[ian-h-chamberlain]

Thanks for the fix! I will give the new version a try and report back if I still run into trouble.