Introduce standarddir caching

This makes things a bit more complicated, but is needed to make standarddir (and
thus the config) work without a QApplication.

qutebrowser/utils/standarddir.py

@@ -26,11 +26,14 @@ import os.path
 
 from PyQt5.QtCore import QCoreApplication, QStandardPaths
 
-from qutebrowser.utils import log, qtutils, debug
+from qutebrowser.utils import log, qtutils, debug, usertypes
+
+# The cached locations
+_locations = {}
 
 
-# The argparse namespace passed to init()
+Location = usertypes.enum('Location', ['config', 'data', 'system_data',
-_args = None
+                                       'cache', 'download', 'runtime'])
 
 
 class EmptyValueError(Exception):
@@ -38,10 +41,10 @@ class EmptyValueError(Exception):
     """Error raised when QStandardPaths returns an empty value."""
 
 
-def config():
+def _init_config(args):
-    """Get a location for configs."""
+    """Initialize the location for configs."""
     typ = QStandardPaths.ConfigLocation
-    overridden, path = _from_args(typ, _args)
+    overridden, path = _from_args(typ, args)
     if not overridden:
         path = _writable_location(typ)
         appname = QCoreApplication.instance().applicationName()
@@ -50,13 +53,17 @@ def config():
             # https://bugreports.qt.io/browse/QTBUG-38872
             path = os.path.join(path, appname)
     _create(path)
-    return path
+    _locations[Location.config] = path
 
 
-def data():
+def config():
-    """Get a location for data."""
+    return _locations[Location.config]
+
+
+def _init_data(args):
+    """Initialize the location for data."""
     typ = QStandardPaths.DataLocation
-    overridden, path = _from_args(typ, _args)
+    overridden, path = _from_args(typ, args)
     if not overridden:
         path = _writable_location(typ)
         if os.name == 'nt':
@@ -68,49 +75,71 @@ def data():
             if data_path == config_path:
                 path = os.path.join(path, 'data')
     _create(path)
-    return path
+    _locations[Location.data] = path
+
+
+def data():
+    return _locations[Location.data]
+
+
+def _init_system_data(_args):
+    """Initialize the location for system-wide data.
+
+    This path may be read-only."""
+    _locations.pop(Location.system_data, None)  # Remove old state
+    if sys.platform.startswith('linux'):
+        path = "/usr/share/qutebrowser"
+        if os.path.exists(path):
+            _locations[Location.system_data] = path
 
 
 def system_data():
-    """Get a location for system-wide data. This path may be read-only."""
+    try:
-    if sys.platform.startswith('linux'):
+        return _locations[Location.system_data]
-        path = "/usr/share/qutebrowser"
+    except KeyError:
-        if not os.path.exists(path):
+        return _locations[Location.data]
-            path = data()
+
-    else:
+
-        path = data()
+def _init_cache(args):
-    return path
+    """Initialize the location for the cache."""
+    typ = QStandardPaths.CacheLocation
+    overridden, path = _from_args(typ, args)
+    if not overridden:
+        path = _writable_location(typ)
+    _create(path)
+    _locations[Location.cache] = path
 
 
 def cache():
-    """Get a location for the cache."""
+    return _locations[Location.cache]
-    typ = QStandardPaths.CacheLocation
+
-    overridden, path = _from_args(typ, _args)
+
+def _init_download(args):
+    """Initialize the location for downloads.
+
+    Note this is only the default directory as found by Qt.
+    """
+    typ = QStandardPaths.DownloadLocation
+    overridden, path = _from_args(typ, args)
     if not overridden:
         path = _writable_location(typ)
     _create(path)
-    return path
+    _locations[Location.download] = path
 
 
 def download():
-    """Get a location for downloads."""
+    return _locations[Location.download]
-    typ = QStandardPaths.DownloadLocation
-    overridden, path = _from_args(typ, _args)
-    if not overridden:
-        path = _writable_location(typ)
-    _create(path)
-    return path
 
 
-def runtime():
+def _init_runtime(args):
-    """Get a location for runtime data."""
+    """Initialize location for runtime data."""
     if sys.platform.startswith('linux'):
         typ = QStandardPaths.RuntimeLocation
-    else:  # pragma: no cover
+    else:
         # RuntimeLocation is a weird path on macOS and Windows.
         typ = QStandardPaths.TempLocation
 
-    overridden, path = _from_args(typ, _args)
+    overridden, path = _from_args(typ, args)
 
     if not overridden:
         try:
@@ -132,7 +161,11 @@ def runtime():
         appname = QCoreApplication.instance().applicationName()
         path = os.path.join(path, appname)
     _create(path)
-    return path
+    _locations[Location.runtime] = path
+
+
+def runtime():
+    return _locations[Location.runtime]
 
 
 def _writable_location(typ):
@@ -195,13 +228,26 @@ def _create(path):
         pass
 
 
+def _init_dirs(args=None):
+    """Create and cache standard directory locations.
+
+    Mainly in a separate function because we need to call it in tests.
+    """
+    _init_config(args)
+    _init_data(args)
+    _init_system_data(args)
+    _init_cache(args)
+    _init_download(args)
+    _init_runtime(args)
+
+
 def init(args):
     """Initialize all standard dirs."""
-    global _args
     if args is not None:
         # args can be None during tests
         log.init.debug("Base directory: {}".format(args.basedir))
-    _args = args
+
+    _init_dirs(args)
     _init_cachedir_tag()
     if args is not None:
         _move_webengine_data()

tests/conftest.py

@@ -35,7 +35,7 @@ from helpers import logfail
 from helpers.logfail import fail_on_logging
 from helpers.messagemock import message_mock
 from helpers.fixtures import *
-from qutebrowser.utils import qtutils
+from qutebrowser.utils import qtutils, standarddir
 
 import qutebrowser.app  # To register commands
 
@@ -157,6 +157,14 @@ def qapp(qapp):
     return qapp
 
 
+@pytest.fixture
+def init_standarddir(qapp):
+    """Initialize the standarddir module for tests which need it."""
+    standarddir._init_dirs()
+    yield
+    standarddir._locations = {}
+
+
 def pytest_addoption(parser):
     parser.addoption('--qute-delay', action='store', default=0, type=int,
                      help="Delay between qutebrowser commands.")

tests/unit/browser/test_adblock.py

@@ -316,7 +316,8 @@ def test_failed_dl_update(config_stub, basedir, download_stub,
 
 
 @pytest.mark.parametrize('location', ['content', 'comment'])
-def test_invalid_utf8(config_stub, download_stub, tmpdir, caplog, location):
+def test_invalid_utf8(config_stub, download_stub, init_standarddir, tmpdir,
+                      caplog, location):
     """Make sure invalid UTF-8 is handled correctly.
 
     See https://github.com/qutebrowser/qutebrowser/issues/2301

tests/unit/commands/test_userscripts.py

@@ -59,7 +59,7 @@ class TestQtFIFOReader:
     userscripts._POSIXUserscriptRunner,
     userscripts._WindowsUserscriptRunner,
 ])
-def runner(request):
+def runner(request, init_standarddir):
     if (os.name != 'posix' and
             request.param is userscripts._POSIXUserscriptRunner):
         pytest.skip("Requires a POSIX os")

tests/unit/misc/test_ipc.py

@@ -35,7 +35,7 @@ from PyQt5.QtTest import QSignalSpy
 
 import qutebrowser
 from qutebrowser.misc import ipc
-from qutebrowser.utils import objreg, qtutils
+from qutebrowser.utils import objreg, qtutils, standarddir
 from helpers import stubs
 
 
@@ -88,6 +88,7 @@ def qlocalsocket(qapp):
 @pytest.fixture(autouse=True)
 def fake_runtime_dir(monkeypatch, short_tmpdir):
     monkeypatch.setenv('XDG_RUNTIME_DIR', str(short_tmpdir))
+    standarddir._init_dirs()
     return short_tmpdir
 
 

tests/unit/utils/test_standarddir.py

@@ -32,6 +32,9 @@ import pytest
 from qutebrowser.utils import standarddir
 
 
+pytestmark = pytest.mark.usefixtures('qapp')
+
+
 @pytest.fixture(autouse=True)
 def change_qapp_name(qapp):
     """Change the name of the QApplication instance.
@@ -45,21 +48,14 @@ def change_qapp_name(qapp):
     qapp.setApplicationName(old_name)
 
 
-@pytest.fixture
+@pytest.fixture(autouse=True)
-def no_cachedir_tag(monkeypatch):
+def clear_standarddir_cache(monkeypatch):
-    """Fixture to prevent writing a CACHEDIR.TAG."""
+    """Make sure the standarddir cache is cleared before/after each test."""
-    monkeypatch.setattr(standarddir, '_init_cachedir_tag', lambda: None)
+    monkeypatch.setattr(standarddir, '_locations', {})
-
-
-@pytest.fixture
-def reset_standarddir(no_cachedir_tag):
-    """Clean up standarddir arguments before and after each test."""
-    standarddir.init(None)
     yield
-    standarddir.init(None)
+    monkeypatch.setattr(standarddir, '_locations', {})
 
 
-@pytest.mark.usefixtures('reset_standarddir')
 @pytest.mark.parametrize('data_subdir, config_subdir, expected', [
     ('foo', 'foo', 'foo/data'),
     ('foo', 'bar', 'foo'),
@@ -74,11 +70,11 @@ def test_get_fake_windows_equal_dir(data_subdir, config_subdir, expected,
     monkeypatch.setattr(standarddir.os, 'name', 'nt')
     monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation',
                         locations.get)
+    standarddir._init_data(args=None)
     expected = str(tmpdir / expected)
     assert standarddir.data() == expected
 
 
-@pytest.mark.usefixtures('reset_standarddir')
 class TestWritableLocation:
 
     """Tests for _writable_location."""
@@ -99,7 +95,6 @@ class TestWritableLocation:
         assert '\\' in loc
 
 
-@pytest.mark.usefixtures('reset_standarddir')
 class TestStandardDir:
 
     """Tests for standarddir."""
@@ -119,6 +114,7 @@ class TestStandardDir:
             varname: The environment variable which should be set.
         """
         monkeypatch.setenv(varname, str(tmpdir))
+        standarddir._init_dirs()
         assert func() == str(tmpdir / 'qute_test')
 
     @pytest.mark.parametrize('func, subdirs', [
@@ -133,6 +129,7 @@ class TestStandardDir:
         monkeypatch.setenv('HOME', str(tmpdir))
         for var in ['DATA', 'CONFIG', 'CACHE']:
             monkeypatch.delenv('XDG_{}_HOME'.format(var), raising=False)
+        standarddir._init_dirs()
         assert func() == str(tmpdir.join(*subdirs))
 
     @pytest.mark.linux
@@ -141,6 +138,7 @@ class TestStandardDir:
         """With invalid XDG_RUNTIME_DIR, fall back to TempLocation."""
         monkeypatch.setenv('XDG_RUNTIME_DIR', str(tmpdir / 'does-not-exist'))
         monkeypatch.setenv('TMPDIR', str(tmpdir / 'temp'))
+        standarddir._init_dirs()
         assert standarddir.runtime() == str(tmpdir / 'temp' / 'qute_test')
 
     def test_runtimedir_empty_tempdir(self, monkeypatch, tmpdir):
@@ -149,7 +147,7 @@ class TestStandardDir:
         monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation',
                             lambda typ: '')
         with pytest.raises(standarddir.EmptyValueError):
-            standarddir.runtime()
+            standarddir._init_runtime(args=None)
 
     @pytest.mark.parametrize('func, elems, expected', [
         (standarddir.data, 2, ['qute_test', 'data']),
@@ -159,6 +157,7 @@ class TestStandardDir:
     ])
     @pytest.mark.windows
     def test_windows(self, func, elems, expected):
+        standarddir._init_dirs()
         assert func().split(os.sep)[-elems:] == expected
 
     @pytest.mark.parametrize('func, elems, expected', [
@@ -169,13 +168,13 @@ class TestStandardDir:
     ])
     @pytest.mark.mac
     def test_mac(self, func, elems, expected):
+        standarddir._init_dirs()
         assert func().split(os.sep)[-elems:] == expected
 
 
 DirArgTest = collections.namedtuple('DirArgTest', 'arg, expected')
 
 
-@pytest.mark.usefixtures('reset_standarddir')
 class TestArguments:
 
     """Tests the --basedir argument."""
@@ -187,7 +186,7 @@ class TestArguments:
         """Test --basedir."""
         expected = str(tmpdir / typ)
         args = types.SimpleNamespace(basedir=str(tmpdir))
-        standarddir.init(args)
+        standarddir._init_dirs(args)
         func = getattr(standarddir, typ)
         assert func() == expected
 
@@ -197,7 +196,7 @@ class TestArguments:
         basedir.ensure(dir=True)
         with tmpdir.as_cwd():
             args = types.SimpleNamespace(basedir='basedir')
-            standarddir.init(args)
+            standarddir._init_dirs(args)
             assert standarddir.config() == str(basedir / 'config')
 
 
@@ -252,7 +251,7 @@ class TestCreatingDir:
         basedir = tmpdir / 'basedir'
         assert not basedir.exists()
         args = types.SimpleNamespace(basedir=str(basedir))
-        standarddir.init(args)
+        standarddir._init_dirs(args)
 
         func = getattr(standarddir, typ)
         func()
@@ -262,7 +261,6 @@ class TestCreatingDir:
         if os.name == 'posix':
             assert basedir.stat().mode & 0o777 == 0o700
 
-    @pytest.mark.usefixtures('reset_standarddir')
     @pytest.mark.parametrize('typ', DIR_TYPES)
     def test_exists_race_condition(self, mocker, tmpdir, typ):
         """Make sure there can't be a TOCTOU issue when creating the file.
@@ -279,13 +277,12 @@ class TestCreatingDir:
         m.path.abspath = lambda x: x
 
         args = types.SimpleNamespace(basedir=str(tmpdir))
-        standarddir.init(args)
+        standarddir._init_dirs(args)
 
         func = getattr(standarddir, typ)
         func()
 
 
-@pytest.mark.usefixtures('reset_standarddir')
 class TestSystemData:
 
     """Test system data path."""
@@ -294,6 +291,7 @@ class TestSystemData:
         """Test that /usr/share/qutebrowser is used if path exists."""
         monkeypatch.setattr('sys.platform', "linux")
         monkeypatch.setattr(os.path, 'exists', lambda path: True)
+        standarddir._init_dirs()
         assert standarddir.system_data() == "/usr/share/qutebrowser"
 
     @pytest.mark.linux
@@ -301,16 +299,16 @@ class TestSystemData:
                                             fake_args):
         """Test that system-wide path isn't used on linux if path not exist."""
         fake_args.basedir = str(tmpdir)
-        standarddir.init(fake_args)
         monkeypatch.setattr(os.path, 'exists', lambda path: False)
+        standarddir._init_dirs(fake_args)
         assert standarddir.system_data() == standarddir.data()
 
     def test_system_datadir_unsupportedos(self, monkeypatch, tmpdir,
                                           fake_args):
         """Test that system-wide path is not used on non-Linux OS."""
         fake_args.basedir = str(tmpdir)
-        standarddir.init(fake_args)
         monkeypatch.setattr('sys.platform', "potato")
+        standarddir._init_dirs(fake_args)
         assert standarddir.system_data() == standarddir.data()
 
 
@@ -411,3 +409,25 @@ class TestMoveWebEngineData:
         record = caplog.records[-1]
         expected = "Failed to move old QtWebEngine data/cache: error"
         assert record.message == expected
+
+
+@pytest.mark.parametrize('with_args', [True, False])
+def test_init(mocker, tmpdir, with_args):
+    """Do some sanity checks for standarddir.init().
+
+    Things like _init_cachedir_tag() and _move_webengine_data() are tested in
+    more detail in other tests.
+    """
+    assert standarddir._locations == {}
+
+    if with_args:
+        args = types.SimpleNamespace(basedir=str(tmpdir))
+        m = mocker.patch('qutebrowser.utils.standarddir._move_webengine_data')
+    else:
+        args = None
+
+    standarddir.init(args)
+
+    assert standarddir._locations != {}
+    if with_args:
+        assert m.called