Repository: borgbackup/borg

Issue:
Generic error message when trying to use non-upgraded attic repository
Hi,

while playing with borg (as attic user) i accidentally run borg commands on attic*s repository and i get ugly trackback:
```
Traceback (most recent call last):
  File "/usr/lib64/python3.4/site-packages/borg/archiver.py", line 1928, in main
    exit_code = archiver.run(args)
  File "/usr/lib64/python3.4/site-packages/borg/archiver.py", line 1873, in run
    return args.func(args)
  File "/usr/lib64/python3.4/site-packages/borg/archiver.py", line 81, in wrapper
    kwargs['manifest'], kwargs['key'] = Manifest.load(repository)
  File "/usr/lib64/python3.4/site-packages/borg/helpers.py", line 113, in load
    cdata = repository.get(cls.MANIFEST_ID)
  File "/usr/lib64/python3.4/site-packages/borg/repository.py", line 512, in get
    self.index = self.open_index(self.get_transaction_id())
  File "/usr/lib64/python3.4/site-packages/borg/repository.py", line 152, in get_transaction_id
    raise self.CheckNeeded(self.path)
borg.repository.CheckNeeded: /srv/backup/system.borg
```
IMO, this exception have to be catched and only error message needs to be reported, to get more better look ;-)

---

:moneybag: [there is a bounty for this](https://www.bountysource.com/issues/39795380-generic-error-message-when-trying-to-use-non-upgraded-attic-repository)

Discussion:

Patch:
diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst
index c41d427e..677db738 100644
--- a/docs/internals/frontends.rst
+++ b/docs/internals/frontends.rst
@@ -499,6 +499,8 @@ Errors
         Insufficient free space to complete transaction (required: {}, available: {}).
     Repository.InvalidRepository
         {} is not a valid repository. Check repo config.
+    Repository.AtticRepository
+        Attic repository detected. Please run "borg upgrade {}".
     Repository.ObjectNotFound
         Object with key {} not found in repository {}.
 
diff --git a/src/borg/remote.py b/src/borg/remote.py
index d131a826..a508e04f 100644
--- a/src/borg/remote.py
+++ b/src/borg/remote.py
@@ -733,6 +733,11 @@ def handle_error(unpacked):
                     raise IntegrityError('(not available)')
                 else:
                     raise IntegrityError(args[0].decode())
+            elif error == 'AtticRepository':
+                if old_server:
+                    raise Repository.AtticRepository('(not available)')
+                else:
+                    raise Repository.AtticRepository(args[0].decode())
             elif error == 'PathNotAllowed':
                 if old_server:
                     raise PathNotAllowed('(unknown)')
diff --git a/src/borg/repository.py b/src/borg/repository.py
index a61e4e94..942b862d 100644
--- a/src/borg/repository.py
+++ b/src/borg/repository.py
@@ -30,6 +30,8 @@
 
 MAGIC = b'BORG_SEG'
 MAGIC_LEN = len(MAGIC)
+ATTIC_MAGIC = b'ATTICSEG'
+assert len(ATTIC_MAGIC) == MAGIC_LEN
 TAG_PUT = 0
 TAG_DELETE = 1
 TAG_COMMIT = 2
@@ -116,6 +118,9 @@ class AlreadyExists(Error):
     class InvalidRepository(Error):
         """{} is not a valid repository. Check repo config."""
 
+    class AtticRepository(Error):
+        """Attic repository detected. Please run "borg upgrade {}"."""
+
     class CheckNeeded(ErrorWithTraceback):
         """Inconsistency detected. Please run "borg check {}"."""
 
@@ -134,7 +139,7 @@ class StorageQuotaExceeded(Error):
         """The storage quota ({}) has been exceeded ({}). Try deleting some archives."""
 
     def __init__(self, path, create=False, exclusive=False, lock_wait=None, lock=True,
-                 append_only=False, storage_quota=None):
+                 append_only=False, storage_quota=None, check_segment_magic=True):
         self.path = os.path.abspath(path)
         self._location = Location('file://%s' % self.path)
         self.io = None  # type: LoggedIO
@@ -154,6 +159,7 @@ def __init__(self, path, create=False, exclusive=False, lock_wait=None, lock=Tru
         self.storage_quota = storage_quota
         self.storage_quota_use = 0
         self.transaction_doomed = None
+        self.check_segment_magic = check_segment_magic
 
     def __del__(self):
         if self.lock:
@@ -375,6 +381,12 @@ def open(self, path, exclusive, lock_wait=None, lock=True):
             self.storage_quota = self.config.getint('repository', 'storage_quota', fallback=0)
         self.id = unhexlify(self.config.get('repository', 'id').strip())
         self.io = LoggedIO(self.path, self.max_segment_size, self.segments_per_dir)
+        if self.check_segment_magic:
+            # read a segment and check whether we are dealing with a non-upgraded Attic repository
+            segment = self.io.get_latest_segment()
+            if segment is not None and self.io.get_segment_magic(segment) == ATTIC_MAGIC:
+                self.close()
+                raise self.AtticRepository(path)
 
     def close(self):
         if self.lock:
@@ -1250,6 +1262,11 @@ def segment_exists(self, segment):
     def segment_size(self, segment):
         return os.path.getsize(self.segment_filename(segment))
 
+    def get_segment_magic(self, segment):
+        fd = self.get_fd(segment)
+        fd.seek(0)
+        return fd.read(MAGIC_LEN)
+
     def iter_objects(self, segment, offset=0, include_data=False, read_data=True):
         """
         Return object iterator for *segment*.
diff --git a/src/borg/upgrader.py b/src/borg/upgrader.py
index 0b92ce8e..1044f649 100644
--- a/src/borg/upgrader.py
+++ b/src/borg/upgrader.py
@@ -19,6 +19,7 @@
 class AtticRepositoryUpgrader(Repository):
     def __init__(self, *args, **kw):
         kw['lock'] = False  # do not create borg lock files (now) in attic repo
+        kw['check_segment_magic'] = False  # skip the Attic check when upgrading
         super().__init__(*args, **kw)
 
     def upgrade(self, dryrun=True, inplace=False, progress=False):


Test patch:
diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py
index a26bc47c..5242d6db 100644
--- a/src/borg/testsuite/archiver.py
+++ b/src/borg/testsuite/archiver.py
@@ -55,6 +55,7 @@
 from . import BaseTestCase, changedir, environment_variable, no_selinux
 from . import are_symlinks_supported, are_hardlinks_supported, are_fifos_supported, is_utime_fully_supported
 from .platform import fakeroot_detected
+from .upgrader import attic_repo
 from . import key
 
 
@@ -2731,6 +2732,27 @@ def test_extract_hardlinks(self):
             assert os.stat('input/dir1/aaaa').st_nlink == 2
             assert os.stat('input/dir1/source2').st_nlink == 2
 
+    def test_detect_attic_repo(self):
+        path = attic_repo(self.repository_path)
+        cmds = [
+            ['create', path + '::test', self.tmpdir],
+            ['extract', path + '::test'],
+            ['check', path],
+            ['rename', path + '::test', 'newname'],
+            ['list', path],
+            ['delete', path],
+            ['prune', path],
+            ['info', path + '::test'],
+            ['mount', path, self.tmpdir],
+            ['key', 'export', path, 'exported'],
+            ['key', 'import', path, 'import'],
+            ['change-passphrase', path],
+            ['break-lock', path],
+        ]
+        for args in cmds:
+            output = self.cmd(*args, fork=True, exit_code=2)
+            assert 'Attic repository detected.' in output
+
 
 @unittest.skipUnless('binary' in BORG_EXES, 'no borg.exe available')
 class ArchiverTestCaseBinary(ArchiverTestCase):
diff --git a/src/borg/testsuite/upgrader.py b/src/borg/testsuite/upgrader.py
index 3fd7500c..08c0693b 100644
--- a/src/borg/testsuite/upgrader.py
+++ b/src/borg/testsuite/upgrader.py
@@ -85,8 +85,8 @@ def test_convert_segments(attic_repo, inplace):
     :param attic_repo: a populated attic repository (fixture)
     """
     repo_path = attic_repo
-    # check should fail because of magic number
-    assert not repo_valid(repo_path)
+    with pytest.raises(Repository.AtticRepository):
+        repo_valid(repo_path)
     repository = AtticRepositoryUpgrader(repo_path, create=False)
     with repository:
         segments = [filename for i, filename in repository.io.segment_iterator()]
@@ -149,8 +149,8 @@ def test_convert_all(attic_repo, attic_key_file, inplace):
     """
     repo_path = attic_repo
 
-    # check should fail because of magic number
-    assert not repo_valid(repo_path)
+    with pytest.raises(Repository.AtticRepository):
+        repo_valid(repo_path)
 
     def stat_segment(path):
         return os.stat(os.path.join(path, 'data', '0', '0'))