diff --git a/CHANGELOG.md b/CHANGELOG.md index 90f7ad15..4a92b04e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## main (unreleased) +* Report the saved stash and recovery command when restoring repository state fails (#762). + * Add an opt-in `update` option to `BundleAudit` to refresh ruby-advisory-db before checking (#812). * Include command file contents in hook signatures when an absolute path is configured. diff --git a/lib/overcommit/hook_context/helpers/stash_unstaged_changes.rb b/lib/overcommit/hook_context/helpers/stash_unstaged_changes.rb index 2b805af9..9983e1ef 100644 --- a/lib/overcommit/hook_context/helpers/stash_unstaged_changes.rb +++ b/lib/overcommit/hook_context/helpers/stash_unstaged_changes.rb @@ -96,6 +96,7 @@ def stash_changes end @changes_stashed = `git stash list -1`.include?(stash_message) + @stash_commit = `git rev-parse stash@{0}`.strip if @changes_stashed end # Restore the staged tree explicitly: older Git versions' --keep-index @@ -106,7 +107,7 @@ def restore_staged_changes unless result.success? raise Overcommit::Exceptions::HookSetupFailed, "Unable to restore staged changes for #{hook_script_name} hook run:" \ - "\nSTDOUT:#{result.stdout}\nSTDERR:#{result.stderr}" + "\nSTDOUT:#{result.stdout}\nSTDERR:#{result.stderr}#{stash_recovery_message}" end end @@ -129,7 +130,7 @@ def clear_working_tree unless result.success? raise Overcommit::Exceptions::HookCleanupFailed, "Unable to cleanup working tree after #{hook_script_name} hooks run:" \ - "\nSTDOUT:#{result.stdout}\nSTDERR:#{result.stderr}" + "\nSTDOUT:#{result.stdout}\nSTDERR:#{result.stderr}#{stash_recovery_message}" end # Hard-resetting a staged submodule removal results in the index being @@ -149,9 +150,17 @@ def restore_working_tree unless result.success? raise Overcommit::Exceptions::HookCleanupFailed, "Unable to restore working tree after #{hook_script_name} hooks run:" \ - "\nSTDOUT:#{result.stdout}\nSTDERR:#{result.stderr}" + "\nSTDOUT:#{result.stdout}\nSTDERR:#{result.stderr}#{stash_recovery_message}" end end + + def stash_recovery_message + return '' unless @stash_commit + + "\nYour changes are preserved in Git stash #{@stash_commit}.\n" \ + "Release any file locks and use a clean working tree to recover on a new branch:\n" \ + " git stash branch overcommit-recovery #{@stash_commit}\n" + end end end end diff --git a/spec/integration/stash_recovery_spec.rb b/spec/integration/stash_recovery_spec.rb new file mode 100644 index 00000000..01fab7b4 --- /dev/null +++ b/spec/integration/stash_recovery_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'repository state after a configuration signature error' do + let(:enable_verification) { true } + + it 'preserves staged and unstaged changes without leaving a stash behind' do + repo do + echo("verify_signatures: true\nPreCommit:\n ALL:\n enabled: false", '.overcommit.yml') + echo('Original staged file', 'staged.txt') + echo('Original unstaged file', 'unstaged.txt') + shell(%w[git add .]).status.should == 0 + shell(%w[git commit -m Initial]).status.should == 0 + install_overcommit_hooks + shell(%w[overcommit --sign]).status.should == 0 + + echo('Staged change', 'staged.txt') + shell(%w[git add staged.txt]).status.should == 0 + echo('Unstaged change', 'unstaged.txt') + echo('concurrency: 4', '.overcommit.yml', append: true) + index = shell(%w[git write-tree]).stdout + diff = shell(%w[git diff --binary]).stdout + stashes = shell(%w[git stash list]).stdout + + result = shell(%w[git commit -m Test]) + + result.status.should_not == 0 + (result.stdout + result.stderr).should include 'Signature of configuration file has changed' + shell(%w[git write-tree]).stdout.should == index + shell(%w[git diff --binary]).stdout.should == diff + shell(%w[git stash list]).stdout.should == stashes + end + end +end diff --git a/spec/overcommit/hook_context/helpers/stash_unstaged_changes_spec.rb b/spec/overcommit/hook_context/helpers/stash_unstaged_changes_spec.rb index c3bfc08b..90f215a0 100644 --- a/spec/overcommit/hook_context/helpers/stash_unstaged_changes_spec.rb +++ b/spec/overcommit/hook_context/helpers/stash_unstaged_changes_spec.rb @@ -99,19 +99,40 @@ def emulate_legacy_stash end end - it 'preserves the stash for recovery if restoring the staged tree fails' do - state = repository_state - result = Overcommit::Subprocess::Result.new(128, '', 'fatal: unable to restore index') - allow(Overcommit::Utils).to receive(:execute).and_call_original - allow(Overcommit::Utils).to receive(:execute). - with(%w[git read-tree --reset -u stash@{0}^2]).and_return(result) - - expect { context.setup_environment }. - to raise_error(Overcommit::Exceptions::HookSetupFailed, /fatal: unable to restore index/) - - shell(%w[git stash list]).stdout.should include 'Overcommit: Stash of repo state' - shell(%w[git stash pop --index]).status.should == 0 - repository_state.should == state + { + 'restoring the staged tree' => %w[git read-tree --reset -u stash@{0}^2], + 'clearing the working tree' => %w[git reset --hard], + 'restoring the working tree' => %w[git stash pop --index], + }.each do |operation, command| + it "preserves the stash and explains recovery when #{operation} fails" do + state = repository_state + result = Overcommit::Subprocess::Result.new(128, '', 'fatal: file is locked') + allow(Overcommit::Utils).to receive(:execute).and_call_original + allow(Overcommit::Utils).to receive(:execute).with(command).and_return(result) + error = if operation == 'restoring the staged tree' + Overcommit::Exceptions::HookSetupFailed + else + Overcommit::Exceptions::HookCleanupFailed + end + + stash_commit = nil + expect do + context.setup_environment + context.cleanup_environment + end.to raise_error(error) { |failure| + stash_commit = shell(%w[git rev-parse stash@{0}]).stdout.strip + failure.message.should include 'fatal: file is locked' + failure.message.should include "git stash branch overcommit-recovery #{stash_commit}" + } + + # Release the simulated lock and follow the advertised recovery command. + allow(Overcommit::Utils).to receive(:execute).with(command).and_call_original + shell(%w[git reset --hard]).status.should == 0 + shell(['git', 'stash', 'branch', 'overcommit-recovery', stash_commit]).status.should == 0 + repository_state.reject { |key, _| key == :stashes }. + should == state.reject { |key, _| key == :stashes } + shell(%w[git stash list]).stdout.should include 'previous-stash' + end end it 'leaves the previous stash alone if there are no longer any changes to stash' do