WIP Review comments

- Add TODO for 22.02 to remove the compatibility code
- Make Warning a single Log statement
- mycroft-config script now uses XDG-environment variable
- Remove redundant code
- Replace hard coded references to ~/.config
- Explicitly remove new path before move of "filesystem" (if needed)

bin/mycroft-config

@@ -95,11 +95,11 @@ function validate_config_file() {
     return $result
 }
 
-_conf_file="~/.config/mycroft/mycroft.conf"
+_conf_file="${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf"
 function name_to_path() {
     case ${1} in
         "system")     _conf_file="/etc/mycroft/mycroft.conf" ;;
-        "user")       _conf_file=$(readlink -f ~/.config/mycroft/mycroft.conf) ;;
+        "user")       _conf_file=$(readlink -f ${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf) ;;
         "default")    _conf_file="$DIR/../mycroft/configuration/mycroft.conf" ;;
         "remote")     _conf_file="$HOME/.cache/mycroft/web_cache.json" ;;
 

mycroft/client/enclosure/mark1/__init__.py

@@ -19,6 +19,8 @@ from threading import Thread, Timer
 
 import serial
 
+import xdg.BaseDirectory
+
 import mycroft.dialog
 from mycroft.client.enclosure.base import Enclosure
 from mycroft.api import has_been_paired
@@ -164,7 +166,8 @@ class EnclosureReader(Thread):
             self.bus.emit(Message("speak", {
                 'utterance': mycroft.dialog.get("reset to factory defaults")}))
             subprocess.call(
-                'rm ~/.config/mycroft/identity/identity2.json',
+                (f'rm {xdg.BaseDirectory.save_config_path("mycroft")}'
+                 '/mycroft/identity/identity2.json'),
                 shell=True)
             subprocess.call(
                 'rm ~/.mycroft/identity/identity2.json',

mycroft/client/speech/hotword_factory.py

@@ -280,7 +280,7 @@ class PreciseHotword(HotWordEngine):
         old_path = join(expanduser('~'), '.mycroft', 'precise')
         if os.path.isdir(old_path):
             return old_path
-        return join(xdg.BaseDirectory.save_data_path('mycroft', 'precise'))
+        return xdg.BaseDirectory.save_data_path('mycroft', 'precise')
 
     @property
     def install_destination(self):

mycroft/configuration/config.py

@@ -24,7 +24,7 @@ import xdg.BaseDirectory
 from mycroft.util.json_helper import load_commented_json, merge_dict
 from mycroft.util.log import LOG
 
-from .locations import DEFAULT_CONFIG, OLD_USER_CONFIG, USER_CONFIG
+from .locations import DEFAULT_CONFIG, USER_CONFIG, OLD_USER_CONFIG
 from .locations import SYSTEM_CONFIG
 
 
@@ -170,6 +170,17 @@ class RemoteConf(LocalConf):
             self.load_local(cache)
 
 
+def _log_old_location_deprecation():
+    LOG.warning("\n ===============================================\n"
+                " ==             DEPRECATION WARNING           ==\n"
+                " ===============================================\n"
+                f" You still have a config file at {OLD_USER_CONFIG}\n"
+                " Note that this location is deprecated and will"
+                " not be used in the future\n"
+                " Please move it to "
+                f"{xdg.BaseDirectory.save_config_path('mycroft')}")
+
+
 class Configuration:
     """Namespace for operations on the configuration singleton."""
     __config = {}  # Cached config
@@ -185,9 +196,7 @@ class Configuration:
         Args:
             configs (list): List of configuration dicts
             cache (boolean): True if the result should be cached
-            remote (boolean): False if the Mycroft Home settings shouldn't
-                              be loaded
-
+            remote (boolean): False if the Remote settings shouldn't be loaded
 
         Returns:
             (dict) configuration dictionary.
@@ -210,7 +219,7 @@ class Configuration:
             (dict) merged dict of all configuration files
         """
         if not configs:
-            configs = configs or []
+            configs = []
 
             # First use the patched config
             configs.append(Configuration.__patch)
@@ -223,21 +232,9 @@ class Configuration:
 
             # Then check the old user config
             if isfile(OLD_USER_CONFIG):
-                LOG.warning(" ===============================================")
-                LOG.warning(" ==             DEPRECATION WARNING           ==")
-                LOG.warning(" ===============================================")
-                LOG.warning(" You still have a config file at " +
-                            OLD_USER_CONFIG)
-                LOG.warning(" Note that this location is deprecated and will" +
-                            " not be used in the future")
-                LOG.warning(" Please move it to " +
-                            xdg.BaseDirectory.save_config_path('mycroft'))
+                _log_old_location_deprecation()
                 configs.append(LocalConf(OLD_USER_CONFIG))
 
-            # Then check the XDG user config
-            if isfile(USER_CONFIG):
-                configs.append(LocalConf(USER_CONFIG))
-
             # Then use remote config
             if remote:
                 configs.append(RemoteConf())

mycroft/configuration/locations.py

@@ -18,11 +18,13 @@ import xdg.BaseDirectory
 DEFAULT_CONFIG = join(dirname(__file__), 'mycroft.conf')
 SYSTEM_CONFIG = os.environ.get('MYCROFT_SYSTEM_CONFIG',
                                '/etc/mycroft/mycroft.conf')
+# TODO: remove in 22.02
 # Make sure we support the old location still
 # Deprecated and will be removed eventually
 OLD_USER_CONFIG = join(expanduser('~'), '.mycroft/mycroft.conf')
 USER_CONFIG = join(xdg.BaseDirectory.save_config_path('mycroft'),
                    'mycroft.conf')
+
 REMOTE_CONFIG = "mycroft.ai"
 WEB_CONFIG_CACHE = os.environ.get('MYCROFT_WEB_CACHE',
                                   '/var/tmp/mycroft_web_cache.json')

mycroft/filesystem/__init__.py

@@ -38,7 +38,10 @@ class FileSystemAccess:
         path = join(xdg.BaseDirectory.save_config_path('mycroft'), path)
 
         # Migrate from the old location if it still exists
+        # TODO: remove in 22.02
         if isdir(old_path):
+            if isdir(path):
+                shutil.rmtree(path)
             shutil.move(old_path, path)
 
         if not isdir(path):