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

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

Open
KathyReid opened this issue Jul 5, 2018 · 4 comments

Comments

@KathyReid
Copy link
Contributor

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.

@MatthewScholefield
Copy link
Contributor

MatthewScholefield commented Jul 5, 2018

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.

@vipulgupta2048
Copy link
Contributor

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

@penrods
Copy link
Contributor

penrods commented Jan 25, 2019

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
Copy link
Contributor

penrods commented Jan 28, 2019

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

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

No branches or pull requests

4 participants