Make moving data in standarddir more generic

qutebrowser/utils/standarddir.py

@@ -26,7 +26,7 @@ import os.path
 
 from PyQt5.QtCore import QCoreApplication, QStandardPaths
 
-from qutebrowser.utils import log, qtutils, debug, usertypes
+from qutebrowser.utils import log, qtutils, debug, usertypes, message
 
 # The cached locations
 _locations = {}
@@ -46,13 +46,20 @@ def _init_config(args):
     typ = QStandardPaths.ConfigLocation
     overridden, path = _from_args(typ, args)
     if not overridden:
-        path = _writable_location(typ)
+        if os.name == 'nt':
-        appname = QCoreApplication.instance().applicationName()
+            app_data_path = _writable_location(
-        if path.split(os.sep)[-1] != appname:  # pragma: no branch
+                QStandardPaths.AppDataLocation)
-            # WORKAROUND - see
+            path = os.path.join(app_data_path, 'config')
-            # https://bugreports.qt.io/browse/QTBUG-38872
+        else:
-            path = os.path.join(path, appname)
+            path = _writable_location(typ)
+            appname = QCoreApplication.instance().applicationName()
+            if path.split(os.sep)[-1] != appname:  # pragma: no branch
+                # WORKAROUND - see
+                # https://bugreports.qt.io/browse/QTBUG-38872
+                path = os.path.join(path, appname)
     _create(path)
+    if sys.platform == 'darwin':  # pragma: no cover
+        _create(os.path.expanduser('~/.qutebrowser'))
     _locations[Location.config] = path
 
 
@@ -62,7 +69,8 @@ def config(auto=False):
     If auto=True is given, get the location for the autoconfig.yml directory,
     which is different on macOS.
     """
-    # FIXME:conf handle auto=True
+    if auto and sys.platform == 'darwin':
+        return os.path.expanduser('~/.qutebrowser')
     return _locations[Location.config]
 
 
@@ -71,15 +79,12 @@ def _init_data(args):
     typ = QStandardPaths.DataLocation
     overridden, path = _from_args(typ, args)
     if not overridden:
-        path = _writable_location(typ)
         if os.name == 'nt':
-            # Under windows, config/data might end up in the same directory.
+            app_data_path = _writable_location(
-            data_path = QStandardPaths.writableLocation(
+                QStandardPaths.AppDataLocation)
-                QStandardPaths.DataLocation)
+            path = os.path.join(app_data_path, 'data')
-            config_path = QStandardPaths.writableLocation(
+        else:
-                QStandardPaths.ConfigLocation)
+            path = _writable_location(typ)
-            if data_path == config_path:
-                path = os.path.join(path, 'data')
     _create(path)
     _locations[Location.data] = path
 
@@ -255,6 +260,42 @@ def init(args):
     _init_cachedir_tag()
     if args is not None:
         _move_webengine_data()
+        if sys.platform == 'darwin':  # pragma: no cover
+            _move_macos()
+        elif os.name == 'nt':  # pragma: no cover
+            _move_windows()
+
+
+def _move_macos():
+    """Move most config files to new location on macOS."""
+    old_config = config(auto=True)  # ~/Library/Preferences/qutebrowser
+    new_config = config()  # ~/.qutebrowser
+    for f in os.listdir(old_config):
+        if f not in ['qsettings', 'autoconfig.yml']:
+            _move_data(os.path.join(old_config, f),
+                       os.path.join(new_config, f))
+
+
+def _move_windows():
+    """Move the whole qutebrowser directory from Local to Roaming AppData."""
+    # %APPDATA%\Local\qutebrowser
+    old_appdata_dir = _writable_location(QStandardPaths.DataLocation)
+    # %APPDATA%\Roaming\qutebrowser
+    new_appdata_dir = _writable_location(QStandardPaths.AppDataLocation)
+
+    # data subfolder
+    old_data = os.path.join(old_appdata_dir, 'data')
+    new_data = os.path.join(new_appdata_dir, 'data')
+    ok = _move_data(old_data, new_data)
+    if not ok:  # pragma: no cover
+        return
+
+    # config files
+    new_config_dir = os.path.join(new_appdata_dir, 'config')
+    _create(new_config_dir)
+    for f in os.listdir(old_appdata_dir):
+        _move_data(os.path.join(old_appdata_dir, f),
+                   os.path.join(new_config_dir, f))
 
 
 def _init_cachedir_tag():
@@ -280,43 +321,53 @@ def _move_webengine_data():
     """Move QtWebEngine data from an older location to the new one."""
     # Do NOT use _writable_location here as that'd give us a wrong path
     old_data_dir = QStandardPaths.writableLocation(QStandardPaths.DataLocation)
+    new_data_dir = os.path.join(data(), 'webengine')
+    ok = _move_data(os.path.join(old_data_dir, 'QtWebEngine', 'Default'),
+                    new_data_dir)
+    if not ok:
+        return
+
     old_cache_dir = QStandardPaths.writableLocation(
         QStandardPaths.CacheLocation)
-    new_data_dir = os.path.join(data(), 'webengine')
     new_cache_dir = os.path.join(cache(), 'webengine')
-
+    ok = _move_data(os.path.join(old_cache_dir, 'QtWebEngine', 'Default'),
-    if (not os.path.exists(os.path.join(old_data_dir, 'QtWebEngine')) and
+                    new_cache_dir)
-            not os.path.exists(os.path.join(old_cache_dir, 'QtWebEngine'))):
+    if not ok:
         return
 
-    log.init.debug("Moving QtWebEngine data from {} to {}".format(
+    # Remove e.g.
-        old_data_dir, new_data_dir))
+    # ~/.local/share/qutebrowser/qutebrowser/QtWebEngine/Default
-    log.init.debug("Moving QtWebEngine cache from {} to {}".format(
+    if old_data_dir.split(os.sep)[-2:] == ['qutebrowser', 'qutebrowser']:
-        old_cache_dir, new_cache_dir))
+        log.init.debug("Removing {} / {}".format(
+            old_data_dir, old_cache_dir))
+        for old_dir in old_data_dir, old_cache_dir:
+            os.rmdir(os.path.join(old_dir, 'QtWebEngine'))
+            os.rmdir(old_dir)
 
-    if os.path.exists(new_data_dir):
+
-        log.init.warning("Failed to move old QtWebEngine data as {} already "
+def _move_data(old, new):
-                         "exists!".format(new_data_dir))
+    """Migrate data from an old to a new directory.
-        return
+
-    if os.path.exists(new_cache_dir):
+    If the old directory does not exist, the migration is skipped.
-        log.init.warning("Failed to move old QtWebEngine cache as {} already "
+    If the new directory already exists, an error is shown.
-                         "exists!".format(new_cache_dir))
+
-        return
+    Return: True if moving succeeded, False otherwise.
+    """
+    if not os.path.exists(old):
+        return False
+
+    log.init.debug("Migrating data from {} to {}".format(old, new))
+
+    if os.path.exists(new):
+        message.error("Failed to move data from {} as {} already exists!"
+                      .format(old, new))
+        return False
 
     try:
-        shutil.move(os.path.join(old_data_dir, 'QtWebEngine', 'Default'),
+        shutil.move(old, new)
-                    new_data_dir)
-        shutil.move(os.path.join(old_cache_dir, 'QtWebEngine', 'Default'),
-                    new_cache_dir)
-
-        # Remove e.g.
-        # ~/.local/share/qutebrowser/qutebrowser/QtWebEngine/Default
-        if old_data_dir.split(os.sep)[-2:] == ['qutebrowser', 'qutebrowser']:
-            log.init.debug("Removing {} / {}".format(
-                old_data_dir, old_cache_dir))
-            for old_dir in old_data_dir, old_cache_dir:
-                os.rmdir(os.path.join(old_dir, 'QtWebEngine'))
-                os.rmdir(old_dir)
     except OSError as e:
-        log.init.exception("Failed to move old QtWebEngine data/cache: "
+        message.error("Failed to move data from {} to {}: {}".format(
-                           "{}".format(e))
+            old, new, e))
+        return False
+
+    return True

tests/unit/utils/test_standarddir.py

@@ -20,6 +20,7 @@
 """Tests for qutebrowser.utils.standarddir."""
 
 import os
+import sys
 import os.path
 import types
 import collections
@@ -43,23 +44,24 @@ def clear_standarddir_cache(monkeypatch):
     monkeypatch.setattr(standarddir, '_locations', {})
 
 
-@pytest.mark.parametrize('data_subdir, config_subdir, expected', [
+def test_fake_mac_auto_config(tmpdir, monkeypatch):
-    ('foo', 'foo', 'foo/data'),
+    """Test standardir.config(auto=True) on a fake Mac."""
-    ('foo', 'bar', 'foo'),
+    monkeypatch.setattr(sys, 'platform', 'darwin')
-])
+    monkeypatch.setenv('HOME', str(tmpdir))
-def test_get_fake_windows_equal_dir(data_subdir, config_subdir, expected,
+    expected = str(tmpdir) + '/.qutebrowser'  # always with /
-                                    monkeypatch, tmpdir):
+    assert standarddir.config(auto=True) == expected
-    """Test _get with a fake Windows OS with equal data/config dirs."""
+
-    locations = {
+
-        QStandardPaths.DataLocation: str(tmpdir / data_subdir),
+@pytest.mark.parametrize('what', ['data', 'config'])
-        QStandardPaths.ConfigLocation: str(tmpdir / config_subdir),
+def test_fake_windows_data_config(tmpdir, monkeypatch, what):
-    }
+    """Make sure the config is correct on a fake Windows."""
-    monkeypatch.setattr(standarddir.os, 'name', 'nt')
+    monkeypatch.setattr(os, 'name', 'nt')
     monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation',
-                        locations.get)
+                        lambda typ: str(tmpdir))
+    standarddir._init_config(args=None)
     standarddir._init_data(args=None)
-    expected = str(tmpdir / expected)
+    func = getattr(standarddir, what)
-    assert standarddir.data() == expected
+    assert func() == str(tmpdir / what)
 
 
 class TestWritableLocation:
@@ -140,8 +142,8 @@ class TestStandardDir:
 
     @pytest.mark.parametrize('func, elems, expected', [
         (standarddir.data, 2, ['qute_test', 'data']),
-        (standarddir.config, 1, ['qute_test']),
+        (standarddir.config, 2, ['qute_test', 'config']),
-        (lambda: standarddir.config(auto=True), 1, ['qute_test']),
+        (lambda: standarddir.config(auto=True), 2, ['qute_test', 'config']),
         (standarddir.cache, 2, ['qute_test', 'cache']),
         (standarddir.download, 1, ['Downloads']),
     ])
@@ -153,8 +155,8 @@ class TestStandardDir:
     @pytest.mark.parametrize('func, elems, expected', [
         (standarddir.data, 2, ['Application Support', 'qute_test']),
         (standarddir.config, 1, ['qute_test']),
-        # FIXME:conf Actually support auto=True
+        (lambda: standarddir.config(auto=True), 2,
-        (lambda: standarddir.config(auto=True), 1, ['qute_test']),
+         [os.path.expanduser('~'), '.qutebrowser']),
         (standarddir.cache, 2, ['Caches', 'qute_test']),
         (standarddir.download, 1, ['Downloads']),
     ])
@@ -308,33 +310,47 @@ class TestSystemData:
         assert standarddir.data(system=True) == standarddir.data()
 
 
-class TestMoveWebEngineData:
+class TestDataMigrations:
 
-    """Test moving QtWebEngine data from an old location."""
+    """Test moving various data from an old to a new location."""
 
     @pytest.fixture(autouse=True)
-    def patch_standardpaths(self, tmpdir, monkeypatch):
+    def patch_standardpaths(self, files, tmpdir, monkeypatch):
         locations = {
-            QStandardPaths.DataLocation: str(tmpdir / 'data'),
+            QStandardPaths.DataLocation: str(files.local_data_dir),
             QStandardPaths.CacheLocation: str(tmpdir / 'cache'),
+            QStandardPaths.AppDataLocation: str(files.roaming_data_dir),
         }
         monkeypatch.setattr(standarddir.QStandardPaths, 'writableLocation',
                             locations.get)
+
         monkeypatch.setattr(standarddir, 'data',
                             lambda: str(tmpdir / 'new_data'))
         monkeypatch.setattr(standarddir, 'cache',
                             lambda: str(tmpdir / 'new_cache'))
+        monkeypatch.setattr(
+            standarddir, 'config', lambda auto=False:
+            str(files.auto_config_dir if auto else files.config_dir))
 
     @pytest.fixture
     def files(self, tmpdir):
-        files = collections.namedtuple('Files', ['old_data', 'new_data',
+        files = collections.namedtuple('Files', [
-                                                 'old_cache', 'new_cache'])
+            'old_webengine_data', 'new_webengine_data',
+            'old_webengine_cache', 'new_webengine_cache',
+            'auto_config_dir', 'config_dir',
+            'local_data_dir', 'roaming_data_dir'])
         return files(
-            old_data=tmpdir / 'data' / 'QtWebEngine' / 'Default' / 'datafile',
+            old_webengine_data=(tmpdir / 'data' / 'QtWebEngine' / 'Default' /
-            new_data=tmpdir / 'new_data' / 'webengine' / 'datafile',
+                                'datafile'),
-            old_cache=(tmpdir / 'cache' / 'QtWebEngine' / 'Default' /
+            new_webengine_data=tmpdir / 'new_data' / 'webengine' / 'datafile',
-                       'cachefile'),
+            old_webengine_cache=(tmpdir / 'cache' / 'QtWebEngine' / 'Default' /
-            new_cache=(tmpdir / 'new_cache' / 'webengine' / 'cachefile'),
+                                 'cachefile'),
+            new_webengine_cache=(tmpdir / 'new_cache' / 'webengine' /
+                                 'cachefile'),
+            auto_config_dir=tmpdir / 'auto_config',
+            config_dir=tmpdir / 'config',
+            local_data_dir=tmpdir / 'data',
+            roaming_data_dir=tmpdir / 'roaming-data',
         )
 
     def test_no_webengine_dir(self, caplog):
@@ -344,32 +360,37 @@ class TestMoveWebEngineData:
                        for rec in caplog.records)
 
     def test_moving_data(self, files):
-        files.old_data.ensure()
+        files.old_webengine_data.ensure()
-        files.old_cache.ensure()
+        files.old_webengine_cache.ensure()
 
         standarddir._move_webengine_data()
 
-        assert not files.old_data.exists()
+        assert not files.old_webengine_data.exists()
-        assert not files.old_cache.exists()
+        assert not files.old_webengine_cache.exists()
-        assert files.new_data.exists()
+        assert files.new_webengine_data.exists()
-        assert files.new_cache.exists()
+        assert files.new_webengine_cache.exists()
 
     @pytest.mark.parametrize('what', ['data', 'cache'])
     def test_already_existing(self, files, caplog, what):
-        files.old_data.ensure()
+        files.old_webengine_data.ensure()
-        files.old_cache.ensure()
+        files.old_webengine_cache.ensure()
 
         if what == 'data':
-            files.new_data.ensure()
+            files.new_webengine_data.ensure()
+            old_path = str(files.old_webengine_data.dirname)
+            new_path = str(files.new_webengine_data.dirname)
         else:
-            files.new_cache.ensure()
+            files.new_webengine_cache.ensure()
+            old_path = str(files.old_webengine_cache.dirname)
+            new_path = str(files.new_webengine_cache.dirname)
 
-        with caplog.at_level(logging.WARNING):
+        with caplog.at_level(logging.ERROR):
             standarddir._move_webengine_data()
 
         record = caplog.records[-1]
-        expected = "Failed to move old QtWebEngine {}".format(what)
+        expected = "Failed to move data from {} as {} already exists!".format(
-        assert record.message.startswith(expected)
+            old_path, new_path)
+        assert record.message == expected
 
     def test_deleting_empty_dirs(self, monkeypatch, tmpdir):
         """When we have a qutebrowser/qutebrowser subfolder, clean it up."""
@@ -396,16 +417,41 @@ class TestMoveWebEngineData:
         """When there was an error it should be logged."""
         mock = mocker.Mock(side_effect=OSError('error'))
         monkeypatch.setattr(standarddir.shutil, 'move', mock)
-        files.old_data.ensure()
+        files.old_webengine_data.ensure()
-        files.old_cache.ensure()
+        files.old_webengine_cache.ensure()
 
         with caplog.at_level(logging.ERROR):
             standarddir._move_webengine_data()
 
         record = caplog.records[-1]
-        expected = "Failed to move old QtWebEngine data/cache: error"
+        expected = "Failed to move data from {} to {}: error".format(
+            files.old_webengine_data.dirname, files.new_webengine_data.dirname)
         assert record.message == expected
 
+    def test_move_macos(self, files):
+        """Test moving configs on macOS."""
+        (files.auto_config_dir / 'autoconfig.yml').ensure()
+        (files.auto_config_dir / 'quickmarks').ensure()
+        files.config_dir.ensure(dir=True)
+
+        standarddir._move_macos()
+
+        assert (files.auto_config_dir / 'autoconfig.yml').exists()
+        assert not (files.config_dir / 'autoconfig.yml').exists()
+        assert not (files.auto_config_dir / 'quickmarks').exists()
+        assert (files.config_dir / 'quickmarks').exists()
+
+    def test_move_windows(self, files):
+        """Test moving configs on Windows."""
+        (files.local_data_dir / 'data' / 'blocked-hosts').ensure()
+        (files.local_data_dir / 'qutebrowser.conf').ensure()
+
+        standarddir._move_windows()
+
+        assert (files.roaming_data_dir / 'data' / 'blocked-hosts').exists()
+        assert (files.roaming_data_dir / 'config' /
+                'qutebrowser.conf').exists()
+
 
 @pytest.mark.parametrize('with_args', [True, False])
 def test_init(mocker, tmpdir, with_args):