Gracefully handle failed cleanups

"Graceful" is a bit misleading here, as the best we can do is not ignore
that a problem occurred.

A problem that could occur is that if a file was placed in between `git
reset`ing the tree and applying the stash, the stash would silently fail
to apply.

We need to tell the user that something funky happened. While one
approach would be to "pause" and ask the user what to do, this is
exceptional enough that we're just going to fail loudly and present some
instructions for recovering.

Change-Id: I391574ffba861f94cc46ca74f51f640e5d913a5b
Reviewed-on: http://gerrit.causes.com/41194
Tested-by: jenkins <jenkins@causes.com>
Reviewed-by: Shane da Silva <shane.dasilva@brigade.com>

Author: sds

CHANGELOG.md

@@ -7,6 +7,9 @@
 * Fix edge case where hitting Ctrl-C after all pre-commit hooks had run
   but before the cleanup had finished would result in a lost working
   tree
+* Handle edge case where if a file was created in the working directory by a
+  separate process in between the working tree being reset and the stash being
+  applied, the hook runner would silently fail
 
 ## 0.15.0
 

lib/overcommit/exceptions.rb

@@ -8,6 +8,9 @@ class GitConfigError < StandardError; end
   # Raised when a {HookContext} is unable to setup the environment before a run.
   class HookSetupFailed < StandardError; end
 
+  # Raised when a {HookContext} is unable to clean the environment after a run.
+  class HookCleanupFailed < StandardError; end
+
   # Raised when a hook run was cancelled by the user.
   class HookCancelled < StandardError; end
 

lib/overcommit/hook_context/pre_commit.rb

@@ -42,11 +42,11 @@ def setup_environment
     # as if nothing ever changed.
     def cleanup_environment
       unless initial_commit? || (@stash_attempted && !@changes_stashed)
-        `git reset --hard &> /dev/null` # Ensure working tree is clean before popping stash
+        clear_working_tree # Ensure working tree is clean before restoring it
       end
 
       if @changes_stashed
-        `git stash pop --index --quiet`
+        restore_working_tree
       end
 
       Overcommit::GitRepo.restore_merge_state
@@ -69,6 +69,26 @@ def modified_lines(file)
 
   private
 
+    # Clears the working tree so that the stash can be applied.
+    def clear_working_tree
+      result = Overcommit::Utils.execute(%w[git reset --hard])
+      unless result.success?
+        raise Overcommit::Exceptions::HookCleanupFailed,
+              "Unable to cleanup working tree after #{hook_script_name} hooks run:" \
+              "\n#{result.stderr}"
+      end
+    end
+
+    # Applies the stash to the working tree to restore the user's state.
+    def restore_working_tree
+      result = Overcommit::Utils.execute(%w[git stash pop --index --quiet])
+      unless result.success?
+        raise Overcommit::Exceptions::HookCleanupFailed,
+              "Unable to restore working tree after #{hook_script_name} hooks run:" \
+              "\n#{result.stderr}"
+      end
+    end
+
     # Returns whether there are any changes to the working tree, staged or
     # otherwise.
     def any_changes?

template-dir/hooks/overcommit-hook

@@ -78,8 +78,9 @@ rescue Overcommit::Exceptions::HookContextLoadError => error
   puts error
   puts 'Are you running an old version of Overcommit?'
   exit 69 # EX_UNAVAILABLE
-rescue Overcommit::Exceptions::HookSetupFailed => error
-  puts error
+rescue Overcommit::Exceptions::HookSetupFailed,
+       Overcommit::Exceptions::HookCleanupFailed => error
+  puts error.message
   exit 74 # EX_IOERR
 rescue Overcommit::Exceptions::HookCancelled
   puts 'You cancelled the hook run'