From fb8441f18e613145f3eee8ed59e25e093ddc5c14 Mon Sep 17 00:00:00 2001 From: jsuchan-reef <135112713+jsuchan-reef@users.noreply.github.com> Date: Fri, 1 Dec 2023 20:23:54 +0700 Subject: [PATCH] use xdg compliant path for config (#447) * change: config lookup path is xdg-compliant on such OSes * add backward-compatible lookup for pre-existing config file * remove obsolete monkeypatch * CR feedback * fix tests --- b2sdk/account_info/sqlite_account_info.py | 42 +++++++++++++------ changelog.d/b2cli958.changed.md | 1 + test/unit/account_info/test_account_info.py | 41 +++++++++++------- .../account_info/test_sqlite_account_info.py | 18 ++++++-- 4 files changed, 71 insertions(+), 31 deletions(-) create mode 100644 changelog.d/b2cli958.changed.md diff --git a/b2sdk/account_info/sqlite_account_info.py b/b2sdk/account_info/sqlite_account_info.py index c1d858a78..fffd663cc 100644 --- a/b2sdk/account_info/sqlite_account_info.py +++ b/b2sdk/account_info/sqlite_account_info.py @@ -15,6 +15,7 @@ import re import sqlite3 import stat +import sys import threading from .exception import CorruptAccountInfo, MissingAccountData @@ -53,18 +54,19 @@ def __init__(self, file_name=None, last_upgrade_to_run=None, profile: str | None If ``profile`` arg is provided: - * ``{XDG_CONFIG_HOME_ENV_VAR}/b2/db-.sqlite``, if ``{XDG_CONFIG_HOME_ENV_VAR}`` env var is set + * ``{B2_ACCOUNT_INFO_PROFILE_FILE}`` if it already exists + * ``${XDG_CONFIG_HOME_ENV_VAR}/b2/db-.sqlite`` on XDG-compatible OSes (Linux, BSD) * ``{B2_ACCOUNT_INFO_PROFILE_FILE}`` Otherwise: * ``file_name``, if truthy * ``{B2_ACCOUNT_INFO_ENV_VAR}`` env var's value, if set - * ``{B2_ACCOUNT_INFO_DEFAULT_FILE}``, if it exists - * ``{XDG_CONFIG_HOME_ENV_VAR}/b2/account_info``, if ``{XDG_CONFIG_HOME_ENV_VAR}`` env var is set + * ``{B2_ACCOUNT_INFO_DEFAULT_FILE}``, if it already exists + * ``${XDG_CONFIG_HOME_ENV_VAR}/b2/account_info`` on XDG-compatible OSes (Linux, BSD) * ``{B2_ACCOUNT_INFO_DEFAULT_FILE}``, as default - If the directory ``{XDG_CONFIG_HOME_ENV_VAR}/b2`` does not exist (and is needed), it is created. + If the directory ``${XDG_CONFIG_HOME_ENV_VAR}/b2`` does not exist (and is needed), it is created. :param str file_name: The sqlite file to use; overrides the default. :param int last_upgrade_to_run: For testing only, override the auto-update on the db. @@ -91,10 +93,25 @@ def __init__(self, file_name=None, last_upgrade_to_run=None, profile: str | None ) @classmethod - def _get_user_account_info_path(cls, file_name=None, profile=None): + def _get_xdg_config_path(cls) -> str | None: + """ + Return XDG config path if the OS is XDG-compatible (Linux, BSD), None otherwise. + + If $XDG_CONFIG_HOME is empty but the OS is XDG compliant, fallback to ~/.config as expected by XDG standard. + """ + xdg_config_home = os.getenv(XDG_CONFIG_HOME_ENV_VAR) + if xdg_config_home or sys.platform not in ('win32', 'darwin'): + return xdg_config_home or os.path.join(os.path.expanduser('~/.config')) + return None + + @classmethod + def _get_user_account_info_path(cls, file_name: str | None = None, profile: str | None = None): if profile and not B2_ACCOUNT_INFO_PROFILE_NAME_REGEXP.match(profile): raise ValueError(f'Invalid profile name: {profile}') + profile_file = B2_ACCOUNT_INFO_PROFILE_FILE.format(profile=profile) if profile else None + xdg_config_path = cls._get_xdg_config_path() + if file_name: if profile: raise ValueError('Provide either file_name or profile, not both') @@ -106,16 +123,17 @@ def _get_user_account_info_path(cls, file_name=None, profile=None): format(B2_ACCOUNT_INFO_ENV_VAR) ) user_account_info_path = os.environ[B2_ACCOUNT_INFO_ENV_VAR] - elif os.path.exists(os.path.expanduser(B2_ACCOUNT_INFO_DEFAULT_FILE)) and not profile: + elif not profile and os.path.exists(os.path.expanduser(B2_ACCOUNT_INFO_DEFAULT_FILE)): user_account_info_path = B2_ACCOUNT_INFO_DEFAULT_FILE - elif XDG_CONFIG_HOME_ENV_VAR in os.environ: - config_home = os.environ[XDG_CONFIG_HOME_ENV_VAR] + elif profile and os.path.exists(profile_file): + user_account_info_path = profile_file + elif xdg_config_path: + os.makedirs(os.path.join(xdg_config_path, 'b2'), mode=0o755, exist_ok=True) + file_name = f'db-{profile}.sqlite' if profile else 'account_info' - user_account_info_path = os.path.join(config_home, 'b2', file_name) - if not os.path.exists(os.path.join(config_home, 'b2')): - os.makedirs(os.path.join(config_home, 'b2'), mode=0o755) + user_account_info_path = os.path.join(xdg_config_path, 'b2', file_name) elif profile: - user_account_info_path = B2_ACCOUNT_INFO_PROFILE_FILE.format(profile=profile) + user_account_info_path = profile_file else: user_account_info_path = B2_ACCOUNT_INFO_DEFAULT_FILE diff --git a/changelog.d/b2cli958.changed.md b/changelog.d/b2cli958.changed.md new file mode 100644 index 000000000..804d8a35a --- /dev/null +++ b/changelog.d/b2cli958.changed.md @@ -0,0 +1 @@ +On XDG compatible OSes (Linux, BSD), the profile file is now created in `$XDG_CONFIG_HOME` (with a fallback to `~/.config/` in absence of given env. variable). diff --git a/test/unit/account_info/test_account_info.py b/test/unit/account_info/test_account_info.py index e67893344..b332ffd71 100644 --- a/test/unit/account_info/test_account_info.py +++ b/test/unit/account_info/test_account_info.py @@ -14,6 +14,7 @@ import platform import shutil import stat +import sys import tempfile import unittest.mock as mock from abc import ABCMeta, abstractmethod @@ -345,6 +346,7 @@ def setUp(self, request): self.test_home = tempfile.mkdtemp() yield + for cleanup_method in [ lambda: os.unlink(self.db_path), lambda: shutil.rmtree(self.test_home) ]: @@ -414,30 +416,26 @@ def _make_sqlite_account_info(self, env=None, last_upgrade_to_run=None): last_upgrade_to_run=last_upgrade_to_run, ) - def test_uses_default(self): - account_info = self._make_sqlite_account_info( - env={ + def test_uses_xdg_config_home(self, apiver): + is_xdg_os = bool(SqliteAccountInfo._get_xdg_config_path()) + with WindowsSafeTempDir() as d: + env = { 'HOME': self.test_home, 'USERPROFILE': self.test_home, } - ) - actual_path = os.path.abspath(account_info.filename) - assert os.path.join(self.test_home, '.b2_account_info') == actual_path - def test_uses_xdg_config_home(self, apiver): - with WindowsSafeTempDir() as d: - account_info = self._make_sqlite_account_info( - env={ - 'HOME': self.test_home, - 'USERPROFILE': self.test_home, - XDG_CONFIG_HOME_ENV_VAR: d, - } - ) + if is_xdg_os: + # pass the env. variable on XDG-like OS only + env[XDG_CONFIG_HOME_ENV_VAR] = d + + account_info = self._make_sqlite_account_info(env=env) if apiver in ['v0', 'v1']: expected_path = os.path.abspath(os.path.join(self.test_home, '.b2_account_info')) - else: + elif is_xdg_os: assert os.path.exists(os.path.join(d, 'b2')) expected_path = os.path.abspath(os.path.join(d, 'b2', 'account_info')) + else: + expected_path = os.path.abspath(os.path.join(self.test_home, '.b2_account_info')) actual_path = os.path.abspath(account_info.filename) assert expected_path == actual_path @@ -469,3 +467,14 @@ def test_account_info_env_var_overrides_xdg_config_home(self): expected_path = os.path.abspath(os.path.join(d, 'b2_account_info')) actual_path = os.path.abspath(account_info.filename) assert expected_path == actual_path + + def test_resolve_xdg_os_default(self): + is_xdg_os = bool(SqliteAccountInfo._get_xdg_config_path()) + assert is_xdg_os == (sys.platform not in ('win32', 'darwin')) + + def test_resolve_xdg_os_default_no_env_var(self, monkeypatch): + # ensure that XDG_CONFIG_HOME_ENV_VAR doesn't to resolve XDG-like OS + monkeypatch.delenv(XDG_CONFIG_HOME_ENV_VAR, raising=False) + + is_xdg_os = bool(SqliteAccountInfo._get_xdg_config_path()) + assert is_xdg_os == (sys.platform not in ('win32', 'darwin')) diff --git a/test/unit/account_info/test_sqlite_account_info.py b/test/unit/account_info/test_sqlite_account_info.py index 75f7b9ca1..1bcd28c50 100644 --- a/test/unit/account_info/test_sqlite_account_info.py +++ b/test/unit/account_info/test_sqlite_account_info.py @@ -95,9 +95,15 @@ def test_profile_and_xdg_config_env_var(self, monkeypatch): os.path.join('~', 'custom', 'b2', 'db-secondary.sqlite') ) - def test_profile(self): + def test_profile(self, monkeypatch): + xdg_config_path = SqliteAccountInfo._get_xdg_config_path() + if xdg_config_path: + expected_path = (xdg_config_path, 'b2', 'db-foo.sqlite') + else: + expected_path = ('~', '.b2db-foo.sqlite') + account_info_path = SqliteAccountInfo._get_user_account_info_path(profile='foo') - assert account_info_path == os.path.expanduser(os.path.join('~', '.b2db-foo.sqlite')) + assert account_info_path == os.path.expanduser(os.path.join(*expected_path)) def test_file_name(self): account_info_path = SqliteAccountInfo._get_user_account_info_path( @@ -129,5 +135,11 @@ def test_xdg_config_env_var(self, monkeypatch): ) def test_default_file(self): + xdg_config_path = SqliteAccountInfo._get_xdg_config_path() + if xdg_config_path: + expected_path = os.path.join(xdg_config_path, 'b2', 'account_info') + else: + expected_path = B2_ACCOUNT_INFO_DEFAULT_FILE + account_info_path = SqliteAccountInfo._get_user_account_info_path() - assert account_info_path == os.path.expanduser(B2_ACCOUNT_INFO_DEFAULT_FILE) + assert account_info_path == os.path.expanduser(expected_path)