Repository: borgbackup/borg

Issue:
highlander options "special effects"
borg has some cli options with `action=Highlander`, which raises an error if there is more than one (see also #6269).

E.g. it does not make sense to give `--umask` more than once (especially not with different values).

But, this error is not raised, if the first of these options happens to use the same value as the default value of the option.

E.g.:
```
borg --umask 077 --umask 007 rlist --repo ...  # no error (0o077 is the default)
borg --umask 007 --umask 077 rlist --repo ...  # error "there can be only one"
borg --umask 777 --umask 007 rlist --repo ...  # error "there can be only one"
borg --umask 007 --umask 777 rlist --repo ...  # error "there can be only one"
```

Discussion:
ThomasWaldmann: @snsmac seen my feedback? I restarted the windows CI, guess that failure wasn't related to your PR.
codecov-commenter: ## [Codecov](https://codecov.io/gh/borgbackup/borg/pull/7517?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup) Report
> Merging [#7517](https://codecov.io/gh/borgbackup/borg/pull/7517?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup) (922f207) into [master](https://codecov.io/gh/borgbackup/borg/commit/1e1c92229807f7522a2f7a6aa4bba3bfe46ddbaf?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup) (1e1c922) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 922f207 differs from pull request most recent head ba64122. Consider uploading reports for the commit ba64122 to get more accurate results

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup)

```diff
@@            Coverage Diff             @@
##           master    #7517      +/-   ##
==========================================
+ Coverage   83.87%   83.89%   +0.02%     
==========================================
  Files          67       67              
  Lines       11817    11821       +4     
  Branches     2155     2155              
==========================================
+ Hits         9911     9917       +6     
+ Misses       1332     1331       -1     
+ Partials      574      573       -1     
```


| [Impacted Files](https://codecov.io/gh/borgbackup/borg/pull/7517?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup) | Coverage Δ | |
|---|---|---|
| [src/borg/archiver/\_common.py](https://codecov.io/gh/borgbackup/borg/pull/7517?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup#diff-c3JjL2JvcmcvYXJjaGl2ZXIvX2NvbW1vbi5weQ==) | `91.00% <100.00%> (+1.27%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=borgbackup)

snsmac: @ThomasWaldmann Updated
ThomasWaldmann: src/borg/testsuite/archiver/argparsing.py:2:1: F401 'os.path' imported but unused


Patch:
diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py
index 51423abd..724afe08 100644
--- a/src/borg/archiver/__init__.py
+++ b/src/borg/archiver/__init__.py
@@ -564,29 +564,6 @@ def sig_trace_handler(sig_no, stack):  # pragma: no cover
     faulthandler.dump_traceback()
 
 
-def format_tb(exc):
-    qualname = type(exc).__qualname__
-    remote = isinstance(exc, RemoteRepository.RPCError)
-    if remote:
-        prefix = "Borg server: "
-        trace_back = "\n".join(prefix + l for l in exc.exception_full.splitlines())
-        sys_info = "\n".join(prefix + l for l in exc.sysinfo.splitlines())
-    else:
-        trace_back = traceback.format_exc()
-        sys_info = sysinfo()
-    result = f"""
-Error:
-
-{qualname}: {exc}
-
-If reporting bugs, please include the following:
-
-{trace_back}
-{sys_info}
-"""
-    return result
-
-
 def main():  # pragma: no cover
     # Make sure stdout and stderr have errors='replace' to avoid unicode
     # issues when print()-ing unicode file names
@@ -617,11 +594,12 @@ def main():  # pragma: no cover
         try:
             args = archiver.get_args(sys.argv, os.environ.get("SSH_ORIGINAL_COMMAND"))
         except Error as e:
-            # we might not have logging setup yet, so get out quickly
             msg = e.get_message()
+            tb_log_level = logging.ERROR if e.traceback else logging.DEBUG
+            tb = f"{traceback.format_exc()}\n{sysinfo()}"
+            # we might not have logging setup yet, so get out quickly
             print(msg, file=sys.stderr)
-            if e.traceback:
-                tb = format_tb(e)
+            if tb_log_level == logging.ERROR:
                 print(tb, file=sys.stderr)
             sys.exit(e.exit_code)
         try:
@@ -631,37 +609,39 @@ def main():  # pragma: no cover
             msg = e.get_message()
             msgid = type(e).__qualname__
             tb_log_level = logging.ERROR if e.traceback else logging.DEBUG
-            tb = format_tb(e)
+            tb = f"{traceback.format_exc()}\n{sysinfo()}"
             exit_code = e.exit_code
         except RemoteRepository.RPCError as e:
             important = e.exception_class not in ("LockTimeout",) and e.traceback
-            msg = e.exception_full if important else e.get_message()
             msgid = e.exception_class
             tb_log_level = logging.ERROR if important else logging.DEBUG
-            tb = format_tb(e)
+            if important:
+                msg = e.exception_full
+            else:
+                msg = e.get_message()
+            tb = "\n".join("Borg server: " + l for l in e.sysinfo.splitlines())
+            tb += "\n" + sysinfo()
             exit_code = EXIT_ERROR
-        except Exception as e:
+        except Exception:
             msg = "Local Exception"
             msgid = "Exception"
             tb_log_level = logging.ERROR
-            tb = format_tb(e)
+            tb = f"{traceback.format_exc()}\n{sysinfo()}"
             exit_code = EXIT_ERROR
-        except KeyboardInterrupt as e:
+        except KeyboardInterrupt:
             msg = "Keyboard interrupt"
             tb_log_level = logging.DEBUG
-            tb = format_tb(e)
+            tb = f"{traceback.format_exc()}\n{sysinfo()}"
             exit_code = EXIT_SIGNAL_BASE + 2
-        except SigTerm as e:
+        except SigTerm:
             msg = "Received SIGTERM"
             msgid = "Signal.SIGTERM"
             tb_log_level = logging.DEBUG
-            tb = format_tb(e)
+            tb = f"{traceback.format_exc()}\n{sysinfo()}"
             exit_code = EXIT_SIGNAL_BASE + 15
-        except SigHup as e:
+        except SigHup:
             msg = "Received SIGHUP."
             msgid = "Signal.SIGHUP"
-            tb_log_level = logging.DEBUG
-            tb = format_tb(e)
             exit_code = EXIT_SIGNAL_BASE + 1
         if msg:
             logger.error(msg, msgid=msgid)
diff --git a/src/borg/archiver/_common.py b/src/borg/archiver/_common.py
index 00729e22..38226e82 100644
--- a/src/borg/archiver/_common.py
+++ b/src/borg/archiver/_common.py
@@ -249,9 +249,14 @@ def wrapper(self, args, repository, manifest, **kwargs):
 class Highlander(argparse.Action):
     """make sure some option is only given once"""
 
+    def __init__(self, *args, **kwargs):
+        self.__called = False
+        super().__init__(*args, **kwargs)
+
     def __call__(self, parser, namespace, values, option_string=None):
-        if getattr(namespace, self.dest, None) != self.default:
+        if self.__called:
             raise argparse.ArgumentError(self, "There can be only one.")
+        self.__called = True
         setattr(namespace, self.dest, values)
 
 
diff --git a/src/borg/compress.pyx b/src/borg/compress.pyx
index e259c374..e120f133 100644
--- a/src/borg/compress.pyx
+++ b/src/borg/compress.pyx
@@ -15,7 +15,6 @@ which compressor has been used to compress the data and dispatch to the correct
 decompressor.
 """
 
-from argparse import ArgumentTypeError
 import random
 from struct import Struct
 import zlib
@@ -653,7 +652,7 @@ class CompressionSpec:
         values = s.split(',')
         count = len(values)
         if count < 1:
-            raise ArgumentTypeError("not enough arguments")
+            raise ValueError
         # --compression algo[,level]
         self.name = values[0]
         if self.name in ('none', 'lz4', ):
@@ -664,9 +663,9 @@ class CompressionSpec:
             elif count == 2:
                 level = int(values[1])
                 if not 0 <= level <= 9:
-                    raise ArgumentTypeError("level must be >= 0 and <= 9")
+                    raise ValueError
             else:
-                raise ArgumentTypeError("too many arguments")
+                raise ValueError
             self.level = level
         elif self.name in ('zstd', ):
             if count < 2:
@@ -674,28 +673,28 @@ class CompressionSpec:
             elif count == 2:
                 level = int(values[1])
                 if not 1 <= level <= 22:
-                    raise ArgumentTypeError("level must be >= 1 and <= 22")
+                    raise ValueError
             else:
-                raise ArgumentTypeError("too many arguments")
+                raise ValueError
             self.level = level
         elif self.name == 'auto':
             if 2 <= count <= 3:
                 compression = ','.join(values[1:])
             else:
-                raise ArgumentTypeError("bad arguments")
+                raise ValueError
             self.inner = CompressionSpec(compression)
         elif self.name == 'obfuscate':
             if 3 <= count <= 5:
                 level = int(values[1])
                 if not ((1 <= level <= 6) or (110 <= level <= 123)):
-                    raise ArgumentTypeError("level must be >= 1 and <= 6 or >= 110 and <= 123")
+                    raise ValueError
                 self.level = level
                 compression = ','.join(values[2:])
             else:
-                raise ArgumentTypeError("bad arguments")
+                raise ValueError
             self.inner = CompressionSpec(compression)
         else:
-            raise ArgumentTypeError("unsupported compression type")
+            raise ValueError
 
     @property
     def compressor(self):
diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py
index ade24241..85dc0178 100644
--- a/src/borg/helpers/parseformat.py
+++ b/src/borg/helpers/parseformat.py
@@ -137,7 +137,7 @@ def ChunkerParams(s):
     params = s.strip().split(",")
     count = len(params)
     if count == 0:
-        raise argparse.ArgumentTypeError("no chunker params given")
+        raise ValueError("no chunker params given")
     algo = params[0].lower()
     if algo == CH_FAIL and count == 3:
         block_size = int(params[1])
@@ -152,11 +152,9 @@ def ChunkerParams(s):
             # or in-memory chunk management.
             # choose the block (chunk) size wisely: if you have a lot of data and you cut
             # it into very small chunks, you are asking for trouble!
-            raise argparse.ArgumentTypeError("block_size must not be less than 64 Bytes")
+            raise ValueError("block_size must not be less than 64 Bytes")
         if block_size > MAX_DATA_SIZE or header_size > MAX_DATA_SIZE:
-            raise argparse.ArgumentTypeError(
-                "block_size and header_size must not exceed MAX_DATA_SIZE [%d]" % MAX_DATA_SIZE
-            )
+            raise ValueError("block_size and header_size must not exceed MAX_DATA_SIZE [%d]" % MAX_DATA_SIZE)
         return algo, block_size, header_size
     if algo == "default" and count == 1:  # default
         return CHUNKER_PARAMS
@@ -164,18 +162,14 @@ def ChunkerParams(s):
     if algo == CH_BUZHASH and count == 5 or count == 4:  # [buzhash, ]chunk_min, chunk_max, chunk_mask, window_size
         chunk_min, chunk_max, chunk_mask, window_size = (int(p) for p in params[count - 4 :])
         if not (chunk_min <= chunk_mask <= chunk_max):
-            raise argparse.ArgumentTypeError("required: chunk_min <= chunk_mask <= chunk_max")
+            raise ValueError("required: chunk_min <= chunk_mask <= chunk_max")
         if chunk_min < 6:
             # see comment in 'fixed' algo check
-            raise argparse.ArgumentTypeError(
-                "min. chunk size exponent must not be less than 6 (2^6 = 64B min. chunk size)"
-            )
+            raise ValueError("min. chunk size exponent must not be less than 6 (2^6 = 64B min. chunk size)")
         if chunk_max > 23:
-            raise argparse.ArgumentTypeError(
-                "max. chunk size exponent must not be more than 23 (2^23 = 8MiB max. chunk size)"
-            )
+            raise ValueError("max. chunk size exponent must not be more than 23 (2^23 = 8MiB max. chunk size)")
         return CH_BUZHASH, chunk_min, chunk_max, chunk_mask, window_size
-    raise argparse.ArgumentTypeError("invalid chunker params")
+    raise ValueError("invalid chunker params")
 
 
 def FilesCacheMode(s):
@@ -183,13 +177,11 @@ def FilesCacheMode(s):
     VALID_MODES = ("cis", "ims", "cs", "ms", "cr", "mr", "d", "s")  # letters in alpha order
     entries = set(s.strip().split(","))
     if not entries <= set(ENTRIES_MAP):
-        raise argparse.ArgumentTypeError(
-            "cache mode must be a comma-separated list of: %s" % ",".join(sorted(ENTRIES_MAP))
-        )
+        raise ValueError("cache mode must be a comma-separated list of: %s" % ",".join(sorted(ENTRIES_MAP)))
     short_entries = {ENTRIES_MAP[entry] for entry in entries}
     mode = "".join(sorted(short_entries))
     if mode not in VALID_MODES:
-        raise argparse.ArgumentTypeError("cache mode short must be one of: %s" % ",".join(VALID_MODES))
+        raise ValueError("cache mode short must be one of: %s" % ",".join(VALID_MODES))
     return mode
 
 
@@ -285,7 +277,7 @@ def SortBySpec(text):
 
     for token in text.split(","):
         if token not in AI_HUMAN_SORT_KEYS:
-            raise argparse.ArgumentTypeError("Invalid sort key: %s" % token)
+            raise ValueError("Invalid sort key: %s" % token)
     return text.replace("timestamp", "ts")
 
 


Test patch:
diff --git a/src/borg/testsuite/archiver/argparsing.py b/src/borg/testsuite/archiver/argparsing.py
index 14859237..1ce41122 100644
--- a/src/borg/testsuite/archiver/argparsing.py
+++ b/src/borg/testsuite/archiver/argparsing.py
@@ -11,6 +11,23 @@ def test_bad_filters(self):
         self.cmd(f"--repo={self.repository_location}", "create", "test", "input")
         self.cmd(f"--repo={self.repository_location}", "delete", "--first", "1", "--last", "1", fork=True, exit_code=2)
 
+    def test_highlander(self):
+        self.cmd(f"--repo={self.repository_location}", "rcreate", RK_ENCRYPTION)
+        self.cmd(f"--repo={self.repository_location}", "create", "--comment", "comment 1", "test-1", __file__)
+        error_msg = "There can be only one"
+        # Default umask value is 0077
+        # Test that it works with a one time specified default or custom value
+        output_default = self.cmd(f"--repo={self.repository_location}", "--umask", "0077", "rlist")
+        assert error_msg not in output_default
+        output_custom = self.cmd(f"--repo={self.repository_location}", "--umask", "0007", "rlist")
+        assert error_msg not in output_custom
+        # Test that all combinations of custom and default values fail
+        for first, second in [("0007", "0007"), ("0007", "0077"), ("0077", "0007"), ("0077", "0077")]:
+            output_custom = self.cmd(
+                f"--repo={self.repository_location}", "--umask", first, "--umask", second, "rlist", exit_code=2
+            )
+            assert error_msg in output_custom
+
 
 def test_get_args():
     archiver = Archiver()
diff --git a/src/borg/testsuite/compress.py b/src/borg/testsuite/compress.py
index fe951dc5..49819145 100644
--- a/src/borg/testsuite/compress.py
+++ b/src/borg/testsuite/compress.py
@@ -1,4 +1,3 @@
-import argparse
 import os
 import zlib
 
@@ -227,7 +226,7 @@ def test_obfuscate_meta():
 
 
 def test_compression_specs():
-    with pytest.raises(argparse.ArgumentTypeError):
+    with pytest.raises(ValueError):
         CompressionSpec("")
 
     assert isinstance(CompressionSpec("none").compressor, CNONE)
@@ -242,7 +241,7 @@ def test_compression_specs():
     zlib = CompressionSpec("zlib,9").compressor
     assert isinstance(zlib, ZLIB)
     assert zlib.level == 9
-    with pytest.raises(argparse.ArgumentTypeError):
+    with pytest.raises(ValueError):
         CompressionSpec("zlib,9,invalid")
 
     lzma = CompressionSpec("lzma").compressor
@@ -265,7 +264,7 @@ def test_compression_specs():
     assert isinstance(zstd, ZSTD)
     assert zstd.level == 22
 
-    with pytest.raises(argparse.ArgumentTypeError):
+    with pytest.raises(ValueError):
         CompressionSpec("lzma,9,invalid")
-    with pytest.raises(argparse.ArgumentTypeError):
+    with pytest.raises(ValueError):
         CompressionSpec("invalid")
diff --git a/src/borg/testsuite/helpers.py b/src/borg/testsuite/helpers.py
index bfb4f5c3..df9b2ea2 100644
--- a/src/borg/testsuite/helpers.py
+++ b/src/borg/testsuite/helpers.py
@@ -368,19 +368,19 @@ def test_chunkerparams():
     assert ChunkerParams("fixed,4096") == ("fixed", 4096, 0)
     assert ChunkerParams("fixed,4096,200") == ("fixed", 4096, 200)
     # invalid values checking
-    with pytest.raises(ArgumentTypeError):
+    with pytest.raises(ValueError):
         ChunkerParams("crap,1,2,3,4")  # invalid algo
-    with pytest.raises(ArgumentTypeError):
+    with pytest.raises(ValueError):
         ChunkerParams("buzhash,5,7,6,4095")  # too small min. size
-    with pytest.raises(ArgumentTypeError):
+    with pytest.raises(ValueError):
         ChunkerParams("buzhash,19,24,21,4095")  # too big max. size
-    with pytest.raises(ArgumentTypeError):
+    with pytest.raises(ValueError):
         ChunkerParams("buzhash,23,19,21,4095")  # violates min <= mask <= max
-    with pytest.raises(ArgumentTypeError):
+    with pytest.raises(ValueError):
         ChunkerParams("fixed,63")  # too small block size
-    with pytest.raises(ArgumentTypeError):
+    with pytest.raises(ValueError):
         ChunkerParams("fixed,%d,%d" % (MAX_DATA_SIZE + 1, 4096))  # too big block size
-    with pytest.raises(ArgumentTypeError):
+    with pytest.raises(ValueError):
         ChunkerParams("fixed,%d,%d" % (4096, MAX_DATA_SIZE + 1))  # too big header size