From ad122e554ae5e218f840632fb1f24cbc0295b3fc Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 30 Jan 2022 03:10:55 +0100 Subject: [PATCH 1/2] repo::archive location placeholder expansion fixes, fixes #5826, fixes #5998 - use expanded location for log output - support placeholder expansion for BORG_REPO env var - use Location.raw for the unprocessed, not expanded location string --- src/borg/helpers/parseformat.py | 15 +++++++++------ src/borg/testsuite/helpers.py | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index 6cdd4a898..7d23cbb15 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -382,19 +382,21 @@ def __init__(self, text='', overrides={}): raise ValueError('Invalid location format: "%s"' % self.orig) def parse(self, text, overrides={}): - self.orig = text - text = replace_placeholders(text, overrides) + self.raw = text # as given by user, might contain placeholders + self.orig = text = replace_placeholders(text, overrides) # after placeholder replacement valid = self._parse(text) if valid: return True m = self.env_re.match(text) if not m: return False - repo = os.environ.get('BORG_REPO') - if repo is None: + repo_raw = os.environ.get('BORG_REPO') + if repo_raw is None: return False + repo = replace_placeholders(repo_raw, overrides) valid = self._parse(repo) self.archive = m.group('archive') + self.raw = repo_raw if not self.archive else repo_raw + self.raw self.orig = repo if not self.archive else '%s::%s' % (repo, self.archive) return valid @@ -488,14 +490,15 @@ def canonical_path(self): path) def with_timestamp(self, timestamp): - return Location(self.orig, overrides={ + return Location(self.raw, overrides={ 'now': DatetimeWrapper(timestamp.astimezone(None)), 'utcnow': DatetimeWrapper(timestamp), }) def omit_archive(self): - loc = Location(self.orig) + loc = Location(self.raw) loc.archive = None + loc.raw = loc.raw.split("::")[0] loc.orig = loc.orig.split("::")[0] return loc diff --git a/src/borg/testsuite/helpers.py b/src/borg/testsuite/helpers.py index 642ea725a..e80cc6e49 100644 --- a/src/borg/testsuite/helpers.py +++ b/src/borg/testsuite/helpers.py @@ -235,10 +235,12 @@ def test_bad_syntax(self): Location('ssh://user@host:/path') def test_omit_archive(self): - loc = Location('ssh://user@host:1234/some/path::archive') + from borg.platform import hostname + loc = Location('ssh://user@host:1234/repos/{hostname}::archive') loc_without_archive = loc.omit_archive() assert loc_without_archive.archive is None - assert loc_without_archive.orig == "ssh://user@host:1234/some/path" + assert loc_without_archive.raw == "ssh://user@host:1234/repos/{hostname}" + assert loc_without_archive.orig == "ssh://user@host:1234/repos/%s" % hostname class TestLocationWithEnv: @@ -251,6 +253,16 @@ def test_ssh(self, monkeypatch): assert repr(Location()) == \ "Location(proto='ssh', user='user', host='host', port=1234, path='/some/path', archive=None)" + def test_ssh_placeholder(self, monkeypatch): + from borg.platform import hostname + monkeypatch.setenv('BORG_REPO', 'ssh://user@host:1234/{hostname}') + assert repr(Location('::archive')) == \ + "Location(proto='ssh', user='user', host='host', port=1234, path='/{}', archive='archive')".format(hostname) + assert repr(Location('::')) == \ + "Location(proto='ssh', user='user', host='host', port=1234, path='/{}', archive=None)".format(hostname) + assert repr(Location()) == \ + "Location(proto='ssh', user='user', host='host', port=1234, path='/{}', archive=None)".format(hostname) + def test_file(self, monkeypatch): monkeypatch.setenv('BORG_REPO', 'file:///some/path') assert repr(Location('::archive')) == \ From c4116b26c866f190d9e1cbadd76b9780bec8ab5a Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 30 Jan 2022 03:46:11 +0100 Subject: [PATCH 2/2] Location: fix bad naming: rename .orig -> .processed - .raw is the unprocessed location (as given by user / env). - .processed is the processed location (after placeholder replacement). --- src/borg/archiver.py | 2 +- src/borg/helpers/parseformat.py | 8 ++++---- src/borg/remote.py | 10 +++++----- src/borg/testsuite/helpers.py | 2 +- src/borg/testsuite/key.py | 4 ++-- src/borg/testsuite/repository.py | 8 ++++---- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 22bcc0542..7560c557c 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -636,7 +636,7 @@ def create_inner(archive, cache, fso): dry_run = args.dry_run t0 = datetime.utcnow() t0_monotonic = time.monotonic() - logger.info('Creating archive at "%s"' % args.location.orig) + logger.info('Creating archive at "%s"' % args.location.processed) if not dry_run: with Cache(repository, key, manifest, progress=args.progress, lock_wait=self.lock_wait, permit_adhoc_cache=args.no_cache_sync, diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index 7d23cbb15..bc3359807 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -379,11 +379,11 @@ class Location: def __init__(self, text='', overrides={}): if not self.parse(text, overrides): - raise ValueError('Invalid location format: "%s"' % self.orig) + raise ValueError('Invalid location format: "%s"' % self.processed) def parse(self, text, overrides={}): self.raw = text # as given by user, might contain placeholders - self.orig = text = replace_placeholders(text, overrides) # after placeholder replacement + self.processed = text = replace_placeholders(text, overrides) # after placeholder replacement valid = self._parse(text) if valid: return True @@ -397,7 +397,7 @@ def parse(self, text, overrides={}): valid = self._parse(repo) self.archive = m.group('archive') self.raw = repo_raw if not self.archive else repo_raw + self.raw - self.orig = repo if not self.archive else '%s::%s' % (repo, self.archive) + self.processed = repo if not self.archive else '%s::%s' % (repo, self.archive) return valid def _parse(self, text): @@ -499,7 +499,7 @@ def omit_archive(self): loc = Location(self.raw) loc.archive = None loc.raw = loc.raw.split("::")[0] - loc.orig = loc.orig.split("::")[0] + loc.processed = loc.processed.split("::")[0] return loc diff --git a/src/borg/remote.py b/src/borg/remote.py index 5d32a4ff6..9017e2aa1 100644 --- a/src/borg/remote.py +++ b/src/borg/remote.py @@ -738,11 +738,11 @@ def handle_error(unpacked): args = unpacked.get(b'exception_args') if error == 'DoesNotExist': - raise Repository.DoesNotExist(self.location.orig) + raise Repository.DoesNotExist(self.location.processed) elif error == 'AlreadyExists': - raise Repository.AlreadyExists(self.location.orig) + raise Repository.AlreadyExists(self.location.processed) elif error == 'CheckNeeded': - raise Repository.CheckNeeded(self.location.orig) + raise Repository.CheckNeeded(self.location.processed) elif error == 'IntegrityError': if old_server: raise IntegrityError('(not available)') @@ -762,9 +762,9 @@ def handle_error(unpacked): raise Repository.ParentPathDoesNotExist(args[0].decode()) elif error == 'ObjectNotFound': if old_server: - raise Repository.ObjectNotFound('(not available)', self.location.orig) + raise Repository.ObjectNotFound('(not available)', self.location.processed) else: - raise Repository.ObjectNotFound(args[0].decode(), self.location.orig) + raise Repository.ObjectNotFound(args[0].decode(), self.location.processed) elif error == 'InvalidRPCMethod': if old_server: raise InvalidRPCMethod('(not available)') diff --git a/src/borg/testsuite/helpers.py b/src/borg/testsuite/helpers.py index e80cc6e49..63dae7804 100644 --- a/src/borg/testsuite/helpers.py +++ b/src/borg/testsuite/helpers.py @@ -240,7 +240,7 @@ def test_omit_archive(self): loc_without_archive = loc.omit_archive() assert loc_without_archive.archive is None assert loc_without_archive.raw == "ssh://user@host:1234/repos/{hostname}" - assert loc_without_archive.orig == "ssh://user@host:1234/repos/%s" % hostname + assert loc_without_archive.processed == "ssh://user@host:1234/repos/%s" % hostname class TestLocationWithEnv: diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index 1e09802ea..92f763584 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -87,10 +87,10 @@ def key(self, request, monkeypatch): class MockRepository: class _Location: - orig = '/some/place' + raw = processed = '/some/place' def canonical_path(self): - return self.orig + return self.processed _location = _Location() id = bytes(32) diff --git a/src/borg/testsuite/repository.py b/src/borg/testsuite/repository.py index 8149af3cd..580f52e2e 100644 --- a/src/borg/testsuite/repository.py +++ b/src/borg/testsuite/repository.py @@ -887,19 +887,19 @@ def test_rpc_exception_transport(self): self.repository.call('inject_exception', {'kind': 'DoesNotExist'}) except Repository.DoesNotExist as e: assert len(e.args) == 1 - assert e.args[0] == self.repository.location.orig + assert e.args[0] == self.repository.location.processed try: self.repository.call('inject_exception', {'kind': 'AlreadyExists'}) except Repository.AlreadyExists as e: assert len(e.args) == 1 - assert e.args[0] == self.repository.location.orig + assert e.args[0] == self.repository.location.processed try: self.repository.call('inject_exception', {'kind': 'CheckNeeded'}) except Repository.CheckNeeded as e: assert len(e.args) == 1 - assert e.args[0] == self.repository.location.orig + assert e.args[0] == self.repository.location.processed try: self.repository.call('inject_exception', {'kind': 'IntegrityError'}) @@ -918,7 +918,7 @@ def test_rpc_exception_transport(self): except Repository.ObjectNotFound as e: assert len(e.args) == 2 assert e.args[0] == s1 - assert e.args[1] == self.repository.location.orig + assert e.args[1] == self.repository.location.processed try: self.repository.call('inject_exception', {'kind': 'InvalidRPCMethod'})