From 35a470f85b3c1f86bcdfbf54d1f54daeed1d1978 Mon Sep 17 00:00:00 2001 From: Bart Ribbers Date: Fri, 15 Jan 2021 16:29:09 +0100 Subject: [PATCH] Read skills from XDG home directory Also move over skills from data_dir if they still exist --- mycroft/configuration/mycroft.conf | 4 +--- mycroft/skills/msm_wrapper.py | 13 +++++-------- mycroft/skills/mycroft_skill/mycroft_skill.py | 2 +- mycroft/skills/settings.py | 2 +- mycroft/skills/skill_manager.py | 9 +++++++-- mycroft/skills/skill_updater.py | 3 ++- pytest.ini | 2 ++ requirements/requirements.txt | 2 +- requirements/tests.txt | 1 + test/unittests/base.py | 3 +++ test/unittests/skills/test_skill_manager.py | 8 +++++--- 11 files changed, 29 insertions(+), 20 deletions(-) diff --git a/mycroft/configuration/mycroft.conf b/mycroft/configuration/mycroft.conf index 17bb75e4112..69e9f9c2f15 100644 --- a/mycroft/configuration/mycroft.conf +++ b/mycroft/configuration/mycroft.conf @@ -106,13 +106,11 @@ } }, "upload_skill_manifest": true, - // Directory to look for user skills - "directory": "~/.local/share/mycroft/skills", // Enable auto update by msm "auto_update": true, // blacklisted skills to not load // NB: This is the basename() of the directory where the skill lives, so if - // the skill you want to blacklist is in /opt/mycroft/skills/mycroft-alarm.mycroftai/ + // the skill you want to blacklist is in $XDG_DATA_DIR/mycroft/skills/mycroft-alarm.mycroftai/ // then you should write `["mycroft-alarm.mycroftai"]` below. "blacklisted_skills": [], // priority skills to be loaded first diff --git a/mycroft/skills/msm_wrapper.py b/mycroft/skills/msm_wrapper.py index 7878e9c8e8f..4f41970afe0 100644 --- a/mycroft/skills/msm_wrapper.py +++ b/mycroft/skills/msm_wrapper.py @@ -22,6 +22,7 @@ frequently. To improve performance, the MSM instance is cached. from collections import namedtuple from functools import lru_cache from os import path, makedirs +from xdg import BaseDirectory from msm import MycroftSkillsManager, SkillRepo @@ -34,9 +35,8 @@ MsmConfig = namedtuple( [ 'platform', 'repo_branch', - 'repo_cache', 'repo_url', - 'skills_dir', + 'old_skills_dir', 'versioned' ] ) @@ -71,9 +71,8 @@ def build_msm_config(device_config: dict) -> MsmConfig: return MsmConfig( platform=enclosure_config.get('platform', 'default'), repo_branch=msm_repo_config['branch'], - repo_cache=path.join(data_dir, msm_repo_config['cache']), repo_url=msm_repo_config['url'], - skills_dir=path.join(data_dir, msm_config['directory']), + old_skills_dir=path.join(data_dir, msm_config['directory']), versioned=msm_config['versioned'] ) @@ -95,17 +94,15 @@ def create_msm(msm_config: MsmConfig) -> MycroftSkillsManager: msm_lock = _init_msm_lock() LOG.info('Acquiring lock to instantiate MSM') with msm_lock: - if not path.exists(msm_config.skills_dir): - makedirs(msm_config.skills_dir) + BaseDirectory.save_data_path('mycroft/skills') msm_skill_repo = SkillRepo( - msm_config.repo_cache, msm_config.repo_url, msm_config.repo_branch ) msm_instance = MycroftSkillsManager( platform=msm_config.platform, - skills_dir=msm_config.skills_dir, + old_skills_dir=msm_config.old_skills_dir, repo=msm_skill_repo, versioned=msm_config.versioned ) diff --git a/mycroft/skills/mycroft_skill/mycroft_skill.py b/mycroft/skills/mycroft_skill/mycroft_skill.py index 31db7d5381a..402a9b4f6c7 100644 --- a/mycroft/skills/mycroft_skill/mycroft_skill.py +++ b/mycroft/skills/mycroft_skill/mycroft_skill.py @@ -124,7 +124,7 @@ class MycroftSkill: # Get directory of skill #: Member variable containing the absolute path of the skill's root - #: directory. E.g. /opt/mycroft/skills/my-skill.me/ + #: directory. E.g. $XDG_DATA_HOME/mycroft/skills/my-skill.me/ self.root_dir = dirname(abspath(sys.modules[self.__module__].__file__)) self.gui = SkillGUI(self) diff --git a/mycroft/skills/settings.py b/mycroft/skills/settings.py index c48416afac0..6e70783df0c 100644 --- a/mycroft/skills/settings.py +++ b/mycroft/skills/settings.py @@ -99,7 +99,7 @@ def save_settings(skill_dir, skill_settings): """Save skill settings to file.""" settings_path = Path(skill_dir).joinpath('settings.json') - # Either the file already exists in /opt, or we are writing + # Either the file already exists or we are writing # to XDG_CONFIG_DIR and always have the permission to make # sure the file always exists if not Path(settings_path).exists(): diff --git a/mycroft/skills/skill_manager.py b/mycroft/skills/skill_manager.py index acfbc5c92dd..44ff856c965 100644 --- a/mycroft/skills/skill_manager.py +++ b/mycroft/skills/skill_manager.py @@ -18,6 +18,8 @@ from glob import glob from threading import Thread, Event, Lock from time import sleep, time, monotonic from inspect import signature +import shutil +from xdg import BaseDirectory from mycroft.api import is_paired from mycroft.enclosure.api import EnclosureAPI @@ -264,7 +266,9 @@ class SkillManager(Thread): def _remove_git_locks(self): """If git gets killed from an abrupt shutdown it leaves lock files.""" - for i in glob(os.path.join(self.msm.skills_dir, '*/.git/index.lock')): + for i in glob(os.path.join( + BaseDirectory.save_data_path('mycroft/skills'), + '*/.git/index.lock')): LOG.warning('Found and removed git lock file: ' + i) os.remove(i) @@ -310,7 +314,8 @@ class SkillManager(Thread): return skill_loader if load_status else None def _get_skill_directories(self): - skill_glob = glob(os.path.join(self.msm.skills_dir, '*/')) + skill_glob = glob(os.path.join( + BaseDirectory.save_data_path('mycroft/skills'), '*/')) skill_directories = [] for skill_dir in skill_glob: diff --git a/mycroft/skills/skill_updater.py b/mycroft/skills/skill_updater.py index 013411ce50e..d5f8120338b 100644 --- a/mycroft/skills/skill_updater.py +++ b/mycroft/skills/skill_updater.py @@ -55,7 +55,8 @@ class SkillUpdater: self.config = Configuration.get() update_interval = self.config['skills']['update_interval'] self.update_interval = int(update_interval) * ONE_HOUR - self.dot_msm_path = os.path.join(self.msm.skills_dir, '.msm') + self.dot_msm_path = os.path.join( + BaseDirectory.save_data_path('mycroft/skills'), '.msm') self.next_download = self._determine_next_download_time() self._log_next_download_time() self.installed_skills = set() diff --git a/pytest.ini b/pytest.ini index 6634fabcb06..92780db4d40 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,3 +1,5 @@ [pytest] testpaths = test norecursedirs = wake_word +env = + XDG_DATA_HOME=/tmp/mycroft-test diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 5234739cf9c..179495f1fde 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -16,7 +16,7 @@ fasteners==0.14.1 PyYAML==5.4 lingua-franca==0.4.1 -msm==0.8.9 +msm==0.9.0 msk==0.3.16 mycroft-messagebus-client==0.9.2 adapt-parser==0.3.7 diff --git a/requirements/tests.txt b/requirements/tests.txt index 9e038123e51..3d2e2b9a4c9 100644 --- a/requirements/tests.txt +++ b/requirements/tests.txt @@ -2,6 +2,7 @@ coveralls==1.8.2 flake8==3.7.9 pytest==5.2.4 pytest-cov==2.8.1 +pytest-env==0.6.2 cov-core==1.15.0 sphinx==2.2.1 sphinx-rtd-theme==0.4.3 diff --git a/test/unittests/base.py b/test/unittests/base.py index fed93941769..ee86778fa10 100644 --- a/test/unittests/base.py +++ b/test/unittests/base.py @@ -18,6 +18,8 @@ from shutil import rmtree from unittest import TestCase from unittest.mock import patch +from xdg import BaseDirectory + from .mocks import mock_msm, mock_config, MessageBusMock @@ -54,3 +56,4 @@ class MycroftUnitTestBase(TestCase): def tearDown(self): rmtree(str(self.temp_dir)) + rmtree(BaseDirectory.save_data_path('mycroft')) diff --git a/test/unittests/skills/test_skill_manager.py b/test/unittests/skills/test_skill_manager.py index fc5eee68a3d..f82ff55418d 100644 --- a/test/unittests/skills/test_skill_manager.py +++ b/test/unittests/skills/test_skill_manager.py @@ -13,6 +13,8 @@ # limitations under the License. # from os import path +from pathlib import Path +from xdg import BaseDirectory from unittest import TestCase from unittest.mock import Mock, patch @@ -95,7 +97,8 @@ class TestSkillManager(MycroftUnitTestBase): self.skill_updater_mock = skill_updater_patch.start() def _mock_skill_loader_instance(self): - self.skill_dir = self.temp_dir.joinpath('test_skill') + self.skill_dir = (Path(BaseDirectory.save_data_path('mycroft/skills')) + .joinpath('test_skill')) self.skill_loader_mock = Mock(spec=SkillLoader) self.skill_loader_mock.instance = Mock() self.skill_loader_mock.instance.default_shutdown = Mock() @@ -128,8 +131,7 @@ class TestSkillManager(MycroftUnitTestBase): ) def test_remove_git_locks(self): - git_dir = self.temp_dir.joinpath('foo/.git') - git_dir.mkdir(parents=True) + git_dir = Path(BaseDirectory.save_data_path('mycroft/skills/foo/.git')) git_lock_file_path = str(git_dir.joinpath('index.lock')) with open(git_lock_file_path, 'w') as git_lock_file: git_lock_file.write('foo') -- 2.32.0