enhance tag_validator, fixes #9466

src/borg/archiver/create_cmd.py

@@ -211,14 +211,7 @@ def create_inner(archive, cache, fso):
                     # do not save the archive if the user ctrl-c-ed.
                     raise Error("Got Ctrl-C / SIGINT.")
                 else:
-                    # deal with tags
-                    if args.tags is not None:
-                        tags = {tag for tag in args.tags if tag}
-                        special = {tag for tag in tags if tag.startswith("@")}
-                        if not special.issubset(SPECIAL_TAGS):
-                            raise Error("Unknown special tags given.")
-                        archive.tags = tags
-
+                    archive.tags = set(args.tags or [])
                     archive.save(comment=args.comment, timestamp=args.timestamp)
                     args.stats |= args.json
                     if args.stats:

src/borg/archiver/tag_cmd.py

@@ -1,7 +1,7 @@
 from ._common import with_repository, define_archive_filters_group
 from ..archive import Archive
 from ..constants import *  # NOQA
-from ..helpers import bin_to_hex, archivename_validator, tag_validator, Error
+from ..helpers import bin_to_hex, archivename_validator, tag_validator
 from ..helpers.argparsing import ArgumentParser
 from ..manifest import Manifest
 
@@ -15,39 +15,23 @@ class TagMixIn:
     def do_tag(self, args, repository, manifest, cache):
         """Manage tags."""
 
-        def tags_set(tags):
-            """Return a set of tags, removing empty tags."""
-            return {tag for tag in tags if tag}
-
         if args.name:
             archive_infos = [manifest.archives.get_one([args.name])]
         else:
             archive_infos = manifest.archives.list_considering(args)
 
-        def check_special(tags):
-            if tags:
-                special = {tag for tag in tags_set(tags) if tag.startswith("@")}
-                if not special.issubset(SPECIAL_TAGS):
-                    raise Error("Unknown special tags given.")
-
-        check_special(args.set_tags)
-        check_special(args.add_tags)
-        check_special(args.remove_tags)
-
         for archive_info in archive_infos:
             archive = Archive(manifest, archive_info.id, cache=cache)
-            if args.set_tags:
+            if args.set_tags is not None:
                 # avoid that --set (accidentally) erases existing special tags,
                 # but allow --set if the existing special tags are also given.
-                new_tags = tags_set(args.set_tags)
+                new_tags = set(args.set_tags)
                 existing_special = {tag for tag in archive.tags if tag.startswith("@")}
                 clobber = not existing_special.issubset(new_tags)
                 if not clobber:
                     archive.tags = new_tags
-            if args.add_tags:
-                archive.tags |= tags_set(args.add_tags)
-            if args.remove_tags:
-                archive.tags -= tags_set(args.remove_tags)
+            archive.tags |= set(args.add_tags or [])
+            archive.tags -= set(args.remove_tags or [])
             old_id = archive.id
             archive.set_meta("tags", list(sorted(archive.tags)))
             if old_id != archive.id:
@@ -81,10 +65,10 @@ def build_parser_tag(self, subparsers, common_parser, mid_common_parser):
         )
         subparser = ArgumentParser(parents=[common_parser], description=self.do_tag.__doc__, epilog=tag_epilog)
         subparsers.add_subcommand("tag", subparser, help="tag archives")
-        subparser.add_argument("--set", dest="set_tags", metavar="TAG", type=tag_validator, nargs="+", help="set tags")
-        subparser.add_argument("--add", dest="add_tags", metavar="TAG", type=tag_validator, nargs="+", help="add tags")
+        subparser.add_argument("--set", dest="set_tags", metavar="TAG", type=tag_validator, nargs="*", help="set tags")
+        subparser.add_argument("--add", dest="add_tags", metavar="TAG", type=tag_validator, nargs="*", help="add tags")
         subparser.add_argument(
-            "--remove", dest="remove_tags", metavar="TAG", type=tag_validator, nargs="+", help="remove tags"
+            "--remove", dest="remove_tags", metavar="TAG", type=tag_validator, nargs="*", help="remove tags"
         )
         define_archive_filters_group(subparser)
         subparser.add_argument(

src/borg/helpers/parseformat.py

@@ -765,7 +765,14 @@ def validator(text):
 
 
 comment_validator = text_validator(name="comment", max_length=10000)
-tag_validator = text_validator(name="tag", min_length=0, max_length=10, invalid_chars=" ,$")
+validate_tag_text = text_validator(name="tag", min_length=1, max_length=10, invalid_chars=" ,$")
+
+
+def tag_validator(text):
+    validated_text = validate_tag_text(text)
+    if validated_text.startswith("@") and validated_text not in SPECIAL_TAGS:
+        raise ArgumentTypeError("Unknown special tags given.")
+    return validated_text
 
 
 def archivename_validator(text):

src/borg/testsuite/archiver/create_cmd_test.py

@@ -15,7 +15,7 @@
 from ...manifest import Manifest
 from ...platform import is_win32
 from ...repository import Repository
-from ...helpers import CommandError, BackupPermissionError, Error
+from ...helpers import CommandError, BackupPermissionError
 from .. import has_lchflags, has_mknod
 from .. import changedir
 from .. import (
@@ -696,12 +696,8 @@ def test_create_invalid_tags(archivers, request):
     archiver = request.getfixturevalue(archivers)
     create_test_files(archiver.input_path)
     cmd(archiver, "repo-create", RK_ENCRYPTION)
-    if archiver.FORK_DEFAULT:
-        output = cmd(archiver, "create", "--tags", "@INVALID", "--", "test", "input", exit_code=EXIT_ERROR)
-        assert "Unknown special tags given" in output
-    else:
-        with pytest.raises(Error):
-            cmd(archiver, "create", "--tags", "@INVALID", "--", "test", "input")
+    output = cmd(archiver, "create", "--tags", "@INVALID", "--", "test", "input", exit_code=EXIT_ERROR)
+    assert "Unknown special tags given" in output
 
 
 @pytest.mark.skipif(

src/borg/testsuite/archiver/tag_cmd_test.py

@@ -1,8 +1,5 @@
-import pytest
-
 from ...constants import *  # NOQA
 from . import cmd, generate_archiver_tests, RK_ENCRYPTION
-from ...helpers import Error
 
 pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local")  # NOQA
 
@@ -17,7 +14,7 @@ def test_tag_set(archivers, request):
     assert "tags: bb." in output
     output = cmd(archiver, "tag", "-a", "archive", "--set", "bb", "aa")
     assert "tags: aa,bb." in output  # sorted!
-    output = cmd(archiver, "tag", "-a", "archive", "--set", "")
+    output = cmd(archiver, "tag", "-a", "archive", "--set")
     assert "tags: ." in output  # no tags!
 
 
@@ -55,9 +52,6 @@ def test_tag_only_known_special(archivers, request):
     cmd(archiver, "repo-create", RK_ENCRYPTION)
     cmd(archiver, "create", "archive", archiver.input_path)
     # user can't set / add / remove unknown special tags
-    with pytest.raises(Error):
-        cmd(archiver, "tag", "-a", "archive", "--set", "@UNKNOWN")
-    with pytest.raises(Error):
-        cmd(archiver, "tag", "-a", "archive", "--add", "@UNKNOWN")
-    with pytest.raises(Error):
-        cmd(archiver, "tag", "-a", "archive", "--remove", "@UNKNOWN")
+    cmd(archiver, "tag", "-a", "archive", "--set", "@UNKNOWN", exit_code=EXIT_ERROR)
+    cmd(archiver, "tag", "-a", "archive", "--add", "@UNKNOWN", exit_code=EXIT_ERROR)
+    cmd(archiver, "tag", "-a", "archive", "--remove", "@UNKNOWN", exit_code=EXIT_ERROR)