introduce popen_with_error_handling to handle common user errors

(without tracebacks)

src/borg/archiver.py

@@ -63,6 +63,7 @@ from .helpers import ProgressIndicatorPercent
 from .helpers import basic_json_data, json_print
 from .helpers import replace_placeholders
 from .helpers import ChunkIteratorFileWrapper
+from .helpers import popen_with_error_handling
 from .patterns import ArgparsePatternAction, ArgparseExcludeFileAction, ArgparsePatternFileAction, parse_exclude_pattern
 from .patterns import PatternMatcher
 from .item import Item
@@ -747,9 +748,9 @@ class Archiver:
             # There is no deadlock potential here (the subprocess docs warn about this), because
             # communication with the process is a one-way road, i.e. the process can never block
             # for us to do something while we block on the process for something different.
-            filtercmd = shlex.split(filter)
-            logger.debug('--tar-filter command line: %s', filtercmd)
-            filterproc = subprocess.Popen(filtercmd, stdin=subprocess.PIPE, stdout=filterout)
+            filterproc = popen_with_error_handling(filter, stdin=subprocess.PIPE, stdout=filterout, log_prefix='--tar-filter: ')
+            if not filterproc:
+                return EXIT_ERROR
             # Always close the pipe, otherwise the filter process would not notice when we are done.
             tarstream = filterproc.stdin
             tarstream_close = True

src/borg/helpers.py

@@ -11,9 +11,11 @@ import os.path
 import platform
 import pwd
 import re
+import shlex
 import signal
 import socket
 import stat
+import subprocess
 import sys
 import textwrap
 import threading
@@ -1962,3 +1964,34 @@ def secure_erase(path):
         fd.flush()
         os.fsync(fd.fileno())
     os.unlink(path)
+
+
+def popen_with_error_handling(cmd_line: str, log_prefix='', **kwargs):
+    """
+    Handle typical errors raised by subprocess.Popen. Return None if an error occurred,
+    otherwise return the Popen object.
+
+    *cmd_line* is split using shlex (e.g. 'gzip -9' => ['gzip', '-9']).
+
+    Log messages will be prefixed with *log_prefix*; if set, it should end with a space
+    (e.g. log_prefix='--some-option: ').
+
+    Does not change the exit code.
+    """
+    assert not kwargs.get('shell'), 'Sorry pal, shell mode is a no-no'
+    try:
+        command = shlex.split(cmd_line)
+        if not command:
+            raise ValueError('an empty command line is not permitted')
+    except ValueError as ve:
+        logger.error('%s%s', log_prefix, ve)
+        return
+    logger.debug('%scommand line: %s', log_prefix, command)
+    try:
+        return subprocess.Popen(command, **kwargs)
+    except FileNotFoundError:
+        logger.error('%sexecutable not found: %s', log_prefix, command[0])
+        return
+    except PermissionError:
+        logger.error('%spermission denied: %s', log_prefix, command[0])
+        return

src/borg/testsuite/helpers.py

@@ -2,6 +2,7 @@ import argparse
 import hashlib
 import io
 import os
+import shutil
 import sys
 from datetime import datetime, timezone, timedelta
 from time import mktime, strptime, sleep
@@ -26,6 +27,7 @@ from ..helpers import ProgressIndicatorPercent, ProgressIndicatorEndless
 from ..helpers import swidth_slice
 from ..helpers import chunkit
 from ..helpers import safe_ns, safe_s, SUPPORT_32BIT_PLATFORMS
+from ..helpers import popen_with_error_handling
 
 from . import BaseTestCase, FakeInputs
 
@@ -816,3 +818,28 @@ def test_safe_timestamps():
             datetime.utcfromtimestamp(beyond_y10k)
         assert datetime.utcfromtimestamp(safe_s(beyond_y10k)) > datetime(2262, 1, 1)
         assert datetime.utcfromtimestamp(safe_ns(beyond_y10k) / 1000000000) > datetime(2262, 1, 1)
+
+
+class TestPopenWithErrorHandling:
+    @pytest.mark.skipif(not shutil.which('test'), reason='"test" binary is needed')
+    def test_simple(self):
+        proc = popen_with_error_handling('test 1')
+        assert proc.wait() == 0
+
+    @pytest.mark.skipif(shutil.which('borg-foobar-test-notexist'), reason='"borg-foobar-test-notexist" binary exists (somehow?)')
+    def test_not_found(self):
+        proc = popen_with_error_handling('borg-foobar-test-notexist 1234')
+        assert proc is None
+
+    @pytest.mark.parametrize('cmd', (
+            'mismatched "quote',
+            'foo --bar="baz',
+            ''
+    ))
+    def test_bad_syntax(self, cmd):
+        proc = popen_with_error_handling(cmd)
+        assert proc is None
+
+    def test_shell(self):
+        with pytest.raises(AssertionError):
+            popen_with_error_handling('', shell=True)