Fix for hook erroring with exit code 1 not being interpreted as an error (#214).

NEWS

@@ -3,6 +3,7 @@
  * #209: Bypass Borg error about a moved repository via "relocated_repo_access_is_ok" option in
    borgmatic storage configuration section.
  * #213: Reorder arguments passed to Borg to fix duplicate directories when using Borg patterns.
+ * #214: Fix for hook erroring with exit code 1 not being interpreted as an error.
 
 1.3.14
  * #204: Do not treat Borg warnings (exit code 1) as failures.

borgmatic/execute.py

@@ -32,7 +32,12 @@ def execute_and_log_output(full_command, output_log_level, shell):
         logger.log(output_log_level, remaining_output)
 
     exit_code = process.poll()
-    if exit_code >= BORG_ERROR_EXIT_CODE:
+
+    # If shell is True, assume we're running something other than Borg and should treat all non-zero
+    # exit codes as errors.
+    error = bool(exit_code != 0) if shell else bool(exit_code >= BORG_ERROR_EXIT_CODE)
+
+    if error:
         # If an error occurs, include its output in the raised exception so that we don't
         # inadvertently hide error output.
         if len(last_lines) == ERROR_OUTPUT_MAX_LINE_COUNT:

tests/integration/test_execute.py

@@ -32,6 +32,15 @@ def test_execute_and_log_output_includes_borg_error_output_in_exception():
     assert error.value.output
 
 
+def test_execute_and_log_output_with_shell_error_raises():
+    flexmock(module.logger).should_receive('log')
+
+    with pytest.raises(subprocess.CalledProcessError) as error:
+        module.execute_and_log_output(['false'], output_log_level=logging.INFO, shell=True)
+
+    assert error.value.returncode == 1
+
+
 def test_execute_and_log_output_truncates_long_borg_error_output():
     flexmock(module).ERROR_OUTPUT_MAX_LINE_COUNT = 0
     flexmock(module.logger).should_receive('log')