mycroft-skills icon indicating copy to clipboard operation
mycroft-skills copied to clipboard

If installing requirements from `requirements.sh` fails, `msm` should halt execution, not continue to install `requirements.txt`

Open KathyReid opened this issue 6 years ago • 4 comments

How was this Issue identified?

This Issue was raised after the Skills Team Meeting on 28th June based on this PR where requirements.sh failed due to conditional distro-based deps, but execution continued and msm attempted to install requrements.txt.

Requested change

If installation of dependencies listed in requirements.sh fails for whatever reason, execution is halted, and msm does not go on the attempt to install pip dependencies listed in requirements.txt.

Broader landscape

The broader issue here is that requirements.sh is open to abuse - how do we move to better package management? We want to do better package management for Skills - such as using manifest.json type packaging, but this is a longer term proposition.

KathyReid avatar Jul 05 '18 18:07 KathyReid

Msm currently will halt if the script requirements.sh returns an error exit code. I'm guessing that the problem here is that the script had another command after the failing command and set -e wasn't enabled so bash continued through the script and from msm's side, the script looked like it completed successfully. In general, the more we move things out of requirements.sh, the less we have to worry about. How about a yaml file with something like this for now: manifest.yml:

dependencies:
    system:
        apt-get: install -y pianobar
        dnf: install -y pianobar
        pacman: -S --needed --noconfirm pianobar
    skill:  # Skill dependencies
        - face-detector
        - mycroft-calendar
    python:
        - requests
        - pydora

This would deprecate requirements.txt and skill_requirements.txt and be favored over requirements.sh just used for apt-get installs. Later on we could work out the details for other fields like name and description.

MatthewScholefield avatar Jul 05 '18 20:07 MatthewScholefield

@KathyReid I could take a look at this. I am not able to find the file requirements.sh

vipulgupta2048 avatar Oct 10 '18 11:10 vipulgupta2048

For system: could we use a generic version, kinda like pacapt and only have to define the distro-specific piece if needed? Pianobar is a good example -- if the author doesn't know about zypper it should still be able to install if the package exists.

dependencies:
    system:
        all: pianobar
        zypper: -S --needed --noconfirm pianobarXY
    skill:  # Skill dependencies
        - face-detector
        - mycroft-calendar
    python:
        - requests
        - pydora

Where the package portion would use the "pianobar" from all: on the majority of systems but would invoke zypper on systems that have it using whatever flags and/or package(s) are listed.

penrods avatar Jan 25 '19 22:01 penrods

See if this is of any use: https://github.com/icy/pacapt

penrods avatar Jan 28 '19 17:01 penrods