Naomi
Naomi copied to clipboard
Refactor bash scripts
Refactoring Bash Scripts
Refactoring the bash install scripts to avoid repetition of commonly used code.
Description
Did some refactoring of the commonly used functions/variables and moved them to one file. So to avoid writing the same commands in different files.
Related Issue
This PR is linked to issue #372
Motivation and Context
I feel it's better to avoid repeating the same thing over and over if we can just reference what we need
How Has This Been Tested?
This has been tested with Distrobox Containers(currently still in testing)
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.
Thanks @rileyhawk1417. I'm not sure why there are so many commits from two years ago included, or why there are conflicts with the current version of the files. I'm working on the install process also right now, so I'll download, merge and test your edits when I have a chance, sometime this week.
Not sure why the commits are pulling back old dates. Unless am creating a new branch wrong.
I usually do git checkout -b new_branch
As for the install files still had some cleaning to do, but I can put it on hold if it's something that might conflict with what you are doing
@rileyhawk1417 no, that sounds like the right way to create a new branch. Sometimes git can be strange. I don't think anything you do will conflict with what I'm doing. I'm just trying to figure out the best combination of packages for installing Naomi with the minimum amount of compiling.
Okay, I understand what you are doing with moving functions into files that can be sourced instead of requiring that the whole setup be copied and edited each time a new version of the installer is added. I like what you've done.
I did notice that you have both the original "python_requirements.txt" and a new "requirements.txt" file with pypi packages. You added pocketsphinx and phonetisaurus to the original "python-requirements.txt" file, and "requirements.txt" appears to be a copy of "python-requirements.txt" from before these changes. I know that "requirements.txt" is a more traditional name for a list of Pipy packages, so I wasn't sure if you were thinking of changing it. Anyway, I don't think we need it.
Instead of installing the Pypi "pocketsphinx" package, we currently build the pocketsphinx python package at the same time as pocketsphinx. In the new way of installing pocketsphinx that I am working on, we would be installing the apt "python3-pocketsphinx" and "python3-sphinxbase" packages along with "pocketsphinx", "sphinxtrain" and "sphinxbase". I do not actually see any pocketsphinx packages in the official Arch packages or in the AUR, so it may be that the best option for Arch is to continue to build pocketsphinx from source.
I'll work on resolving the conflicts in the next couple of days.
Here is an issue that we have had for a while. Using custom colors with the install scripts but not checking or controlling the background. This is a terminal on a new Debian Bullseye installation using Mate desktop:
The background of the terminal window is white by default, making the writing illegible. I think we need to just keep returning to the default text color "\033[0m" instead of setting the color to white.
@rileyhawk1417 The naomi_apt_requirements.sh script is missing. Is that something you are moving into a function? I am testing and had to restore it on my copy because the script.deb.sh still requires it. I'll make a pull request later. I'm just not sure what your goal is.
Was fully working on the Arch side of things, was going to sort out the apt script. The goal was to have one file that checks if it has apt or pacman package manager. Then install the requirements from the relevant txt files
Also, can you change sklearn to scikit-learn and remove pocketsphinx from your modified version of python-requirements.txt?
That will allow you to address issue sklearn breaks installer #371 at the same time.
Thanks!
@rileyhawk1417 Ah, I see that that the functionality from apt_requirements.sh has moved into naomi_requirements.sh which is a better way of doing it. Unfortunately the script.deb.sh still appears to reference the original file. I'll wait until you are done with your edits before testing in debian again. Thank you.
Here is just a written list of how I would like to see the installer functionality go. The first script is one that could be downloaded and run independently (without downloading the whole Naomi repository). The second script would be downloaded by the first (if it doesn't exist yet) and then run. For this reason, the first script should be used for collecting information but not access any include files, and the second script could access include files but should not collect information, if that makes sense.
Here are my thoughts:
- if Naomi has been installed, (~/.config/naomi exists) present the following menu:
Welcome to the Naomi Installer. Pick one of the options below to get started:
'Uninstall': This will remove Naomi from your system.
'Update': This will update Naomi if there is a newer release for your installed version.
'Version': This will allow you to switch what version of Naomi you have installed.
'AutoUpdate': This will allow you to enable/disable Naomi auto updates.
'Quit'
Otherwise, if Naomi has not been installed, present this menu:
Welcome to the Naomi Installer. Pick one of the options below to get started:
'Install': This will fresh install & setup Naomi on your system.
'Quit'
There should probably also be a “reinstall” option. Both reinstall and update should run through the entire install process from the downloaded install script file.
- Ask which version the user wants to install. The versions are:
Version Branch Auto Update
Nightly Naomi-dev Yes
Milestone Naomi-dev No
Stable master No
If the ~/Naomi/installers directory exists already, add an option to use the currently downloaded version. If they want to keep the current version, I think they should be set to “milestone” settings in the .naomi_options.json file.
-
If the user chooses “Install” and either does not have Naomi downloaded or has chosen not to use the currently downloaded version, back up the current ~/Naomi folder if there is one, then git clone the proper version.
-
Check if the user wants to start virtualenvwrapper automatically.
-
Check if the user wants to be prompted each time the installer uses sudo or not.
-
Pass control to the os script (script.apt.sh, script.pacman.sh, etc.)
-
Install system repository packages (apt, yum, zypper, pacman, etc)
-
Download and build any software that needs to be compiled (this should only be for core functionality; plugins should handle this in the plugin’s init method).
-
Install virtualenvwrapper and create a Naomi virtual environment.
-
Activate the virtualenvwrapper and update the .bashrc script to start virtualenvwrapper automatically if requested above.
-
Install Pypi pip packages.
-
Print a “Success” message asking the user to run Naomi with the --repopulate flag.
I concur with this. It does not change the way we currently do things too much but rather fixes some oversights and implements some minor adjustments.
I agree @AustinCasteel it does plug some holes that we might have missed, but at the same time improves a few places where the installer can be handled better.