From c5c44d22d1a1b9c682fb014b5a6c6d3ef50464a5 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 | 12 +++++++++--- test/unittests/skills/test_skill_updater.py | 11 ++++++----- 13 files changed, 46 insertions(+), 37 deletions(-) diff --git a/mycroft/configuration/mycroft.conf b/mycroft/configuration/mycroft.conf index b6922259aa43..2a7cefe1a2e4 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 7878e9c8e8f9..91ed47cbd572 100644 --- a/mycroft/skills/msm_wrapper.py +++ b/mycroft/skills/msm_wrapper.py @@ -22,6 +22,7 @@ from collections import namedtuple from functools import lru_cache from os import path, makedirs +import xdg.BaseDirectory from msm import MycroftSkillsManager, SkillRepo @@ -34,9 +35,8 @@ [ '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) + xdg.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 67bfac529f8a..afac295591e4 100644 --- a/mycroft/skills/mycroft_skill/mycroft_skill.py +++ b/mycroft/skills/mycroft_skill/mycroft_skill.py @@ -124,7 +124,7 @@ def __init__(self, name=None, bus=None, use_settings=True): # 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 051d5896ae5c..26ec2a52fd73 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 9beebe9b6cd6..3256de4abf69 100644 --- a/mycroft/skills/skill_manager.py +++ b/mycroft/skills/skill_manager.py @@ -18,6 +18,8 @@ from threading import Thread, Event, Lock from time import sleep, time, monotonic from inspect import signature +import shutil +import xdg.BaseDirectory from mycroft.api import is_paired from mycroft.enclosure.api import EnclosureAPI @@ -264,7 +266,9 @@ def run(self): 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( + xdg.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 @@ def _load_skill(self, skill_directory): 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( + xdg.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 c4eafdfcef8c..dd2530d31621 100644 --- a/mycroft/skills/skill_updater.py +++ b/mycroft/skills/skill_updater.py @@ -55,7 +55,8 @@ def __init__(self, bus=None): 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( + xdg.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 6634fabcb06c..92780db4d40a 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 0c7c8197a2dd..6cc8ea927a31 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -16,7 +16,7 @@ fasteners==0.14.1 PyYAML==5.4 lingua-franca==0.4.2 -msm==0.8.9 +msm==0.9.0 msk==0.3.16 mycroft-messagebus-client==0.9.1 adapt-parser==0.5.1 diff --git a/requirements/tests.txt b/requirements/tests.txt index 9e038123e51a..3d2e2b9a4c95 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 fed939417699..838626f96ff7 100644 --- a/test/unittests/base.py +++ b/test/unittests/base.py @@ -18,6 +18,8 @@ from unittest import TestCase from unittest.mock import patch +import xdg.BaseDirectory + from .mocks import mock_msm, mock_config, MessageBusMock @@ -54,3 +56,4 @@ def _mock_log(self): def tearDown(self): rmtree(str(self.temp_dir)) + rmtree(xdg.BaseDirectory.save_data_path('mycroft')) diff --git a/test/unittests/skills/test_skill_manager.py b/test/unittests/skills/test_skill_manager.py index fc5eee68a3d1..ca8519b3d6f2 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 +import xdg.BaseDirectory from unittest import TestCase from unittest.mock import Mock, patch @@ -95,7 +97,10 @@ def _mock_skill_updater(self): 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(xdg.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 +133,9 @@ def test_instantiate(self): ) def test_remove_git_locks(self): - git_dir = self.temp_dir.joinpath('foo/.git') - git_dir.mkdir(parents=True) + git_dir = Path( + xdg.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') diff --git a/test/unittests/skills/test_skill_updater.py b/test/unittests/skills/test_skill_updater.py index d7cc72e3e675..a28bfb37393d 100644 --- a/test/unittests/skills/test_skill_updater.py +++ b/test/unittests/skills/test_skill_updater.py @@ -247,10 +247,11 @@ def test_schedule_retry(self): def test_update_download_time(self): """Test updating the next time a download will occur.""" - dot_msm_path = self.temp_dir.joinpath('.msm') - dot_msm_path.touch() - dot_msm_mtime_before = dot_msm_path.stat().st_mtime + skill_updater = SkillUpdater(self.message_bus_mock) + skill_updater.dot_msm_path = self.temp_dir.joinpath('.msm') + skill_updater.dot_msm_path.touch() + dot_msm_mtime_before = skill_updater.dot_msm_path.stat().st_mtime sleep(0.5) - SkillUpdater(self.message_bus_mock)._update_download_time() - dot_msm_mtime_after = dot_msm_path.stat().st_mtime + skill_updater._update_download_time() + dot_msm_mtime_after = skill_updater.dot_msm_path.stat().st_mtime self.assertLess(dot_msm_mtime_before, dot_msm_mtime_after)