Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planned 0.0.7 breaks platforms and distribution packaging. #276

Closed
AIIX opened this issue Feb 10, 2023 · 6 comments · Fixed by #278
Closed

planned 0.0.7 breaks platforms and distribution packaging. #276

AIIX opened this issue Feb 10, 2023 · 6 comments · Fixed by #278

Comments

@AIIX
Copy link

AIIX commented Feb 10, 2023

Issue: Requirements.txt now forces installation of skill-ovos-setup as a plugin in requirements.txt. Platforms already shipping their own setup skill that are git installed causes the skill process to crash due to this.

2023-02-10 14:03:04.934 - skills - mycroft.skills.skill_manager:_load_new_skills:503 - INFO - skill-ovos-setup.openvoiceos plugin will be replaced by a local version: /home/aix/.local/share/mycroft/skills/skill-ovos-setup.openvoiceos
2023-02-10 14:03:04.934 - skills - mycroft.skills.skill_manager:_unload_plugin_skill:591 - INFO - Unloading plugin skill: skill-ovos-setup.openvoiceos
2023-02-10 14:03:04.934 - skills - mycroft.skills.skill_manager:_unload_plugin_skill:591 - INFO - Unloading plugin skill: skill-ovos-setup.openvoiceos
2023-02-10 14:03:04.934 - skills - mycroft.skills.skill_manager:_unload_plugin_skill:591 - INFO - Unloading plugin skill: skill-ovos-setup.openvoiceos
2023-02-10 14:03:04.934 - skills - mycroft.skills.skill_manager:_unload_plugin_skill:591 - INFO - Unloading plugin skill: skill-ovos-setup.openvoiceos
2023-02-10 14:03:04.936 - skills - mycroft.skills.skill_loader:load:399 - INFO - ATTEMPTING TO LOAD SKILL: skill-ovos-setup.openvoiceos
2023-02-10 14:03:04.937 - skills - mycroft.skills.skill_manager:run:453 - ERROR - Something really unexpected has occured and the skill manager loop safety harness was hit.
Traceback (most recent call last):
  File "/home/aix/ovos/ovos-core/mycroft/skills/skill_manager.py", line 449, in run
    self._load_new_skills()
  File "/home/aix/ovos/ovos-core/mycroft/skills/skill_manager.py", line 504, in _load_new_skills
    self._unload_plugin_skill(skill_id)
  File "/home/aix/ovos/ovos-core/mycroft/skills/skill_manager.py", line 598, in _unload_plugin_skill
    self.plugin_skills.pop(skill_id)
KeyError: 'skill-ovos-setup.openvoiceos'
2023-02-10 14:03:04.938 - skills - mycroft.skills.skill_loader:_load_skill_source:506 - ERROR - Failed to load skill-ovos-setup.openvoiceos due to a missing file.
  • Platforms shipping their own setup skill and platforms that don't require a setup skill are now forced to deal with this new plugin install, or need to hack packaging to fix requirements.txt

  • Skills should not be forced with basic requirements.txt, let distributions package their own skills and requirements for the specific platform

  • If platforms/distributions choose they can install the essential skills. Don't make setup skill a hard requirement for every platform out there.

@AIIX AIIX mentioned this issue Feb 10, 2023
93 tasks
@ChanceNCounter
Copy link

This is present in 0.0.6 #266 so it's in the wild

@AIIX
Copy link
Author

AIIX commented Feb 10, 2023

This is present in 0.0.6 #266 so it's in the wild

skills-essential is not an egg manjaro packages, because its got its own set of skills, the skills were not part of the requirements.txt that is the very basic to get any platform running before 0.0.7, we only install requirements.txt

@ChanceNCounter
Copy link

I think this might speak to a broader need to clarify all those dep models, because, now that I've looked at the cleanup pass that introduced the offending dep to requirements.txt, it looks like Jarbas was trying to make them a little easier to read and his clipboard betrayed him. The whole set came over! I'm guessing something else was meant to go in that spot.

@AIIX
Copy link
Author

AIIX commented Feb 10, 2023

Whatever it is, I would suggest and hope not doing an other Mycroft forcing their skills on installations, then causing platforms and distributions and anyone wanting to make any product out of ovos being forced to add ugly workarounds and hacks at various levels to deal with this issue.

@AIIX
Copy link
Author

AIIX commented Feb 10, 2023

Marking #277 this as a potential fix for this issue, where platforms packaging OVOS can choose to only install the platform-requirements.txt and not include base skills

@JarbasAl
Copy link
Member

packaging is roadmapped for 0.0.8, the metapackages automations will make this clearer, this immediate issue is fixed by #278

0.0.7 only has roadmapped to finish the defaults skills list introduced in 0.0.6, we probably should split this into several recommended lists, all these optional and not part of all except for essential

  • essential_skills -> needed for core to work, "unknown", "stop" etc
  • recommended_skills -> what we ship in our images, so we all use the same list
  • media_skills -> recommended list of base skills for media center, some music radio etc skills

PRs welcome to get the above going, many skills still need automations and publishing in pypi

some info for other people, adapted from chat discussion:


an explanation of requirements is warranted:

  • minimal -> this is shared between all services and always needed, its the ovos-core lib
  • bus -> extra deps for running bus service
  • skills -> extra deps for running skill service
  • X -> extra deps for running X service
  • skills_essential -> list of essential ovos skills
  • requirements.txt -> all in setup.py, when you want the full voice assistant

not having skills in all was raised a few times in chat by new users, so they were added there, also this is what manual git installs will usually pull

a distro should never use requirements.txt, distros should use minimal + the relevant individual services, we can even add new optional requirements with default skills for distros we want to officially support (eg, bigscreen). the WIP metapackages will make this easier on the distros side when packaging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants