From fcbd69e209134e0872db2e1f89d92a8ccdc833eb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 26 Feb 2015 07:01:22 +0100 Subject: [PATCH] Clean up standarddir handling #2. We already attempted this in c5a2039da431caa03b839e785d4046f05492c84d, but having the directories as module attributes means they'll be created on start (rather than when they're actually used), and it'd also be impossible to change them after init for some reason in the future. To still have a nice short API, we simply change the attributes to functions. --- qutebrowser/app.py | 4 +- qutebrowser/browser/adblock.py | 2 +- qutebrowser/browser/cache.py | 2 +- qutebrowser/browser/cookies.py | 4 +- qutebrowser/browser/downloads.py | 2 +- qutebrowser/browser/history.py | 4 +- qutebrowser/browser/quickmarks.py | 4 +- qutebrowser/commands/userscripts.py | 2 +- qutebrowser/config/config.py | 21 +++---- qutebrowser/config/websettings.py | 8 +-- qutebrowser/misc/sessions.py | 2 +- qutebrowser/test/utils/test_standarddir.py | 28 +++++----- qutebrowser/utils/standarddir.py | 65 ++++++++++++---------- 13 files changed, 79 insertions(+), 69 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 3b49882ca..407718386 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -225,7 +225,7 @@ class Application(QApplication): def _handle_segfault(self): """Handle a segfault from a previous run.""" - logname = os.path.join(standarddir.data, 'crash.log') + logname = os.path.join(standarddir.data(), 'crash.log') try: # First check if an old logfile exists. if os.path.exists(logname): @@ -249,7 +249,7 @@ class Application(QApplication): def _init_crashlogfile(self): """Start a new logfile and redirect faulthandler to it.""" - logname = os.path.join(standarddir.data, 'crash.log') + logname = os.path.join(standarddir.data(), 'crash.log') try: self._crashlogfile = open(logname, 'w', encoding='ascii') except OSError: diff --git a/qutebrowser/browser/adblock.py b/qutebrowser/browser/adblock.py index e99b6ef9f..5815a6c38 100644 --- a/qutebrowser/browser/adblock.py +++ b/qutebrowser/browser/adblock.py @@ -92,7 +92,7 @@ class HostBlocker: self.blocked_hosts = set() self._in_progress = [] self._done_count = 0 - self._hosts_file = os.path.join(standarddir.data, 'blocked-hosts') + self._hosts_file = os.path.join(standarddir.data(), 'blocked-hosts') objreg.get('config').changed.connect(self.on_config_changed) def read_hosts(self): diff --git a/qutebrowser/browser/cache.py b/qutebrowser/browser/cache.py index 52945660f..72c914edf 100644 --- a/qutebrowser/browser/cache.py +++ b/qutebrowser/browser/cache.py @@ -33,7 +33,7 @@ class DiskCache(QNetworkDiskCache): def __init__(self, parent=None): super().__init__(parent) - self.setCacheDirectory(os.path.join(standarddir.cache, 'http')) + self.setCacheDirectory(os.path.join(standarddir.cache(), 'http')) self.setMaximumCacheSize(config.get('storage', 'cache-size')) objreg.get('config').changed.connect(self.cache_size_changed) diff --git a/qutebrowser/browser/cookies.py b/qutebrowser/browser/cookies.py index 9538c556b..f88ab79a2 100644 --- a/qutebrowser/browser/cookies.py +++ b/qutebrowser/browser/cookies.py @@ -70,8 +70,8 @@ class CookieJar(RAMCookieJar): def __init__(self, parent=None): super().__init__(parent) - self._linecp = lineparser.LineConfigParser(standarddir.data, 'cookies', - binary=True, parent=self) + self._linecp = lineparser.LineConfigParser( + standarddir.data(), 'cookies', binary=True, parent=self) cookies = [] for line in self._linecp: cookies += QNetworkCookie.parseCookies(line) diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 38b1bf1ce..b5fe94bda 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -417,7 +417,7 @@ class DownloadItem(QObject): # save it under that filename in the default directory. download_dir = config.get('storage', 'download-directory') if download_dir is None: - download_dir = standarddir.download + download_dir = standarddir.download() self._filename = os.path.join(download_dir, filename) self.basename = filename log.downloads.debug("Setting filename to {}".format(filename)) diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index f11c647b8..1f7a8dbd2 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -64,8 +64,8 @@ class WebHistory(QWebHistoryInterface): def __init__(self, parent=None): super().__init__(parent) - self._linecp = lineparser.LineConfigParser(standarddir.data, 'history', - parent=self) + self._linecp = lineparser.LineConfigParser( + standarddir.data(), 'history', parent=self) self._history = [HistoryEntry.from_str(e) for e in self._linecp.data] objreg.get('save-manager').add_saveable('history', self.save, self.changed) diff --git a/qutebrowser/browser/quickmarks.py b/qutebrowser/browser/quickmarks.py index 389aee7dd..9224baa5c 100644 --- a/qutebrowser/browser/quickmarks.py +++ b/qutebrowser/browser/quickmarks.py @@ -53,7 +53,7 @@ class QuickmarkManager(QObject): self.marks = collections.OrderedDict() self._linecp = lineparser.LineConfigParser( - standarddir.config, 'quickmarks', parent=self) + standarddir.config(), 'quickmarks', parent=self) for line in self._linecp: try: key, url = line.rsplit(maxsplit=1) @@ -61,7 +61,7 @@ class QuickmarkManager(QObject): message.error(0, "Invalid quickmark '{}'".format(line)) else: self.marks[key] = url - filename = os.path.join(standarddir.config, 'quickmarks') + filename = os.path.join(standarddir.config(), 'quickmarks') objreg.get('save-manager').add_saveable('quickmark-manager', self.save, self.changed, filename=filename) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index bac2f64fc..c60a32868 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -185,7 +185,7 @@ class _POSIXUserscriptRunner(_BaseUserscriptRunner): # os.mkfifo will raise an exception anyways when the path doesn't # exist, it shouldn't be a big issue. self._filepath = tempfile.mktemp(prefix='qutebrowser-userscript-', - dir=standarddir.runtime) + dir=standarddir.runtime()) os.mkfifo(self._filepath) # pylint: disable=no-member except OSError as e: message.error(self._win_id, "Error while creating FIFO: {}".format( diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index b7521e712..a0157fc76 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -117,7 +117,8 @@ def _init_main_config(): """Initialize the main config.""" try: app = objreg.get('app') - config_obj = ConfigManager(standarddir.config, 'qutebrowser.conf', app) + config_obj = ConfigManager(standarddir.config(), 'qutebrowser.conf', + app) except (configexc.Error, configparser.Error, UnicodeDecodeError) as e: log.init.exception(e) errstr = "Error while reading config:" @@ -134,8 +135,8 @@ def _init_main_config(): sys.exit(1) else: objreg.register('config', config_obj) - if standarddir.config is not None: - filename = os.path.join(standarddir.config, 'qutebrowser.conf') + if standarddir.config() is not None: + filename = os.path.join(standarddir.config(), 'qutebrowser.conf') save_manager = objreg.get('save-manager') save_manager.add_saveable( 'config', config_obj.save, config_obj.changed, @@ -152,7 +153,7 @@ def _init_main_config(): def _init_key_config(): """Initialize the key config.""" try: - key_config = keyconf.KeyConfigParser(standarddir.config, 'keys.conf') + key_config = keyconf.KeyConfigParser(standarddir.config(), 'keys.conf') except (keyconf.KeyConfigError, UnicodeDecodeError) as e: log.init.exception(e) errstr = "Error while reading key config:\n" @@ -166,9 +167,9 @@ def _init_key_config(): sys.exit(1) else: objreg.register('key-config', key_config) - if standarddir.config is not None: + if standarddir.config() is not None: save_manager = objreg.get('save-manager') - filename = os.path.join(standarddir.config, 'keys.conf') + filename = os.path.join(standarddir.config(), 'keys.conf') save_manager.add_saveable( 'key-config', key_config.save, key_config.changed, config_opt=('general', 'auto-save-config'), filename=filename) @@ -177,13 +178,13 @@ def _init_key_config(): def _init_misc(): """Initialize misc. config-related files.""" save_manager = objreg.get('save-manager') - state_config = ini.ReadWriteConfigParser(standarddir.data, 'state') + state_config = ini.ReadWriteConfigParser(standarddir.data(), 'state') objreg.register('state-config', state_config) save_manager.add_saveable('state-config', state_config.save) # We need to import this here because lineparser needs config. from qutebrowser.config.parsers import line - command_history = line.LineConfigParser(standarddir.data, 'cmd-history', + command_history = line.LineConfigParser(standarddir.data(), 'cmd-history', ('completion', 'history-length'), parent=objreg.get('config')) objreg.register('command-history', command_history) @@ -197,10 +198,10 @@ def _init_misc(): # This fixes one of the corruption issues here: # https://github.com/The-Compiler/qutebrowser/issues/515 - if standarddir.config is None: + if standarddir.config() is None: path = os.devnull else: - path = os.path.join(standarddir.config, 'qsettings') + path = os.path.join(standarddir.config(), 'qsettings') for fmt in (QSettings.NativeFormat, QSettings.IniFormat): QSettings.setPath(fmt, QSettings.UserScope, path) diff --git a/qutebrowser/config/websettings.py b/qutebrowser/config/websettings.py index eddbe9ef3..46ec84916 100644 --- a/qutebrowser/config/websettings.py +++ b/qutebrowser/config/websettings.py @@ -194,13 +194,13 @@ def _set_setting(typ, arg, default=UNSET, value=UNSET): def init(): """Initialize the global QWebSettings.""" - QWebSettings.setIconDatabasePath(standarddir.cache) + QWebSettings.setIconDatabasePath(standarddir.cache()) QWebSettings.setOfflineWebApplicationCachePath( - os.path.join(standarddir.cache, 'application-cache')) + os.path.join(standarddir.cache(), 'application-cache')) QWebSettings.globalSettings().setLocalStoragePath( - os.path.join(standarddir.data, 'local-storage')) + os.path.join(standarddir.data(), 'local-storage')) QWebSettings.setOfflineStoragePath( - os.path.join(standarddir.data, 'offline-storage')) + os.path.join(standarddir.data(), 'offline-storage')) global settings settings = QWebSettings.globalSettings() diff --git a/qutebrowser/misc/sessions.py b/qutebrowser/misc/sessions.py index 8a1137250..8c60dc162 100644 --- a/qutebrowser/misc/sessions.py +++ b/qutebrowser/misc/sessions.py @@ -64,7 +64,7 @@ class SessionManager(QObject): def __init__(self, parent=None): super().__init__(parent) - self._base_path = os.path.join(standarddir.data, 'sessions') + self._base_path = os.path.join(standarddir.data(), 'sessions') self._last_window_session = None if not os.path.exists(self._base_path): os.mkdir(self._base_path) diff --git a/qutebrowser/test/utils/test_standarddir.py b/qutebrowser/test/utils/test_standarddir.py index ff34760ca..8703eed5d 100644 --- a/qutebrowser/test/utils/test_standarddir.py +++ b/qutebrowser/test/utils/test_standarddir.py @@ -32,7 +32,7 @@ from qutebrowser.test import helpers, qApp class GetStandardDirLinuxTests(unittest.TestCase): - """Tests for standarddir.get under Linux. + """Tests for standarddir under Linux. Attributes: temp_dir: A temporary directory. @@ -50,7 +50,7 @@ class GetStandardDirLinuxTests(unittest.TestCase): with helpers.environ_set_temp({'XDG_DATA_HOME': self.temp_dir}): standarddir.init(None) expected = os.path.join(self.temp_dir, 'qutebrowser') - self.assertEqual(standarddir.data, expected) + self.assertEqual(standarddir.data(), expected) @unittest.skipUnless(sys.platform.startswith("linux"), "requires Linux") def test_config_explicit(self): @@ -58,7 +58,7 @@ class GetStandardDirLinuxTests(unittest.TestCase): with helpers.environ_set_temp({'XDG_CONFIG_HOME': self.temp_dir}): standarddir.init(None) expected = os.path.join(self.temp_dir, 'qutebrowser') - self.assertEqual(standarddir.config, expected) + self.assertEqual(standarddir.config(), expected) @unittest.skipUnless(sys.platform.startswith("linux"), "requires Linux") def test_cache_explicit(self): @@ -66,7 +66,7 @@ class GetStandardDirLinuxTests(unittest.TestCase): with helpers.environ_set_temp({'XDG_CACHE_HOME': self.temp_dir}): standarddir.init(None) expected = os.path.join(self.temp_dir, 'qutebrowser') - self.assertEqual(standarddir.cache, expected) + self.assertEqual(standarddir.cache(), expected) @unittest.skipUnless(sys.platform.startswith("linux"), "requires Linux") def test_data(self): @@ -76,7 +76,7 @@ class GetStandardDirLinuxTests(unittest.TestCase): standarddir.init(None) expected = os.path.join(self.temp_dir, '.local', 'share', 'qutebrowser') - self.assertEqual(standarddir.data, expected) + self.assertEqual(standarddir.data(), expected) @unittest.skipUnless(sys.platform.startswith("linux"), "requires Linux") def test_config(self): @@ -85,7 +85,7 @@ class GetStandardDirLinuxTests(unittest.TestCase): with helpers.environ_set_temp(env): standarddir.init(None) expected = os.path.join(self.temp_dir, '.config', 'qutebrowser') - self.assertEqual(standarddir.config, expected) + self.assertEqual(standarddir.config(), expected) @unittest.skipUnless(sys.platform.startswith("linux"), "requires Linux") def test_cache(self): @@ -94,7 +94,7 @@ class GetStandardDirLinuxTests(unittest.TestCase): with helpers.environ_set_temp(env): standarddir.init(None) expected = os.path.join(self.temp_dir, '.cache', 'qutebrowser') - self.assertEqual(standarddir.cache, expected) + self.assertEqual(standarddir.cache(), expected) def tearDown(self): qApp.setApplicationName(self.old_name) @@ -103,7 +103,7 @@ class GetStandardDirLinuxTests(unittest.TestCase): class GetStandardDirWindowsTests(unittest.TestCase): - """Tests for standarddir.get under Windows. + """Tests for standarddir under Windows. Attributes: old_name: The old applicationName. @@ -121,18 +121,18 @@ class GetStandardDirWindowsTests(unittest.TestCase): @unittest.skipUnless(sys.platform.startswith("win"), "requires Windows") def test_data(self): """Test data dir.""" - self.assertEqual(standarddir.data.split(os.sep)[-2:], - ['qutebrowser_test', 'data'], standarddir.data) + self.assertEqual(standarddir.data().split(os.sep)[-2:], + ['qutebrowser_test', 'data'], standarddir.data()) @unittest.skipUnless(sys.platform.startswith("win"), "requires Windows") def test_config(self): """Test config dir.""" - self.assertEqual(standarddir.config.split(os.sep)[-1], + self.assertEqual(standarddir.config().split(os.sep)[-1], 'qutebrowser_test', - standarddir.config) + standarddir.config()) @unittest.skipUnless(sys.platform.startswith("win"), "requires Windows") def test_cache(self): """Test cache dir.""" - self.assertEqual(standarddir.cache.split(os.sep)[-2:], - ['qutebrowser_test', 'cache'], standarddir.cache) + self.assertEqual(standarddir.cache().split(os.sep)[-2:], + ['qutebrowser_test', 'cache'], standarddir.cache()) diff --git a/qutebrowser/utils/standarddir.py b/qutebrowser/utils/standarddir.py index 51a522e38..a3f608fa1 100644 --- a/qutebrowser/utils/standarddir.py +++ b/qutebrowser/utils/standarddir.py @@ -27,11 +27,33 @@ from PyQt5.QtCore import QCoreApplication, QStandardPaths from qutebrowser.utils import log, qtutils -config = None -data = None -cache = None -download = None -runtime = None +# The argparse namespace passed to init() +_args = None + + +def config(): + """Convenience function to get the config location.""" + return _get(QStandardPaths.ConfigLocation) + + +def data(): + """Convenience function to get the data location.""" + return _get(QStandardPaths.DataLocation) + + +def cache(): + """Convenience function to get the cache location.""" + return _get(QStandardPaths.CacheLocation) + + +def download(): + """Convenience function to get the download location.""" + return _get(QStandardPaths.DownloadLocation) + + +def runtime(): + """Convenience function to get the runtime location.""" + return _get(QStandardPaths.RuntimeLocation) def _writable_location(typ): @@ -75,16 +97,14 @@ def _from_args(typ, args): return (True, arg_value) -def _get(typ, args=None): +def _get(typ): """Get the directory where files of the given type should be written to. Args: typ: A member of the QStandardPaths::StandardLocation enum, see http://qt-project.org/doc/qt-5/qstandardpaths.html#StandardLocation-enum - args: An argparse namespace which could be used to override the - locations. """ - overridden, path = _from_args(typ, args) + overridden, path = _from_args(typ, _args) if not overridden: path = _writable_location(typ) appname = QCoreApplication.instance().applicationName() @@ -101,6 +121,11 @@ def _get(typ, args=None): QStandardPaths.ConfigLocation) if data_path == config_path: path = os.path.join(path, 'data') + # From the XDG basedir spec: + # If, when attempting to write a file, the destination directory is + # non-existant an attempt should be made to create it with permission + # 0700. If the destination directory exists already the permissions + # should not be changed. if path is not None and not os.path.exists(path): os.makedirs(path, 0o700) return path @@ -108,26 +133,10 @@ def _get(typ, args=None): def init(args): """Initialize all standard dirs.""" - global config, data, cache, download, runtime - config = _get(QStandardPaths.ConfigLocation, args) - data = _get(QStandardPaths.DataLocation, args) - cache = _get(QStandardPaths.CacheLocation, args) - download = _get(QStandardPaths.DownloadLocation, args) - runtime = _get(QStandardPaths.RuntimeLocation, args) - # From the XDG basedir spec: - # If, when attempting to write a file, the destination directory is - # non-existant an attempt should be made to create it with permission - # 0700. If the destination directory exists already the permissions - # should not be changed. - # - # This is slightly wrong according to the standard as we ensure these paths - # exists while initializing, not when writing the file - but practicality - # beats purity. - for path in (config, data, cache): - if path is not None and not os.path.exists(path): - os.makedirs(path, 0o700) + global _args + _args = args # http://www.brynosaurus.com/cachedir/spec.html - cachedir_tag = os.path.join(cache, 'CACHEDIR.TAG') + cachedir_tag = os.path.join(cache(), 'CACHEDIR.TAG') if not os.path.exists(cachedir_tag): try: with open(cachedir_tag, 'w', encoding='utf-8') as f: