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

feat: Optimize bash scripts for install #377

Draft
wants to merge 4 commits into
base: naomi-dev
Choose a base branch
from

Conversation

rileyhawk1417
Copy link
Collaborator

Optimize the bash scripts for the installer to avoid repeating code in multiple places.

Description

Optimizing the bash installer scripts to reduce code footprint, as some of the functions
within the files can be re-used in several places. This PR has migrated some of the commonly
used functions and values into specific files. That can be called when needed, this will allow
flexibility when porting to other linux based distros.

Related Issue

Motivation and Context

  • Can allow contributers & maintainers to support multiple distros.
  • Breaking large chunks of code into smaller ones for readability.

How Has This Been Tested?

The code has been tested on docker containers with the following environments:

  • Ubuntu 20.04
  • Debian 11

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@AustinCasteel AustinCasteel self-requested a review March 27, 2023 15:20
@AustinCasteel
Copy link
Member

Here are my findings so far but cant go further until the phonetisaurus issue is resolved.

  1. With these changes the oneliner will no longer work, need to provide catches so if the source is not available then it will download all the extra files these changes introduce needed for the install process to run.
  2. Can not run the Naomi-setup script directly from the installers dir because the extra files pathing is not relative to the script so they can not be found.
  3. 

Install on arch has been added yet there are is no arch process that it is referring to. Either the process needs to be added or the arch install start needs to be removed(or commented out) so that between this PR and whenever arch is actually included there will not be someone that is on arch that tries to install Naomi but it errors and is impossible to install instead of returning the unknown_os message.
  4. WizardSetup is never called but has the initial console outputs and prompts for the first question.
  5. The naomiChannel choices need to break in order to work. Not sure why it was removed.
  6. SUDO_COMMAND approval is not being passed between all the various scripts so you still have to say yes to some things.
  7. Errors when installing python requirements, Could not find a version that satisfies the requirement phonetisaurus (from versions: none) No matching distribution found for phonetisaurus

@aaronchantrill
Copy link
Contributor

The phonetisaurus thing is new. I was able to install with a simple pip install phonetisaurus on my last copy of Raspberry Pi OS just a couple of weeks ago. I am running this on the latest copy of Raspberry Pi OS (released Feb 2023) but the version of python is still 3.9.2. I was able to download the phonetisaurus wheel file from https://github.com/rhasspy/phonetisaurus-pypi/releases/download/v0.3.0/phonetisaurus-0.3.0-py3-none-linux_aarch64.whl but doing that means we will have to be able to detect whether we are running on a an x86_64, arm7l, or aarch64.

@aaronchantrill
Copy link
Contributor

Actually, there is no reason to install either phonetisaurus or pocketsphinx from pip, since we are building and installing python-pocketsphinx from source and installing phonetisaurus from source and not using the python wrapper, so I'll just remove those for now.

@aaronchantrill
Copy link
Contributor

I think it makes sense to keep the question about running uninterrupted should stay with the processing at the top of the setup_wizard() function. I'm not sure what the top two sections of the WizardSetup() function do. The first one just seems to be an introduction which tells you that you are in the "deb" installer and that it will help setup Naomi, the second just says "Let's examine your security settings" which I think used to check if you were using the pi/raspberry username/password but now just does another sleep. In my edits, I have put the call to WizardSetup() right at the beginning of setup_wizard() and just moved the question about running uninterrupted out of that section.

@aaronchantrill
Copy link
Contributor

I get the error message "ImportError: Pocketsphinx not installed!" so it looks like the python setup.py install command has also been removed even though the pocketsphinx-python sources have been downloaded. I ran the installer and changed the "phonetisaurus_executable" setting to "phonetisaurus-g2pfst" and that finally got Naomi running.

aaronchantrill and others added 3 commits March 28, 2023 19:09
This contains some suggested fixes from my first attempt at using
this updated install script. Problems that still remain include
1) Phonetisaurus and the Pocketsphinx python package not being
installed
2) the naomi-setup.sh script importing external files that
would not be available if the user is using the one-liner
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

Successfully merging this pull request may close these issues.

None yet

3 participants