From ce6daff12f624dfa44132115aa3b519f1e3ab748 Mon Sep 17 00:00:00 2001 From: Felix Kaechele Date: Tue, 23 May 2023 17:18:46 -0400 Subject: [PATCH 1/4] Fix importlib.metadata.files workaround Some distributions, such as Fedora, do not install the RECORDS file as part of a package's dist-info. As a result importlib.metadata.files will return None. Use the workaround for these cases as well. Signed-off-by: Felix Kaechele --- borgmatic/config/validate.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index b39199fe..1c9b1050 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -16,19 +16,19 @@ def schema_filename(): ''' Path to the installed YAML configuration schema file, used to validate and parse the configuration. - - Raise FileNotFoundError when the schema path does not exist. ''' - try: - return next( - str(path.locate()) - for path in importlib_metadata.files('borgmatic') - if path.match('config/schema.yaml') - ) - except StopIteration: - # If the schema wasn't found in the package's files, this is probably a pip editable - # install, so try a different approach to get the schema. - return os.path.join(os.path.dirname(borgmatic.config.__file__), 'schema.yaml') + + files = importlib_metadata.files('borgmatic') + if files is not None: + try: + return next(str(path.locate()) for path in files if path.match('config/schema.yaml')) + except StopIteration: + # schema not found in package, fall through to the approach below + pass + + # If the schema wasn't found in the package's files, this is probably a pip editable + # install, so try a different approach to get the schema. + return os.path.join(os.path.dirname(borgmatic.config.__file__), 'schema.yaml') def format_json_error_path_element(path_element): From 15cabb93cab55c3f61a30c9e6bb0fdbf6d06b304 Mon Sep 17 00:00:00 2001 From: Felix Kaechele Date: Fri, 2 Jun 2023 21:35:33 -0400 Subject: [PATCH 2/4] Drop importlib_metadata entirely The fallback option using the dirname of the config module location seems to be more robust in a number of cases. Signed-off-by: Felix Kaechele --- borgmatic/config/validate.py | 16 ---------------- tests/unit/config/test_validate.py | 19 +------------------ 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index 1c9b1050..2ed1af5d 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -3,11 +3,6 @@ import os import jsonschema import ruamel.yaml -try: - import importlib_metadata -except ModuleNotFoundError: # pragma: nocover - import importlib.metadata as importlib_metadata - import borgmatic.config from borgmatic.config import environment, load, normalize, override @@ -17,17 +12,6 @@ def schema_filename(): Path to the installed YAML configuration schema file, used to validate and parse the configuration. ''' - - files = importlib_metadata.files('borgmatic') - if files is not None: - try: - return next(str(path.locate()) for path in files if path.match('config/schema.yaml')) - except StopIteration: - # schema not found in package, fall through to the approach below - pass - - # If the schema wasn't found in the package's files, this is probably a pip editable - # install, so try a different approach to get the schema. return os.path.join(os.path.dirname(borgmatic.config.__file__), 'schema.yaml') diff --git a/tests/unit/config/test_validate.py b/tests/unit/config/test_validate.py index e81f2b02..fdb13b68 100644 --- a/tests/unit/config/test_validate.py +++ b/tests/unit/config/test_validate.py @@ -5,24 +5,7 @@ from borgmatic.config import validate as module def test_schema_filename_finds_schema_path(): - schema_path = '/var/borgmatic/config/schema.yaml' - - flexmock(module.importlib_metadata).should_receive('files').and_return( - flexmock(match=lambda path: False, locate=lambda: None), - flexmock(match=lambda path: True, locate=lambda: schema_path), - flexmock(match=lambda path: False, locate=lambda: None), - ) - - assert module.schema_filename() == schema_path - - -def test_schema_filename_with_missing_schema_path_in_package_still_finds_it_in_config_directory(): - flexmock(module.importlib_metadata).should_receive('files').and_return( - flexmock(match=lambda path: False, locate=lambda: None), - flexmock(match=lambda path: False, locate=lambda: None), - ) - - assert module.schema_filename().endswith('/borgmatic/config/schema.yaml') + module.schema_filename().endswith('/borgmatic/config/schema.yaml') def test_format_json_error_path_element_formats_array_index(): From ba0899660dd3e1bfd0ee6264c7001885719efc42 Mon Sep 17 00:00:00 2001 From: Felix Kaechele Date: Sat, 3 Jun 2023 08:11:56 -0400 Subject: [PATCH 3/4] Verify that schema path exists before returning it Signed-off-by: Felix Kaechele --- borgmatic/config/validate.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index 2ed1af5d..1917eea0 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -11,8 +11,15 @@ def schema_filename(): ''' Path to the installed YAML configuration schema file, used to validate and parse the configuration. + + Raise FileNotFoundError when the schema path does not exist. ''' - return os.path.join(os.path.dirname(borgmatic.config.__file__), 'schema.yaml') + schema_path = os.path.join(os.path.dirname(borgmatic.config.__file__), 'schema.yaml') + + if os.path.exists(schema_path) and os.path.isfile(schema_path): + return schema_path + + raise FileNotFoundError def format_json_error_path_element(path_element): From c61d63b235f6e2bcd59a88baea85012704453983 Mon Sep 17 00:00:00 2001 From: Felix Kaechele Date: Sun, 4 Jun 2023 00:50:35 -0400 Subject: [PATCH 4/4] Use open() to test for file existance and readability Signed-off-by: Felix Kaechele --- borgmatic/config/validate.py | 4 +--- tests/unit/config/test_validate.py | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index 1917eea0..5835ead1 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -16,11 +16,9 @@ def schema_filename(): ''' schema_path = os.path.join(os.path.dirname(borgmatic.config.__file__), 'schema.yaml') - if os.path.exists(schema_path) and os.path.isfile(schema_path): + with open(schema_path): return schema_path - raise FileNotFoundError - def format_json_error_path_element(path_element): ''' diff --git a/tests/unit/config/test_validate.py b/tests/unit/config/test_validate.py index fdb13b68..701ba510 100644 --- a/tests/unit/config/test_validate.py +++ b/tests/unit/config/test_validate.py @@ -1,3 +1,7 @@ +import os +import sys +from io import StringIO + import pytest from flexmock import flexmock @@ -5,7 +9,23 @@ from borgmatic.config import validate as module def test_schema_filename_finds_schema_path(): - module.schema_filename().endswith('/borgmatic/config/schema.yaml') + schema_path = '/var/borgmatic/config/schema.yaml' + + flexmock(os.path).should_receive('dirname').and_return("/var/borgmatic/config") + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args(schema_path).and_return(StringIO()) + assert module.schema_filename() == schema_path + + +def test_schema_filename_raises_filenotfounderror(): + schema_path = '/var/borgmatic/config/schema.yaml' + + flexmock(os.path).should_receive('dirname').and_return("/var/borgmatic/config") + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args(schema_path).and_raise(FileNotFoundError) + + with pytest.raises(FileNotFoundError): + module.schema_filename() def test_format_json_error_path_element_formats_array_index():